Skip to content

Commit d2859a6

Browse files
committed
fix: treat new expression references in template as possibly dynamic
1 parent ee4b1f2 commit d2859a6

File tree

4 files changed

+56
-23
lines changed

4 files changed

+56
-23
lines changed

.changeset/gorgeous-pugs-design.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: treat new expression references in template as possibly dynamic

packages/svelte/src/compiler/phases/2-analyze/index.js

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -997,26 +997,37 @@ const common_visitors = {
997997
const binding = context.state.scope.get(node.name);
998998

999999
// if no binding, means some global variable
1000-
if (binding && binding.kind !== 'normal') {
1001-
if (context.state.expression) {
1002-
context.state.expression.metadata.dynamic = true;
1003-
}
1000+
if (binding) {
1001+
if (binding.kind === 'normal') {
1002+
const initial = binding.initial;
1003+
1004+
if (initial?.type === 'NewExpression' && !binding.reassigned && context.state.expression) {
1005+
// If the identifier comes from a new expression, then the object might have a custom
1006+
// toString() and thus be reactive. So let's treat the expression as dynamic, unless the
1007+
// binding has been re-assigned.
1008+
context.state.expression.metadata.dynamic = true;
1009+
}
1010+
} else {
1011+
if (context.state.expression) {
1012+
context.state.expression.metadata.dynamic = true;
1013+
}
10041014

1005-
if (
1006-
node !== binding.node &&
1007-
// If we have $state that can be proxied or frozen and isn't re-assigned, then that means
1008-
// it's likely not using a primitive value and thus this warning isn't that helpful.
1009-
((binding.kind === 'state' &&
1010-
(binding.reassigned ||
1011-
(binding.initial?.type === 'CallExpression' &&
1012-
binding.initial.arguments.length === 1 &&
1013-
binding.initial.arguments[0].type !== 'SpreadElement' &&
1014-
!should_proxy_or_freeze(binding.initial.arguments[0], context.state.scope)))) ||
1015-
binding.kind === 'frozen_state' ||
1016-
binding.kind === 'derived') &&
1017-
context.state.function_depth === binding.scope.function_depth
1018-
) {
1019-
warn(context.state.analysis.warnings, node, context.path, 'static-state-reference');
1015+
if (
1016+
node !== binding.node &&
1017+
// If we have $state that can be proxied or frozen and isn't re-assigned, then that means
1018+
// it's likely not using a primitive value and thus this warning isn't that helpful.
1019+
((binding.kind === 'state' &&
1020+
(binding.reassigned ||
1021+
(binding.initial?.type === 'CallExpression' &&
1022+
binding.initial.arguments.length === 1 &&
1023+
binding.initial.arguments[0].type !== 'SpreadElement' &&
1024+
!should_proxy_or_freeze(binding.initial.arguments[0], context.state.scope)))) ||
1025+
binding.kind === 'frozen_state' ||
1026+
binding.kind === 'derived') &&
1027+
context.state.function_depth === binding.scope.function_depth
1028+
) {
1029+
warn(context.state.analysis.warnings, node, context.path, 'static-state-reference');
1030+
}
10201031
}
10211032
}
10221033
},

packages/svelte/tests/runtime-runes/samples/date/_config.js

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,24 @@
11
import { flushSync } from '../../../../src/main/main-client';
22
import { test } from '../../test';
33

4+
const date_proto = Date.prototype;
5+
let date_proto_to_string = date_proto.toString;
6+
47
export default test({
5-
html: `<div>getSeconds: 0</div><div>getMinutes: 0</div><div>getHours: 15</div><div>getTime: 1708700400000</div><div>toDateString: Fri Feb 23 2024</div><button>1 second</button><button>1 minute</button><button>1 hour</button>`,
8+
html: `<div>getSeconds: 0</div><div>getMinutes: 0</div><div>getHours: 15</div><div>getTime: 1708700400000</div><div>toDateString: Fri Feb 23 2024</div><div>date: [date: 0, 0, 15]</div><button>1 second</button><button>1 minute</button><button>1 hour</button>`,
9+
10+
before_test() {
11+
date_proto_to_string = date_proto.toString;
12+
13+
// This test will fail between different machines because of timezones, so we instead mock it to be a different toString().
14+
date_proto.toString = function () {
15+
return `[date: ${this.getSeconds()}, ${this.getMinutes()}, ${this.getHours()}]`;
16+
};
17+
},
18+
19+
after_test() {
20+
date_proto.toString = date_proto_to_string;
21+
},
622

723
test({ assert, target }) {
824
const [btn, btn2, btn3] = target.querySelectorAll('button');
@@ -13,7 +29,7 @@ export default test({
1329

1430
assert.htmlEqual(
1531
target.innerHTML,
16-
`<div>getSeconds: 1</div><div>getMinutes: 0</div><div>getHours: 15</div><div>getTime: 1708700401000</div><div>toDateString: Fri Feb 23 2024</div><button>1 second</button><button>1 minute</button><button>1 hour</button>`
32+
`<div>getSeconds: 1</div><div>getMinutes: 0</div><div>getHours: 15</div><div>getTime: 1708700401000</div><div>toDateString: Fri Feb 23 2024</div><div>date: [date: 1, 0, 15]</div><button>1 second</button><button>1 minute</button><button>1 hour</button>`
1733
);
1834

1935
flushSync(() => {
@@ -22,7 +38,7 @@ export default test({
2238

2339
assert.htmlEqual(
2440
target.innerHTML,
25-
`<div>getSeconds: 1</div><div>getMinutes: 1</div><div>getHours: 15</div><div>getTime: 1708700461000</div><div>toDateString: Fri Feb 23 2024</div><button>1 second</button><button>1 minute</button><button>1 hour</button>`
41+
`<div>getSeconds: 1</div><div>getMinutes: 1</div><div>getHours: 15</div><div>getTime: 1708700461000</div><div>toDateString: Fri Feb 23 2024</div><div>date: [date: 1, 1, 15]</div><button>1 second</button><button>1 minute</button><button>1 hour</button>`
2642
);
2743

2844
flushSync(() => {
@@ -31,7 +47,7 @@ export default test({
3147

3248
assert.htmlEqual(
3349
target.innerHTML,
34-
`<div>getSeconds: 1</div><div>getMinutes: 1</div><div>getHours: 16</div><div>getTime: 1708704061000</div><div>toDateString: Fri Feb 23 2024</div><button>1 second</button><button>1 minute</button><button>1 hour</button>`
50+
`<div>getSeconds: 1</div><div>getMinutes: 1</div><div>getHours: 16</div><div>getTime: 1708704061000</div><div>toDateString: Fri Feb 23 2024</div><div>date: [date: 1, 1, 16]</div><button>1 second</button><button>1 minute</button><button>1 hour</button>`
3551
);
3652
}
3753
});

packages/svelte/tests/runtime-runes/samples/date/main.svelte

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
<div>getHours: {date.getUTCHours()}</div>
1010
<div>getTime: {date.getTime()}</div>
1111
<div>toDateString: {date.toDateString()}</div>
12+
<div>date: {date}</div>
1213

1314
<button onclick={() => {
1415
date.setSeconds(date.getSeconds() + 1);

0 commit comments

Comments
 (0)