From 14642b830bd80e60ea2a2c4a18a89954f692e4c5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 17 Apr 2025 21:03:19 -0400 Subject: [PATCH 1/4] fix: put expressions in effects unless known to be static --- .changeset/tender-apples-repair.md | 5 +++++ .../phases/2-analyze/visitors/Identifier.js | 3 ++- .../samples/reactive-identifier/_config.js | 13 +++++++++++++ .../samples/reactive-identifier/main.svelte | 13 +++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 .changeset/tender-apples-repair.md create mode 100644 packages/svelte/tests/runtime-runes/samples/reactive-identifier/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/reactive-identifier/main.svelte diff --git a/.changeset/tender-apples-repair.md b/.changeset/tender-apples-repair.md new file mode 100644 index 000000000000..0e600e66241a --- /dev/null +++ b/.changeset/tender-apples-repair.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: put expressions in effects unless known to be static diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js index efbbe6cfa287..83ac8000d311 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js @@ -90,7 +90,8 @@ export function Identifier(node, context) { if (binding) { if (context.state.expression) { context.state.expression.dependencies.add(binding); - context.state.expression.has_state ||= binding.kind !== 'normal'; + context.state.expression.has_state ||= + !binding.is_function() && !context.state.scope.evaluate(node).is_known; } if ( diff --git a/packages/svelte/tests/runtime-runes/samples/reactive-identifier/_config.js b/packages/svelte/tests/runtime-runes/samples/reactive-identifier/_config.js new file mode 100644 index 000000000000..de59e8c7c7c8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/reactive-identifier/_config.js @@ -0,0 +1,13 @@ +import { flushSync } from '../../../../src/index-client'; +import { test } from '../../test'; + +export default test({ + html: ``, + + test({ assert, target }) { + const btn = target.querySelector('button'); + + flushSync(() => btn?.click()); + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/reactive-identifier/main.svelte b/packages/svelte/tests/runtime-runes/samples/reactive-identifier/main.svelte new file mode 100644 index 000000000000..aee0b15e1beb --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/reactive-identifier/main.svelte @@ -0,0 +1,13 @@ + + + From 29eec3fc3342476d69948d00ee2bf0c4d9d0a0a4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 29 May 2025 09:03:10 -0400 Subject: [PATCH 2/4] update test --- .../_expected/client/main.svelte.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js index 3a13fa7e15d9..28bb01fb18df 100644 --- a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js @@ -9,10 +9,16 @@ export default function Main($$anchor) { let y = () => 'test'; var fragment = root(); var div = $.first_child(fragment); + + $.set_attribute(div, 'foobar', x); + var svg = $.sibling(div, 2); + + $.set_attribute(svg, 'viewBox', x); + var custom_element = $.sibling(svg, 2); - $.template_effect(() => $.set_custom_element_data(custom_element, 'fooBar', x)); + $.set_custom_element_data(custom_element, 'fooBar', x); var div_1 = $.sibling(custom_element, 2); var svg_1 = $.sibling(div_1, 2); @@ -22,8 +28,6 @@ export default function Main($$anchor) { $.template_effect( ($0, $1) => { - $.set_attribute(div, 'foobar', x); - $.set_attribute(svg, 'viewBox', x); $.set_attribute(div_1, 'foobar', $0); $.set_attribute(svg_1, 'viewBox', $1); }, From ffc476957129ec77ae9a32ac6972d131915c261a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 29 May 2025 09:22:15 -0400 Subject: [PATCH 3/4] handle cycles --- packages/svelte/src/compiler/phases/scope.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index b4cdb6b44673..8fc4da815423 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -221,6 +221,8 @@ class Evaluation { * @param {Set} values */ constructor(scope, expression, values) { + current_evaluations.set(expression, this); + this.values = values; switch (expression.type) { @@ -543,6 +545,8 @@ class Evaluation { if (this.values.size > 1 || typeof this.value === 'symbol') { this.is_known = false; } + + current_evaluations.delete(expression); } } @@ -734,10 +738,20 @@ export class Scope { * @param {Set} [values] */ evaluate(expression, values = new Set()) { + const current = current_evaluations.get(expression); + if (current) return current; + return new Evaluation(this, expression, values); } } +/** + * Track which expressions are currently being evaluated — this allows + * us to prevent cyclical evaluations without passing the map around + * @type {Map} + */ +const current_evaluations = new Map(); + /** @type {Record any>} */ const binary = { '!=': (left, right) => left != right, From 930c5325db16cb75b66781ce27109954349f585f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 29 May 2025 09:48:22 -0400 Subject: [PATCH 4/4] fix --- .../src/compiler/phases/2-analyze/visitors/Identifier.js | 4 +++- packages/svelte/src/compiler/phases/scope.js | 2 +- packages/svelte/src/compiler/types/index.d.ts | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js index 098e4535d9df..abf70769c013 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js @@ -91,7 +91,9 @@ export function Identifier(node, context) { if (context.state.expression) { context.state.expression.dependencies.add(binding); context.state.expression.has_state ||= - !binding.is_function() && !context.state.scope.evaluate(node).is_known; + binding.kind !== 'static' && + !binding.is_function() && + !context.state.scope.evaluate(node).is_known; } if ( diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 8fc4da815423..8b4967a82baf 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -1153,7 +1153,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { const is_keyed = node.key && (node.key.type !== 'Identifier' || !node.index || node.key.name !== node.index); - scope.declare(b.id(node.index), is_keyed ? 'template' : 'normal', 'const', node); + scope.declare(b.id(node.index), is_keyed ? 'template' : 'static', 'const', node); } if (node.key) visit(node.key, { scope }); diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 0da036242b07..fdd602472608 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -265,7 +265,8 @@ export type BindingKind = | 'snippet' // A snippet parameter | 'store_sub' // A $store value | 'legacy_reactive' // A `$:` declaration - | 'template'; // A binding declared in the template, e.g. in an `await` block or `const` tag + | 'template' // A binding declared in the template, e.g. in an `await` block or `const` tag + | 'static'; // A binding whose value is known to be static (i.e. each index) export type DeclarationKind = | 'var'