From d2859a6e16fca47644a6c884699c07d498e334e2 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 26 Feb 2024 10:23:44 +0000 Subject: [PATCH] fix: treat new expression references in template as possibly dynamic --- .changeset/gorgeous-pugs-design.md | 5 ++ .../src/compiler/phases/2-analyze/index.js | 49 ++++++++++++------- .../runtime-runes/samples/date/_config.js | 24 +++++++-- .../runtime-runes/samples/date/main.svelte | 1 + 4 files changed, 56 insertions(+), 23 deletions(-) create mode 100644 .changeset/gorgeous-pugs-design.md diff --git a/.changeset/gorgeous-pugs-design.md b/.changeset/gorgeous-pugs-design.md new file mode 100644 index 0000000000..0449a66156 --- /dev/null +++ b/.changeset/gorgeous-pugs-design.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: treat new expression references in template as possibly dynamic diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index f2e2a3dec2..8b2176b731 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -997,26 +997,37 @@ const common_visitors = { const binding = context.state.scope.get(node.name); // if no binding, means some global variable - if (binding && binding.kind !== 'normal') { - if (context.state.expression) { - context.state.expression.metadata.dynamic = true; - } + if (binding) { + if (binding.kind === 'normal') { + const initial = binding.initial; + + if (initial?.type === 'NewExpression' && !binding.reassigned && context.state.expression) { + // If the identifier comes from a new expression, then the object might have a custom + // toString() and thus be reactive. So let's treat the expression as dynamic, unless the + // binding has been re-assigned. + context.state.expression.metadata.dynamic = true; + } + } else { + if (context.state.expression) { + context.state.expression.metadata.dynamic = true; + } - if ( - node !== binding.node && - // If we have $state that can be proxied or frozen and isn't re-assigned, then that means - // it's likely not using a primitive value and thus this warning isn't that helpful. - ((binding.kind === 'state' && - (binding.reassigned || - (binding.initial?.type === 'CallExpression' && - binding.initial.arguments.length === 1 && - binding.initial.arguments[0].type !== 'SpreadElement' && - !should_proxy_or_freeze(binding.initial.arguments[0], context.state.scope)))) || - binding.kind === 'frozen_state' || - binding.kind === 'derived') && - context.state.function_depth === binding.scope.function_depth - ) { - warn(context.state.analysis.warnings, node, context.path, 'static-state-reference'); + if ( + node !== binding.node && + // If we have $state that can be proxied or frozen and isn't re-assigned, then that means + // it's likely not using a primitive value and thus this warning isn't that helpful. + ((binding.kind === 'state' && + (binding.reassigned || + (binding.initial?.type === 'CallExpression' && + binding.initial.arguments.length === 1 && + binding.initial.arguments[0].type !== 'SpreadElement' && + !should_proxy_or_freeze(binding.initial.arguments[0], context.state.scope)))) || + binding.kind === 'frozen_state' || + binding.kind === 'derived') && + context.state.function_depth === binding.scope.function_depth + ) { + warn(context.state.analysis.warnings, node, context.path, 'static-state-reference'); + } } } }, diff --git a/packages/svelte/tests/runtime-runes/samples/date/_config.js b/packages/svelte/tests/runtime-runes/samples/date/_config.js index 8bd2327683..e06d99e6f1 100644 --- a/packages/svelte/tests/runtime-runes/samples/date/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/date/_config.js @@ -1,8 +1,24 @@ import { flushSync } from '../../../../src/main/main-client'; import { test } from '../../test'; +const date_proto = Date.prototype; +let date_proto_to_string = date_proto.toString; + export default test({ - html: `
getSeconds: 0
getMinutes: 0
getHours: 15
getTime: 1708700400000
toDateString: Fri Feb 23 2024
`, + html: `
getSeconds: 0
getMinutes: 0
getHours: 15
getTime: 1708700400000
toDateString: Fri Feb 23 2024
date: [date: 0, 0, 15]
`, + + before_test() { + date_proto_to_string = date_proto.toString; + + // This test will fail between different machines because of timezones, so we instead mock it to be a different toString(). + date_proto.toString = function () { + return `[date: ${this.getSeconds()}, ${this.getMinutes()}, ${this.getHours()}]`; + }; + }, + + after_test() { + date_proto.toString = date_proto_to_string; + }, test({ assert, target }) { const [btn, btn2, btn3] = target.querySelectorAll('button'); @@ -13,7 +29,7 @@ export default test({ assert.htmlEqual( target.innerHTML, - `
getSeconds: 1
getMinutes: 0
getHours: 15
getTime: 1708700401000
toDateString: Fri Feb 23 2024
` + `
getSeconds: 1
getMinutes: 0
getHours: 15
getTime: 1708700401000
toDateString: Fri Feb 23 2024
date: [date: 1, 0, 15]
` ); flushSync(() => { @@ -22,7 +38,7 @@ export default test({ assert.htmlEqual( target.innerHTML, - `
getSeconds: 1
getMinutes: 1
getHours: 15
getTime: 1708700461000
toDateString: Fri Feb 23 2024
` + `
getSeconds: 1
getMinutes: 1
getHours: 15
getTime: 1708700461000
toDateString: Fri Feb 23 2024
date: [date: 1, 1, 15]
` ); flushSync(() => { @@ -31,7 +47,7 @@ export default test({ assert.htmlEqual( target.innerHTML, - `
getSeconds: 1
getMinutes: 1
getHours: 16
getTime: 1708704061000
toDateString: Fri Feb 23 2024
` + `
getSeconds: 1
getMinutes: 1
getHours: 16
getTime: 1708704061000
toDateString: Fri Feb 23 2024
date: [date: 1, 1, 16]
` ); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/date/main.svelte b/packages/svelte/tests/runtime-runes/samples/date/main.svelte index 10083615e0..5eb86dca7d 100644 --- a/packages/svelte/tests/runtime-runes/samples/date/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/date/main.svelte @@ -9,6 +9,7 @@
getHours: {date.getUTCHours()}
getTime: {date.getTime()}
toDateString: {date.toDateString()}
+
date: {date}