From 3c97c0a1a1738e8d515e0fa4f0d2a8eb41e3d1e8 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 16 Sep 2024 18:49:51 +0100 Subject: [PATCH] fix: ensure function calls to identifiers are made reactive (#13264) * fix: ensure function calls to identifiers are made reactive * update test * we have has_local at home * add note * make it a TODO * Update .changeset/red-ladybugs-turn.md --------- Co-authored-by: Rich Harris --- .changeset/red-ladybugs-turn.md | 5 +++++ .../phases/2-analyze/visitors/CallExpression.js | 16 +++++++++++----- .../samples/script-context-module/output.svelte | 2 +- .../state-imported-function/Data.svelte.js | 1 + .../samples/state-imported-function/_config.js | 11 +++++++++++ .../samples/state-imported-function/main.svelte | 11 +++++++++++ .../_expected/server/index.svelte.js | 2 +- .../purity/_expected/client/index.svelte.js | 8 ++------ .../purity/_expected/server/index.svelte.js | 9 ++------- .../tests/snapshot/samples/purity/index.svelte | 12 ++---------- 10 files changed, 47 insertions(+), 30 deletions(-) create mode 100644 .changeset/red-ladybugs-turn.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/state-imported-function/Data.svelte.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/state-imported-function/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/state-imported-function/main.svelte diff --git a/.changeset/red-ladybugs-turn.md b/.changeset/red-ladybugs-turn.md new file mode 100644 index 0000000000..b647c442a5 --- /dev/null +++ b/.changeset/red-ladybugs-turn.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: treat pure call expressions as potentially reactive if they reference local bindings diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js index 0f0555d787..957b27ae9b 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -143,11 +143,6 @@ export function CallExpression(node, context) { break; } - if (context.state.expression && !is_pure(node.callee, context)) { - context.state.expression.has_call = true; - context.state.expression.has_state = true; - } - if (context.state.render_tag) { // Find out which of the render tag arguments contains this call expression const arg_idx = unwrap_optional(context.state.render_tag.expression).arguments.findIndex( @@ -174,4 +169,15 @@ export function CallExpression(node, context) { } else { context.next(); } + + if (context.state.expression) { + // TODO We assume that any dependencies are stateful, which isn't necessarily the case — see + // https://github.com/sveltejs/svelte/issues/13266. This check also includes dependencies + // outside the call expression itself (e.g. `{blah && pure()}`) resulting in additional + // false positives, but for now we accept that trade-off + if (!is_pure(node.callee, context) || context.state.expression.dependencies.size > 0) { + context.state.expression.has_call = true; + context.state.expression.has_state = true; + } + } } diff --git a/packages/svelte/tests/migrate/samples/script-context-module/output.svelte b/packages/svelte/tests/migrate/samples/script-context-module/output.svelte index e508049d5d..065e3ac651 100644 --- a/packages/svelte/tests/migrate/samples/script-context-module/output.svelte +++ b/packages/svelte/tests/migrate/samples/script-context-module/output.svelte @@ -1,3 +1,3 @@ + \ No newline at end of file diff --git a/packages/svelte/tests/runtime-legacy/samples/state-imported-function/Data.svelte.js b/packages/svelte/tests/runtime-legacy/samples/state-imported-function/Data.svelte.js new file mode 100644 index 0000000000..8c8c1527be --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/state-imported-function/Data.svelte.js @@ -0,0 +1 @@ +export let obj = $state({ a: 1, b: 2 }); diff --git a/packages/svelte/tests/runtime-legacy/samples/state-imported-function/_config.js b/packages/svelte/tests/runtime-legacy/samples/state-imported-function/_config.js new file mode 100644 index 0000000000..0c806438d5 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/state-imported-function/_config.js @@ -0,0 +1,11 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [b1] = target.querySelectorAll('button'); + b1.click(); + flushSync(); + assert.htmlEqual(target.innerHTML, `\n9,10,11`); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/state-imported-function/main.svelte b/packages/svelte/tests/runtime-legacy/samples/state-imported-function/main.svelte new file mode 100644 index 0000000000..6bb2acd778 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/state-imported-function/main.svelte @@ -0,0 +1,11 @@ + + + + +{Object.values(obj)} diff --git a/packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/server/index.svelte.js index 6a9027e41f..9dad39d81e 100644 --- a/packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/server/index.svelte.js @@ -7,4 +7,4 @@ const __DECLARED_ASSET_3__ = "__VITE_ASSET__2AM7_y_g__"; export default function Inline_module_vars($$payload) { $$payload.out += ` `; -} +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/purity/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/purity/_expected/client/index.svelte.js index e054bd0c4a..305e9aa0d7 100644 --- a/packages/svelte/tests/snapshot/samples/purity/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/purity/_expected/client/index.svelte.js @@ -4,14 +4,10 @@ import * as $ from "svelte/internal/client"; var root = $.template(`

`, 1); export default function Purity($$anchor) { - let min = 0; - let max = 100; - let number = 50; - let value = 'hello'; var fragment = root(); var p = $.first_child(fragment); - p.textContent = Math.max(min, Math.min(max, number)); + p.textContent = Math.max(0, Math.min(0, 100)); var p_1 = $.sibling(p, 2); @@ -19,6 +15,6 @@ export default function Purity($$anchor) { var node = $.sibling(p_1, 2); - Child(node, { prop: encodeURIComponent(value) }); + Child(node, { prop: encodeURIComponent('hello') }); $.append($$anchor, fragment); } \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/purity/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/purity/_expected/server/index.svelte.js index 3528e8d1b4..1eea71e4dd 100644 --- a/packages/svelte/tests/snapshot/samples/purity/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/purity/_expected/server/index.svelte.js @@ -1,12 +1,7 @@ import * as $ from "svelte/internal/server"; export default function Purity($$payload) { - let min = 0; - let max = 100; - let number = 50; - let value = 'hello'; - - $$payload.out += `

${$.escape(Math.max(min, Math.min(max, number)))}

${$.escape(location.href)}

`; - Child($$payload, { prop: encodeURIComponent(value) }); + $$payload.out += `

${$.escape(Math.max(0, Math.min(0, 100)))}

${$.escape(location.href)}

`; + Child($$payload, { prop: encodeURIComponent('hello') }); $$payload.out += ``; } \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/purity/index.svelte b/packages/svelte/tests/snapshot/samples/purity/index.svelte index 79db5dc7b1..05efe1802d 100644 --- a/packages/svelte/tests/snapshot/samples/purity/index.svelte +++ b/packages/svelte/tests/snapshot/samples/purity/index.svelte @@ -1,12 +1,4 @@ - - -

{Math.max(min, Math.min(max, number))}

+

{Math.max(0, Math.min(0, 100))}

{location.href}

- +