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 <[email protected]>

Author: trueadm

.changeset/red-ladybugs-turn.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: treat pure call expressions as potentially reactive if they reference local bindings

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;
+		}
+	}
 }

packages/svelte/tests/migrate/samples/script-context-module/output.svelte

@@ -1,3 +1,3 @@
 <script module>
 	let foo = true;
-</script>
+</script>

packages/svelte/tests/runtime-legacy/samples/state-imported-function/Data.svelte.js

@@ -0,0 +1 @@
+export let obj = $state({ a: 1, b: 2 });

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, `<button>Replace</button>\n9,10,11`);
+	}
+});

packages/svelte/tests/runtime-legacy/samples/state-imported-function/main.svelte

@@ -0,0 +1,11 @@
+<script>
+	import { obj } from "./Data.svelte.js";
+
+	function replaceProp() {
+		Object.assign(obj, {a:9, b:10, c:11});
+	}
+</script>
+
+<button onclick={replaceProp}>Replace</button>
+
+{Object.values(obj)}

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 += `<picture><source${$.attr("srcset", __DECLARED_ASSET_0__)} type="image/avif"> <source${$.attr("srcset", __DECLARED_ASSET_1__)} type="image/webp"> <source${$.attr("srcset", __DECLARED_ASSET_2__)} type="image/png"> <img${$.attr("src", __DECLARED_ASSET_3__)} alt="production test" width="1440" height="1440"></picture>`;
-}
+}

packages/svelte/tests/snapshot/samples/purity/_expected/client/index.svelte.js

@@ -4,21 +4,17 @@ import * as $ from "svelte/internal/client";
 var root = $.template(`<p></p> <p></p> <!>`, 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);
 
 	p_1.textContent = location.href;
 
 	var node = $.sibling(p_1, 2);
 
-	Child(node, { prop: encodeURIComponent(value) });
+	Child(node, { prop: encodeURIComponent('hello') });
 	$.append($$anchor, fragment);
 }

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 += `<p>${$.escape(Math.max(min, Math.min(max, number)))}</p> <p>${$.escape(location.href)}</p> `;
-	Child($$payload, { prop: encodeURIComponent(value) });
+	$$payload.out += `<p>${$.escape(Math.max(0, Math.min(0, 100)))}</p> <p>${$.escape(location.href)}</p> `;
+	Child($$payload, { prop: encodeURIComponent('hello') });
 	$$payload.out += `<!---->`;
 }

packages/svelte/tests/snapshot/samples/purity/index.svelte

@@ -1,12 +1,4 @@
-<script>
-	let min = 0;
-	let max = 100;
-	let number = 50;
-
-	let value = 'hello';
-</script>
-
-<p>{Math.max(min, Math.min(max, number))}</p>
+<p>{Math.max(0, Math.min(0, 100))}</p>
 <p>{location.href}</p>
 
-<Child prop={encodeURIComponent(value)} />
+<Child prop={encodeURIComponent('hello')} />