Skip to content

Commit 23d42fb

Browse files
authored
fix: put expressions in effects unless known to be static (#15792)
* fix: put expressions in effects unless known to be static * update test * handle cycles * fix
1 parent 7a8cb37 commit 23d42fb

File tree

7 files changed

+59
-6
lines changed

7 files changed

+59
-6
lines changed

.changeset/tender-apples-repair.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: put expressions in effects unless known to be static

packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,10 @@ export function Identifier(node, context) {
9090
if (binding) {
9191
if (context.state.expression) {
9292
context.state.expression.dependencies.add(binding);
93-
context.state.expression.has_state ||= binding.kind !== 'normal';
93+
context.state.expression.has_state ||=
94+
binding.kind !== 'static' &&
95+
!binding.is_function() &&
96+
!context.state.scope.evaluate(node).is_known;
9497
}
9598

9699
if (

packages/svelte/src/compiler/phases/scope.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,8 @@ class Evaluation {
221221
* @param {Set<any>} values
222222
*/
223223
constructor(scope, expression, values) {
224+
current_evaluations.set(expression, this);
225+
224226
this.values = values;
225227

226228
switch (expression.type) {
@@ -543,6 +545,8 @@ class Evaluation {
543545
if (this.values.size > 1 || typeof this.value === 'symbol') {
544546
this.is_known = false;
545547
}
548+
549+
current_evaluations.delete(expression);
546550
}
547551
}
548552

@@ -734,10 +738,20 @@ export class Scope {
734738
* @param {Set<any>} [values]
735739
*/
736740
evaluate(expression, values = new Set()) {
741+
const current = current_evaluations.get(expression);
742+
if (current) return current;
743+
737744
return new Evaluation(this, expression, values);
738745
}
739746
}
740747

748+
/**
749+
* Track which expressions are currently being evaluated — this allows
750+
* us to prevent cyclical evaluations without passing the map around
751+
* @type {Map<Expression | FunctionDeclaration, Evaluation>}
752+
*/
753+
const current_evaluations = new Map();
754+
741755
/** @type {Record<BinaryOperator, (left: any, right: any) => any>} */
742756
const binary = {
743757
'!=': (left, right) => left != right,
@@ -1139,7 +1153,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
11391153
const is_keyed =
11401154
node.key &&
11411155
(node.key.type !== 'Identifier' || !node.index || node.key.name !== node.index);
1142-
scope.declare(b.id(node.index), is_keyed ? 'template' : 'normal', 'const', node);
1156+
scope.declare(b.id(node.index), is_keyed ? 'template' : 'static', 'const', node);
11431157
}
11441158
if (node.key) visit(node.key, { scope });
11451159

packages/svelte/src/compiler/types/index.d.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,8 @@ export type BindingKind =
265265
| 'snippet' // A snippet parameter
266266
| 'store_sub' // A $store value
267267
| 'legacy_reactive' // A `$:` declaration
268-
| 'template'; // A binding declared in the template, e.g. in an `await` block or `const` tag
268+
| 'template' // A binding declared in the template, e.g. in an `await` block or `const` tag
269+
| 'static'; // A binding whose value is known to be static (i.e. each index)
269270

270271
export type DeclarationKind =
271272
| 'var'
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { flushSync } from '../../../../src/index-client';
2+
import { test } from '../../test';
3+
4+
export default test({
5+
html: `<button>0</button>`,
6+
7+
test({ assert, target }) {
8+
const btn = target.querySelector('button');
9+
10+
flushSync(() => btn?.click());
11+
assert.htmlEqual(target.innerHTML, `<button>1</button>`);
12+
}
13+
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<script>
2+
let count = $state(0);
3+
4+
let object = {
5+
toString() {
6+
return count;
7+
}
8+
};
9+
</script>
10+
11+
<button onclick={() => count += 1}>
12+
{object}
13+
</button>

packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,16 @@ export default function Main($$anchor) {
99
let y = () => 'test';
1010
var fragment = root();
1111
var div = $.first_child(fragment);
12+
13+
$.set_attribute(div, 'foobar', x);
14+
1215
var svg = $.sibling(div, 2);
16+
17+
$.set_attribute(svg, 'viewBox', x);
18+
1319
var custom_element = $.sibling(svg, 2);
1420

15-
$.template_effect(() => $.set_custom_element_data(custom_element, 'fooBar', x));
21+
$.set_custom_element_data(custom_element, 'fooBar', x);
1622

1723
var div_1 = $.sibling(custom_element, 2);
1824
var svg_1 = $.sibling(div_1, 2);
@@ -22,8 +28,6 @@ export default function Main($$anchor) {
2228

2329
$.template_effect(
2430
($0, $1) => {
25-
$.set_attribute(div, 'foobar', x);
26-
$.set_attribute(svg, 'viewBox', x);
2731
$.set_attribute(div_1, 'foobar', $0);
2832
$.set_attribute(svg_1, 'viewBox', $1);
2933
},

0 commit comments

Comments
 (0)