fix: make template store subscriptions wait for the promise that assigns the store (#18582)

Blockers didn't include analyzing implicit store subscriptions, which could also only happen in the template.

Also needs to defer store unsubscribe until after the async template has settled in case the store value is read after an async blocker, in which case unsubscribe synchronously is too soon.

Fixes https://github.com/sveltejs/kit/issues/15119

.changeset/async-store-sub-blocker.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: block template store subscriptions on the promise that assigns the store

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

@@ -1228,6 +1228,23 @@ function calculate_blockers(instance, analysis) {
 
 	flush_sync_group();
 
+	// a store subscription must wait on whatever blocks the store itself; this must happen
+	// before function tracing so that functions reading `$store` inherit the blocker
+	for (const [name, binding] of instance.scope.declarations) {
+		if (binding.kind !== 'store_sub') continue;
+
+		const store_blocker = instance.scope.get(name.slice(1))?.blocker;
+		if (!store_blocker) continue;
+
+		if (
+			!binding.blocker ||
+			/** @type {ESTree.SimpleLiteral & { value: number }} */ (binding.blocker.property).value <
+				/** @type {ESTree.SimpleLiteral & { value: number }} */ (store_blocker.property).value
+		) {
+			binding.blocker = store_blocker;
+		}
+	}
+
 	for (const fn of functions) {
 		/** @type {Set<Binding>} */
 		const reads_writes = new Set();

packages/svelte/src/compiler/phases/3-transform/server/transform-server.js

@@ -210,14 +210,25 @@ export function server_component(analysis, options) {
 		];
 	}
 
-	if (
-		[...analysis.instance.scope.declarations.values()].some(
-			(binding) => binding.kind === 'store_sub'
-		)
-	) {
+	const store_subs = [...analysis.instance.scope.declarations.values()].filter(
+		(binding) => binding.kind === 'store_sub'
+	);
+
+	// a blocked subscription is only created once its promise resolves, so its teardown must wait until the render is done
+	const defer_store_teardown = store_subs.some((binding) => binding.blocker);
+
+	if (store_subs.length > 0) {
 		instance.body.unshift(b.var('$$store_subs'));
+
+		const unsubscribe = b.if(
+			b.id('$$store_subs'),
+			b.stmt(b.call('$.unsubscribe_stores', b.id('$$store_subs')))
+		);
+
 		template.body.push(
-			b.if(b.id('$$store_subs'), b.stmt(b.call('$.unsubscribe_stores', b.id('$$store_subs'))))
+			defer_store_teardown
+				? b.stmt(b.call('$$renderer.on_destroy', b.arrow([], b.block([unsubscribe]))))
+				: unsubscribe
 		);
 	}
 
@@ -257,7 +268,7 @@ export function server_component(analysis, options) {
 		);
 	}
 
-	let should_inject_context = dev || analysis.needs_context;
+	let should_inject_context = dev || analysis.needs_context || defer_store_teardown;
 
 	if (should_inject_context) {
 		component_block = b.block([

packages/svelte/tests/runtime-runes/samples/async-store-sub-blocker/_config.js

@@ -0,0 +1,14 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+// Tests that a store subscription only present in the template waits for the
+// promise that assigns the store instead of subscribing to `undefined`,
+// including when the subscription is read through a function.
+export default test({
+	mode: ['client', 'hydrate', 'async-server'],
+	ssrHtml: '<p>hello</p> <p>hello</p>',
+	async test({ assert, target }) {
+		await tick();
+		assert.htmlEqual(target.innerHTML, '<p>hello</p> <p>hello</p>');
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-store-sub-blocker/main.svelte

@@ -0,0 +1,16 @@
+<script>
+	import { writable } from 'svelte/store';
+
+	async function get_store() {
+		return writable('hello');
+	}
+
+	const store = await get_store();
+
+	function read() {
+		return $store;
+	}
+</script>
+
+<p>{$store}</p>
+<p>{read()}</p>

packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/_config.js

@@ -0,0 +1,18 @@
+import { test } from '../../test';
+import { counts, reset } from './store.js';
+
+// A blocked store subscription is created after the synchronous part of the
+// render has finished, so the teardown must wait for the async work.
+export default test({
+	mode: ['async-server'],
+
+	before_test() {
+		reset();
+	},
+
+	ssrHtml: '<p>hello</p>',
+
+	test_ssr({ assert }) {
+		assert.deepEqual(counts, { subscribes: 1, unsubscribes: 1 });
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/main.svelte

@@ -0,0 +1,11 @@
+<script>
+	import { store } from './store.js';
+
+	async function get_store() {
+		return store;
+	}
+
+	const s = await get_store();
+</script>
+
+<p>{$s}</p>

packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/store.js

@@ -0,0 +1,23 @@
+import { writable } from 'svelte/store';
+
+export const counts = { subscribes: 0, unsubscribes: 0 };
+
+const inner = writable('hello');
+
+export const store = {
+	/** @param {(value: string) => void} fn */
+	subscribe(fn) {
+		counts.subscribes += 1;
+		const unsubscribe = inner.subscribe(fn);
+
+		return () => {
+			counts.unsubscribes += 1;
+			unsubscribe();
+		};
+	}
+};
+
+export function reset() {
+	counts.subscribes = 0;
+	counts.unsubscribes = 0;
+}