From ae4317de9c5b8c2aa52fc86f61875044a27a4615 Mon Sep 17 00:00:00 2001 From: Nic <162764842+Nic-Polumeyv@users.noreply.github.com> Date: Fri, 24 Jul 2026 20:08:21 -0400 Subject: [PATCH] fix: compute store subscription blockers before function tracing, defer server store teardown past async work --- .../src/compiler/phases/2-analyze/index.js | 24 ++++++++++++----- .../3-transform/server/transform-server.js | 26 ++++++++++++++----- .../async-store-sub-blocker/_config.js | 7 ++--- .../async-store-sub-blocker/main.svelte | 5 ++++ .../async-store-sub-teardown/_config.js | 18 +++++++++++++ .../async-store-sub-teardown/main.svelte | 11 ++++++++ .../samples/async-store-sub-teardown/store.js | 23 ++++++++++++++++ 7 files changed, 97 insertions(+), 17 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/store.js diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 2044ac562d..350ef89b93 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -1227,6 +1227,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} */ const reads_writes = new Set(); @@ -1265,13 +1282,6 @@ function calculate_blockers(instance, analysis) { binding.blocker = /** @type {typeof binding['blocker']} */ (blocker); } - - // a store subscription must wait on whatever blocks the store itself - for (const [name, binding] of instance.scope.declarations) { - if (binding.kind === 'store_sub') { - binding.blocker ??= instance.scope.get(name.slice(1))?.blocker ?? null; - } - } } /** diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 24ca58bb41..62940493cf 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/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,8 @@ export function server_component(analysis, options) { ); } - let should_inject_context = dev || analysis.needs_context; + // `on_destroy` callbacks are only collected inside a component body + let should_inject_context = dev || analysis.needs_context || defer_store_teardown; if (should_inject_context) { component_block = b.block([ diff --git a/packages/svelte/tests/runtime-runes/samples/async-store-sub-blocker/_config.js b/packages/svelte/tests/runtime-runes/samples/async-store-sub-blocker/_config.js index e120a4bed5..896385e609 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-store-sub-blocker/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-store-sub-blocker/_config.js @@ -2,12 +2,13 @@ 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`. +// 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: '

hello

', + ssrHtml: '

hello

hello

', async test({ assert, target }) { await tick(); - assert.htmlEqual(target.innerHTML, '

hello

'); + assert.htmlEqual(target.innerHTML, '

hello

hello

'); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/async-store-sub-blocker/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-store-sub-blocker/main.svelte index a385400a8f..04c707c66d 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-store-sub-blocker/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-store-sub-blocker/main.svelte @@ -6,6 +6,11 @@ } const store = await get_store(); + + function read() { + return $store; + }

{$store}

+

{read()}

diff --git a/packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/_config.js b/packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/_config.js new file mode 100644 index 0000000000..ac60694648 --- /dev/null +++ b/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: '

hello

', + + test_ssr({ assert }) { + assert.deepEqual(counts, { subscribes: 1, unsubscribes: 1 }); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/main.svelte new file mode 100644 index 0000000000..8d17e701d3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/main.svelte @@ -0,0 +1,11 @@ + + +

{$s}

diff --git a/packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/store.js b/packages/svelte/tests/runtime-runes/samples/async-store-sub-teardown/store.js new file mode 100644 index 0000000000..4922c1b027 --- /dev/null +++ b/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; +}