fix: robustify blocker calculation (#17676)

This was actually several bugs:
- We used `scopes` for the blockers, that's actually the template
scopes, should be `instance.scopes` instead
- We missed setting the scope for `touch`
- We didn't take return statements into account when calculating
blockers. We cannot know when/if something within the return statement
is called, so we gotta assume it is and touch everything transitively
from it

Combined this fixes #17667 (and possibly other cases not showing up in
the issue tracker yet)

Initially I just thought "ok I guess we have to traverse into functions,
too" but then I thought that feels too unoptimized and came up with the
return-statement-inspection, at which point I discovered the other bugs.

### Before submitting the PR, please make sure you do the following

- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`.
- [x] This message body should clearly illustrate what problems it
solves.
- [x] Ideally, include a test that fails without this PR but passes with
it.
- [x] If this PR changes code within `packages/svelte/src`, add a
changeset (`npx changeset`).

### Tests and linting

- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint`

.changeset/true-cities-retire.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: robustify blocker calculation

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

@@ -690,7 +690,7 @@ export function analyze_component(root, source, options) {
 		}
 	}
 
-	calculate_blockers(instance, scopes, analysis);
+	calculate_blockers(instance, analysis);
 
 	if (analysis.runes) {
 		const props_refs = module.scope.references.get('$$props');
@@ -940,11 +940,10 @@ export function analyze_component(root, source, options) {
  * top level statements. This includes indirect blockers such as functions referencing async top level statements.
  *
  * @param {Js} instance
- * @param {Map<AST.SvelteNode, Scope>} scopes
  * @param {ComponentAnalysis} analysis
  * @returns {void}
  */
-function calculate_blockers(instance, scopes, analysis) {
+function calculate_blockers(instance, analysis) {
 	/**
 	 * @param {ESTree.Node} expression
 	 * @param {Scope} scope
@@ -959,6 +958,14 @@ function calculate_blockers(instance, scopes, analysis) {
 			expression,
 			{ scope },
 			{
+				_(node, context) {
+					const scope = instance.scopes.get(node);
+					if (scope) {
+						context.next({ scope });
+					} else {
+						context.next();
+					}
+				},
 				ImportDeclaration(node) {},
 				Identifier(node, context) {
 					const parent = /** @type {ESTree.Node} */ (context.path.at(-1));
@@ -979,14 +986,11 @@ function calculate_blockers(instance, scopes, analysis) {
 
 	/**
 	 * @param {ESTree.Node} node
-	 * @param {Set<ESTree.Node>} seen
 	 * @param {Set<Binding>} reads
 	 * @param {Set<Binding>} writes
+	 * @param {Scope} scope
 	 */
-	const trace_references = (node, reads, writes, seen = new Set()) => {
+	const trace_references = (node, reads, writes, scope) => {
-		if (seen.has(node)) return;
-		seen.add(node);
-
 		/**
 		 * @param {ESTree.Pattern} node
 		 * @param {Scope} scope
@@ -1005,10 +1009,10 @@ function calculate_blockers(instance, scopes, analysis) {
 
 		walk(
 			node,
-			{ scope: instance.scope },
+			{ scope },
 			{
 				_(node, context) {
-					const scope = scopes.get(node);
+					const scope = instance.scopes.get(node);
 					if (scope) {
 						context.next({ scope });
 					} else {
@@ -1040,10 +1044,6 @@ function calculate_blockers(instance, scopes, analysis) {
 						writes.add(b);
 					}
 				},
-				// don't look inside functions until they are called
-				ArrowFunctionExpression(_, context) {},
-				FunctionDeclaration(_, context) {},
-				FunctionExpression(_, context) {},
 				Identifier(node, context) {
 					const parent = /** @type {ESTree.Node} */ (context.path.at(-1));
 					if (is_reference(node, parent)) {
@@ -1052,7 +1052,19 @@ function calculate_blockers(instance, scopes, analysis) {
 							reads.add(binding);
 						}
 					}
-				}
+				},
+				ReturnStatement(node, context) {
+					// We have to assume that anything returned from a function, even if it's a function itself,
+					// might be called immediately, so we have to touch all references within it. Example:
+					// function foo() { return () => blocker; } foo(); // blocker is touched
+					if (node.argument) {
+						touch(node.argument, context.state.scope, reads);
+					}
+				},
+				// don't look inside functions until they are called
+				ArrowFunctionExpression(_, context) {},
+				FunctionDeclaration(_, context) {},
+				FunctionExpression(_, context) {}
 			}
 		);
 	};
@@ -1132,7 +1144,7 @@ function calculate_blockers(instance, scopes, analysis) {
 					/** @type {Set<Binding>} */
 					const writes = new Set();
 
-					trace_references(declarator, reads, writes);
+					trace_references(declarator, reads, writes, instance.scope);
 
 					const blocker = /** @type {NonNullable<Binding['blocker']>} */ (
 						b.member(promises, b.literal(analysis.instance_body.async.length), true)
@@ -1160,7 +1172,7 @@ function calculate_blockers(instance, scopes, analysis) {
 			/** @type {Set<Binding>} */
 			const writes = new Set();
 
-			trace_references(node, reads, writes);
+			trace_references(node, reads, writes, instance.scope);
 
 			const blocker = /** @type {NonNullable<Binding['blocker']>} */ (
 				b.member(promises, b.literal(analysis.instance_body.async.length), true)
@@ -1184,12 +1196,17 @@ function calculate_blockers(instance, scopes, analysis) {
 	for (const fn of functions) {
 		/** @type {Set<Binding>} */
 		const reads_writes = new Set();
-		const body =
+		const init =
 			fn.type === 'VariableDeclarator'
-				? /** @type {ESTree.FunctionExpression | ESTree.ArrowFunctionExpression} */ (fn.init).body
+				? /** @type {ESTree.FunctionExpression | ESTree.ArrowFunctionExpression} */ (fn.init)
-				: fn.body;
+				: fn;
-
+
-		trace_references(body, reads_writes, reads_writes);
+		trace_references(
+			init.body,
+			reads_writes,
+			reads_writes,
+			/** @type {Scope} */ (instance.scopes.get(init))
+		);
 
 		const max = [...reads_writes].reduce((max, binding) => {
 			if (binding.blocker) {

packages/svelte/tests/runtime-runes/samples/async-bind-factory-function-remote/_config.js

@@ -0,0 +1,14 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	mode: ['async-server', 'client', 'hydrate'],
+	ssrHtml: 'true true true true true',
+
+	async test({ assert, target }) {
+		await new Promise((resolve) => setTimeout(resolve, 10));
+		await tick();
+
+		assert.htmlEqual(target.innerHTML, 'true true true true true');
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-bind-factory-function-remote/main.svelte

@@ -0,0 +1,41 @@
+<script>
+	let checked = $derived(await new Promise((r) => setTimeout(() => r(true)), 10));
+
+	const checkedFactory = () => {
+		return () => checked;
+	}
+
+	function indirectCheckedFactory() {
+		return checkedFactory();
+	}
+
+	function callFactory(factory) {
+		return factory();
+	}
+
+	function indirectCallFactory() {
+		return callFactory(indirectCheckedFactory);
+	}
+
+	function indirectChecked2() {
+		const indirect = () => checkedFactory()();
+		return indirect;
+	}
+</script>
+
+<!-- force into separate effects -->
+{#if true}
+	{checkedFactory()()}
+{/if}
+{#if true}
+	{indirectCheckedFactory()()}
+{/if}
+{#if true}
+	{callFactory(checkedFactory)()}
+{/if}
+{#if true}
+	{indirectCallFactory()()}
+{/if}
+{#if true}
+	{indirectChecked2()()}
+{/if}