catch more cases of "we can know it's not in teardown"

packages/svelte/src/compiler/phases/3-transform/client/types.d.ts

@@ -26,8 +26,6 @@ export interface ClientTransformState extends TransformState {
 		{
 			/** turn `foo` into e.g. `$.get(foo)` */
 			read: (id: Identifier) => Expression;
-			/** turn `foo` into e.g. `$$props.foo` inside the template */
-			read_template?: (id: Identifier) => Expression;
 			/** turn `foo = bar` into e.g. `$.set(foo, bar)` */
 			assign?: (node: Identifier, value: Expression, proxy?: boolean) => Expression;
 			/** turn `foo.bar = baz` into e.g. `$.mutate(foo, $.get(foo).bar = baz);` */

packages/svelte/src/compiler/phases/3-transform/client/utils.js

@@ -13,6 +13,7 @@ import {
 	PROPS_IS_UPDATED,
 	PROPS_IS_BINDABLE
 } from '../../../../constants.js';
+import { get_rune } from '../../scope.js';
 
 /**
  * @param {Binding} binding
@@ -37,16 +38,192 @@ export function build_getter(node, state) {
 
 		// don't transform the declaration itself
 		if (node !== binding?.node) {
-			var transform = state.transform[node.name];
+			return state.transform[node.name].read(node);
-			return state.is_instance || transform.read_template === undefined
-				? transform.read(node)
-				: transform.read_template(node);
 		}
 	}
 
 	return node;
 }
 
+/**
+ * @param {Binding} binding
+ * @param {Identifier} node
+ * @param {ClientTransformState} state
+ */
+export function prop_read_may_be_in_teardown(binding, node, state) {
+	const reference = binding.references.find((reference) => reference.node === node);
+	if (reference === undefined) return false;
+
+	return reference_may_be_in_teardown(reference, state, new Set());
+}
+
+/**
+ * @param {Binding['references'][number]} reference
+ * @param {ClientTransformState} state
+ * @param {Set<Binding>} checked
+ */
+function reference_may_be_in_teardown(reference, state, checked) {
+	const { path } = reference;
+
+	for (let i = path.length - 1; i >= 0; i -= 1) {
+		const fn = path[i];
+
+		if (
+			fn.type !== 'ArrowFunctionExpression' &&
+			fn.type !== 'FunctionExpression' &&
+			fn.type !== 'FunctionDeclaration'
+		) {
+			continue;
+		}
+
+		const parent = path[i - 1];
+
+		if (parent?.type === 'CallExpression') {
+			// An IIFE executes in the same context as the function containing it.
+			if (parent.callee === fn) continue;
+
+			if (parent.arguments.includes(/** @type {Expression} */ (fn))) {
+				const rune = get_rune(parent, get_scope(path, i - 1, state));
+
+				if (is_effect_rune(rune)) return false;
+				if (rune === '$derived.by') {
+					return derived_may_be_read_in_teardown(parent, path, i - 1, state, checked);
+				}
+			}
+		}
+
+		const function_binding = get_function_binding(fn, path, i, state);
+		return function_binding === null
+			? true
+			: function_may_be_called_in_teardown(function_binding, state, checked);
+	}
+
+	for (let i = path.length - 1; i >= 0; i -= 1) {
+		const node = path[i];
+		if (node.type !== 'CallExpression') continue;
+
+		const rune = get_rune(node, get_scope(path, i, state));
+		if (rune === '$derived' || rune === '$derived.by') {
+			return derived_may_be_read_in_teardown(node, path, i, state, checked);
+		}
+	}
+
+	return false;
+}
+
+/**
+ * @param {Binding} binding
+ * @param {ClientTransformState} state
+ * @param {Set<Binding>} checked
+ */
+function function_may_be_called_in_teardown(binding, state, checked) {
+	if (checked.has(binding)) return false;
+	checked.add(binding);
+
+	for (const reference of binding.references) {
+		if (reference.node === binding.node) continue;
+
+		const parent = reference.path.at(-1);
+
+		if (parent?.type !== 'CallExpression') return true;
+
+		if (parent.callee === reference.node) {
+			if (reference_may_be_in_teardown(reference, state, checked)) return true;
+			continue;
+		}
+
+		if (!parent.arguments.includes(reference.node)) return true;
+
+		const rune = get_rune(parent, get_scope(reference.path, reference.path.length - 1, state));
+
+		if (is_effect_rune(rune)) continue;
+		if (
+			rune === '$derived.by' &&
+			!derived_may_be_read_in_teardown(
+				parent,
+				reference.path,
+				reference.path.length - 1,
+				state,
+				checked
+			)
+		) {
+			continue;
+		}
+
+		return true;
+	}
+
+	return false;
+}
+
+/**
+ * @param {import('estree').CallExpression} call
+ * @param {import('#compiler').AST.SvelteNode[]} path
+ * @param {number} index
+ * @param {ClientTransformState} state
+ * @param {Set<Binding>} checked
+ */
+function derived_may_be_read_in_teardown(call, path, index, state, checked) {
+	const parent = path[index - 1];
+	if (parent?.type !== 'VariableDeclarator' || parent.init !== call) return true;
+	if (parent.id.type !== 'Identifier') return true;
+
+	const binding = get_scope(path, index - 1, state).get(parent.id.name);
+	if (binding === null || checked.has(binding)) return false;
+
+	checked.add(binding);
+
+	for (const reference of binding.references) {
+		if (reference.node === binding.node) continue;
+		if (reference_may_be_in_teardown(reference, state, checked)) return true;
+	}
+
+	return false;
+}
+
+/**
+ * @param {import('estree').FunctionDeclaration | import('estree').FunctionExpression | import('estree').ArrowFunctionExpression} fn
+ * @param {import('#compiler').AST.SvelteNode[]} path
+ * @param {number} index
+ * @param {ClientTransformState} state
+ * @returns {Binding | null}
+ */
+function get_function_binding(fn, path, index, state) {
+	if (fn.type === 'FunctionDeclaration' && fn.id !== null) {
+		return get_scope(path, index - 1, state).get(fn.id.name);
+	}
+
+	const parent = path[index - 1];
+	if (
+		parent?.type === 'VariableDeclarator' &&
+		parent.init === fn &&
+		parent.id.type === 'Identifier'
+	) {
+		return get_scope(path, index - 1, state).get(parent.id.name);
+	}
+
+	return null;
+}
+
+/**
+ * @param {import('#compiler').AST.SvelteNode[]} path
+ * @param {number} index
+ * @param {ClientTransformState} state
+ */
+function get_scope(path, index, state) {
+	for (let i = index; i >= 0; i -= 1) {
+		const scope = state.scopes.get(path[i]);
+		if (scope !== undefined) return scope;
+	}
+
+	return state.scope;
+}
+
+/** @param {string | null} rune */
+function is_effect_rune(rune) {
+	return rune === '$effect' || rune === '$effect.pre' || rune === '$effect.root';
+}
+
 /**
  * @param {Binding} binding
  * @param {ComponentClientTransformState} state

packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js

@@ -2,7 +2,7 @@
 /** @import { Context } from '../types' */
 import is_reference from 'is-reference';
 import * as b from '#compiler/builders';
-import { build_getter } from '../utils.js';
+import { build_getter, is_prop_source, prop_read_may_be_in_teardown } from '../utils.js';
 
 /**
  * @param {Identifier} node
@@ -16,6 +16,22 @@ export function Identifier(node, context) {
 			return b.id('$$sanitized_props');
 		}
 
+		const binding = context.state.scope.get(node.name);
+		if (
+			context.state.is_instance &&
+			binding !== null &&
+			node !== binding.node &&
+			(binding.kind === 'prop' || binding.kind === 'bindable_prop') &&
+			!is_prop_source(binding, context.state) &&
+			prop_read_may_be_in_teardown(binding, node, context.state)
+		) {
+			return b.call(
+				'$.get_prop_value',
+				b.id('$$props'),
+				b.literal(binding.prop_alias ?? node.name)
+			);
+		}
+
 		return build_getter(node, context.state);
 	}
 }

packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js

@@ -1,6 +1,7 @@
 /** @import { MemberExpression } from 'estree' */
 /** @import { Context } from '../types' */
 import * as b from '#compiler/builders';
+import { prop_read_may_be_in_teardown } from '../utils.js';
 
 /**
  * @param {MemberExpression} node
@@ -23,7 +24,8 @@ export function MemberExpression(node, context) {
 			parent?.type !== 'UpdateExpression' &&
 			!binding.metadata?.exclude_props?.includes(node.property.name)
 		) {
-			return context.state.is_instance
+			return context.state.is_instance &&
+				prop_read_may_be_in_teardown(binding, node.object, context.state)
 				? b.call('$.get_prop_value', b.id('$$props'), b.literal(node.property.name))
 				: b.member(b.id('$$props'), node.property);
 		}

packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js

@@ -126,13 +126,11 @@ export function Program(node, context) {
 				const key = b.key(binding.prop_alias);
 
 				context.state.transform[name] = {
-					read: (_) => b.call('$.get_prop_value', b.id('$$props'), b.literal(binding.prop_alias)),
+					read: (_) => b.member(b.id('$$props'), key, key.type === 'Literal')
-					read_template: (_) => b.member(b.id('$$props'), key, key.type === 'Literal')
 				};
 			} else {
 				context.state.transform[name] = {
-					read: (_) => b.call('$.get_prop_value', b.id('$$props'), b.literal(name)),
+					read: (node) => b.member(b.id('$$props'), node)
-					read_template: (node) => b.member(b.id('$$props'), node)
 				};
 			}
 		}

packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js

@@ -63,11 +63,7 @@ export function SnippetBlock(node, context) {
 			// we need to eagerly evaluate the expression in order to hit any
 			// 'Cannot access x before initialization' errors
 			if (dev) {
-				var read =
+				declarations.push(b.stmt(transform[name].read(b.id(name))));
-					context.state.is_instance || transform[name].read_template === undefined
-						? transform[name].read
-						: transform[name].read_template;
-				declarations.push(b.stmt(read(b.id(name))));
 			}
 		}
 	}

packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js

@@ -416,11 +416,7 @@ export function validate_mutation(node, context, expression) {
 				? context.state.transform[left.property.name]
 				: null;
 			if (left.computed) {
-				var read =
+				path.unshift(transform?.read ? transform.read(left.property) : left.property);
-					state.is_instance || transform?.read_template === undefined
-						? transform?.read
-						: transform.read_template;
-				path.unshift(read ? read(left.property) : left.property);
 			} else {
 				path.unshift(b.literal(left.property.name));
 			}

packages/svelte/tests/runtime-runes/samples/effect-teardown-unrelated-state-write/Child.svelte

@@ -1,9 +1,10 @@
 <script>
 	let { track, value, ...rest } = $props();
+	let indirect = $derived(rest.other);
 
 	$effect(() => {
 		track;
 
-		return () => console.log(`value = ${value}, other = ${rest.other}`);
+		return () => console.log(`value = ${value}, other = ${indirect}`);
 	});
 </script>

packages/svelte/tests/snapshot/samples/props-identifier/_expected/client/index.svelte.js

@@ -8,10 +8,10 @@ export default function Props_identifier($$anchor, $$props) {
 
 	let props = $.rest_props($$props, rest_excludes);
 
-	$.get_prop_value($$props, 'a');
+	$$props.a;
 	props[a];
-	$.get_prop_value($$props, 'a').b;
+	$$props.a.b;
-	$.get_prop_value($$props, 'a').b = true;
+	$$props.a.b = true;
 	props.a = true;
 	props[a] = true;
 	props;

packages/svelte/tests/snapshot/samples/props-teardown-optimization/_expected/client/index.svelte.js

@@ -0,0 +1,25 @@
+import 'svelte/internal/disclose-version';
+import * as $ from 'svelte/internal/client';
+
+export default function Props_teardown_optimization($$anchor, $$props) {
+	$.push($$props, true);
+
+	let doubled = $.derived(() => $$props.derived_prop * 2);
+	let indirect = $.derived(() => $.get_prop_value($$props, 'indirect_prop'));
+	let read_function_prop = () => $$props.function_prop;
+	let read_function_cleanup_prop = () => $.get_prop_value($$props, 'function_cleanup_prop');
+
+	$.user_effect(() => console.log($$props.effect_prop));
+	$.user_effect(() => console.log(read_function_prop()));
+	$.user_effect(() => () => console.log($.get_prop_value($$props, 'cleanup_prop')));
+	$.user_effect(() => () => console.log($.get(indirect)));
+	$.user_effect(() => () => console.log(read_function_cleanup_prop()));
+	someFunction(() => $.get_prop_value($$props, 'unknown_prop'));
+	$.next();
+
+	var text = $.text();
+
+	$.template_effect(() => $.set_text(text, $.get(doubled)));
+	$.append($$anchor, text);
+	$.pop();
+}

packages/svelte/tests/snapshot/samples/props-teardown-optimization/_expected/server/index.svelte.js

@@ -0,0 +1,23 @@
+import * as $ from 'svelte/internal/server';
+
+export default function Props_teardown_optimization($$renderer, $$props) {
+	$$renderer.component(($$renderer) => {
+		let {
+			derived_prop,
+			indirect_prop,
+			function_prop,
+			function_cleanup_prop,
+			effect_prop,
+			unknown_prop,
+			cleanup_prop
+		} = $$props;
+
+		let doubled = $.derived(() => derived_prop * 2);
+		let indirect = $.derived(() => indirect_prop);
+		let read_function_prop = () => function_prop;
+		let read_function_cleanup_prop = () => function_cleanup_prop;
+
+		someFunction(() => unknown_prop);
+		$$renderer.push(`<!---->${$.escape(doubled())}`);
+	});
+}

packages/svelte/tests/snapshot/samples/props-teardown-optimization/index.svelte

@@ -0,0 +1,26 @@
+<script>
+	let {
+		derived_prop,
+		indirect_prop,
+		function_prop,
+		function_cleanup_prop,
+		effect_prop,
+		unknown_prop,
+		cleanup_prop
+	} = $props();
+
+	let doubled = $derived(derived_prop * 2);
+	let indirect = $derived(indirect_prop);
+	let read_function_prop = () => function_prop;
+	let read_function_cleanup_prop = () => function_cleanup_prop;
+
+	$effect(() => console.log(effect_prop));
+	$effect(() => console.log(read_function_prop()));
+	$effect(() => () => console.log(cleanup_prop));
+	$effect(() => () => console.log(indirect));
+	$effect(() => () => console.log(read_function_cleanup_prop()));
+
+	someFunction(() => unknown_prop);
+</script>
+
+{doubled}