From 491fe89bf14036cdaea88e0a5a5c6e3a630a78e1 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 1 Sep 2026 16:08:35 +0200 Subject: [PATCH] catch more cases of "we can know it's not in teardown" --- .../phases/3-transform/client/types.d.ts | 2 - .../phases/3-transform/client/utils.js | 185 +++++++++++++++++- .../3-transform/client/visitors/Identifier.js | 18 +- .../client/visitors/MemberExpression.js | 4 +- .../3-transform/client/visitors/Program.js | 6 +- .../client/visitors/SnippetBlock.js | 6 +- .../client/visitors/shared/utils.js | 6 +- .../Child.svelte | 3 +- .../_expected/client/index.svelte.js | 6 +- .../_expected/client/index.svelte.js | 25 +++ .../_expected/server/index.svelte.js | 23 +++ .../props-teardown-optimization/index.svelte | 26 +++ 12 files changed, 284 insertions(+), 26 deletions(-) create mode 100644 packages/svelte/tests/snapshot/samples/props-teardown-optimization/_expected/client/index.svelte.js create mode 100644 packages/svelte/tests/snapshot/samples/props-teardown-optimization/_expected/server/index.svelte.js create mode 100644 packages/svelte/tests/snapshot/samples/props-teardown-optimization/index.svelte diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 77ca04eace..7a95a2d43c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/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);` */ diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 11d5bc8f39..d402d764da 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/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.is_instance || transform.read_template === undefined - ? transform.read(node) - : transform.read_template(node); + return state.transform[node.name].read(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} 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} 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} 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 diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js index 8525c7abc1..1e339798d1 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Identifier.js +++ b/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); } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js index 34393fb721..86d51234ce 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js +++ b/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); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js index df2de61209..f6352e986c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js +++ b/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_template: (_) => b.member(b.id('$$props'), key, key.type === 'Literal') + read: (_) => 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_template: (node) => b.member(b.id('$$props'), node) + read: (node) => b.member(b.id('$$props'), node) }; } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js index 67444f80e7..6c3a48c5ea 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js +++ b/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 = - context.state.is_instance || transform[name].read_template === undefined - ? transform[name].read - : transform[name].read_template; - declarations.push(b.stmt(read(b.id(name)))); + declarations.push(b.stmt(transform[name].read(b.id(name)))); } } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index b2519c5426..64ae573984 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/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 = - state.is_instance || transform?.read_template === undefined - ? transform?.read - : transform.read_template; - path.unshift(read ? read(left.property) : left.property); + path.unshift(transform?.read ? transform.read(left.property) : left.property); } else { path.unshift(b.literal(left.property.name)); } diff --git a/packages/svelte/tests/runtime-runes/samples/effect-teardown-unrelated-state-write/Child.svelte b/packages/svelte/tests/runtime-runes/samples/effect-teardown-unrelated-state-write/Child.svelte index e2b6c716ab..9bb373117c 100644 --- a/packages/svelte/tests/runtime-runes/samples/effect-teardown-unrelated-state-write/Child.svelte +++ b/packages/svelte/tests/runtime-runes/samples/effect-teardown-unrelated-state-write/Child.svelte @@ -1,9 +1,10 @@ diff --git a/packages/svelte/tests/snapshot/samples/props-identifier/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/props-identifier/_expected/client/index.svelte.js index 8c5db59d4c..7df616f694 100644 --- a/packages/svelte/tests/snapshot/samples/props-identifier/_expected/client/index.svelte.js +++ b/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; - $.get_prop_value($$props, 'a').b = true; + $$props.a.b; + $$props.a.b = true; props.a = true; props[a] = true; props; diff --git a/packages/svelte/tests/snapshot/samples/props-teardown-optimization/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/props-teardown-optimization/_expected/client/index.svelte.js new file mode 100644 index 0000000000..81c6d9c18e --- /dev/null +++ b/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(); +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/props-teardown-optimization/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/props-teardown-optimization/_expected/server/index.svelte.js new file mode 100644 index 0000000000..8f689df23b --- /dev/null +++ b/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())}`); + }); +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/props-teardown-optimization/index.svelte b/packages/svelte/tests/snapshot/samples/props-teardown-optimization/index.svelte new file mode 100644 index 0000000000..d26289a447 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/props-teardown-optimization/index.svelte @@ -0,0 +1,26 @@ + + +{doubled}