From 93ee3d183b946a0b67a4e11cf4e791860149950e Mon Sep 17 00:00:00 2001 From: Nic Polumeyv <162764842+Nic-Polumeyv@users.noreply.github.com> Date: Mon, 20 Jul 2026 21:36:05 -0400 Subject: [PATCH 1/3] fix: refetch queries after a redirect when the cached query survives the navigation --- .changeset/spotty-forks-refresh.md | 5 + packages/kit/kit.vitest.config.js | 2 + packages/kit/package.json | 1 + .../query/instance.reset.svelte.spec.js | 118 ++++++++++++++++++ .../remote-functions/query/instance.svelte.js | 16 ++- .../query/reset-race-harness.spec.svelte | 23 ++++ 6 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 .changeset/spotty-forks-refresh.md create mode 100644 packages/kit/src/runtime/client/remote-functions/query/instance.reset.svelte.spec.js create mode 100644 packages/kit/src/runtime/client/remote-functions/query/reset-race-harness.spec.svelte diff --git a/.changeset/spotty-forks-refresh.md b/.changeset/spotty-forks-refresh.md new file mode 100644 index 000000000000..ce317e860b23 --- /dev/null +++ b/.changeset/spotty-forks-refresh.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: refetch queries after a redirect when the cached query survives the navigation diff --git a/packages/kit/kit.vitest.config.js b/packages/kit/kit.vitest.config.js index e006f20ee7f4..f7ee20fbd99b 100644 --- a/packages/kit/kit.vitest.config.js +++ b/packages/kit/kit.vitest.config.js @@ -46,6 +46,8 @@ export default defineConfig({ }, { extends: true, + // resolve the browser build of `svelte` so specs can `mount` components + resolve: { conditions: ['browser'] }, test: { name: 'client', environment: 'jsdom', diff --git a/packages/kit/package.json b/packages/kit/package.json index ae35707ce5b1..e8e1c39b2034 100644 --- a/packages/kit/package.json +++ b/packages/kit/package.json @@ -67,6 +67,7 @@ "files": [ "src", "!src/**/*.spec.js", + "!src/**/*.spec.svelte", "!src/core/**/fixtures", "!src/core/**/test", "types", diff --git a/packages/kit/src/runtime/client/remote-functions/query/instance.reset.svelte.spec.js b/packages/kit/src/runtime/client/remote-functions/query/instance.reset.svelte.spec.js new file mode 100644 index 000000000000..e0fe7d58ae6b --- /dev/null +++ b/packages/kit/src/runtime/client/remote-functions/query/instance.reset.svelte.spec.js @@ -0,0 +1,118 @@ +import { describe, expect, test, vi } from 'vitest'; +import { mount, unmount, tick } from 'svelte'; + +// Mock `client.js` because the real one pulls in the SvelteKit +// router/hydration machinery and resolves `$app/paths` to a server-side +// virtual module that only exists during a real SvelteKit build. +vi.mock(new URL('../../client.js', import.meta.url).pathname, () => ({ + app: { hooks: { transport: {} }, decoders: {} }, + query_map: new Map(), + query_responses: {}, + live_query_map: new Map(), + goto: () => {} +})); + +const { Query } = await import('./instance.svelte.js'); +const { default: Harness } = await import('./reset-race-harness.spec.svelte'); + +/** + * @param {() => boolean} predicate + * @param {string} label + */ +async function wait_for(predicate, label) { + for (let i = 0; i < 50; i++) { + if (predicate()) return; + await tick(); + await new Promise((resolve) => setTimeout(resolve, 0)); + } + throw new Error(`Timed out waiting for ${label}`); +} + +function deferred() { + /** @type {(value: any) => void} */ + let resolve = () => {}; + const promise = new Promise((r) => (resolve = r)); + return { promise, resolve }; +} + +describe('Query.reset', () => { + // Regression test for #16444: a remote form redirect navigation resets all + // cached queries, but the destination page's query is re-rendered from a + // different batch than the one holding the reset. While the reset batch is + // still pending (the persistent layout's re-fired `await` keeps it so), + // Svelte's time-travel overlay hands the render the pre-reset promise, and + // the lazy init in `#get_promise` used to write that stale promise back + // permanently, so the destination rendered stale data and never refetched. + test('a reset survives a render that evaluates while the resetting batch is pending', async () => { + const target = document.createElement('div'); + document.body.appendChild(target); + + let user_calls = 0; + /** @type {{ promise: Promise, resolve: (value: any) => void } | null} */ + let user_gate = null; + const layout_query = new Query('user/', () => { + user_calls += 1; + if (user_gate) { + const gate = user_gate; + user_gate = null; + return gate.promise; + } + return Promise.resolve(`user-${user_calls}`); + }); + + let items_calls = 0; + const page_query = new Query('items/', () => { + items_calls += 1; + return Promise.resolve(`items-${items_calls}`); + }); + + const page = $state({ show: true }); + const app = mount(Harness, { target, props: { layout_query, page_query, page } }); + + const text = (/** @type {string} */ id) => + target.querySelector(`[data-testid="${id}"]`)?.textContent; + + await wait_for( + () => text('page') === 'items-1' && text('layout') === 'user-1', + 'initial render' + ); + + // navigate away from the page. The spec holds `page_query`, so the cached + // instance survives, as it does in a real app whenever the proxies have + // not been garbage collected yet + page.show = false; + await tick(); + expect(text('page')).toBe(undefined); + + // the next layout fetch hangs until we release it, which keeps the batch + // created below pending across the page's re-render + const gate = deferred(); + user_gate = gate; + + // redirect lands: reset all queries, as `_goto` does in its `accept` + // callback. Resetting `layout_query` re-fires the layout's `await`, so + // this batch stays pending on `user_gate` + layout_query.reset(); + page_query.reset(); + + // give the reset batch a chance to start processing, then re-render the + // destination page from a separate batch, as a real navigation does + await Promise.resolve(); + page.show = true; + + // release the layout fetch so everything can settle + await Promise.resolve(); + gate.resolve('user-2'); + + await wait_for(() => text('page') === 'items-2', 'page to show refetched data'); + await wait_for(() => text('layout') === 'user-2', 'layout to show refetched data'); + + // exactly one refetch each: the reset must trigger a new fetch, but not + // a duplicate one + expect(items_calls).toBe(2); + expect(user_calls).toBe(2); + + await unmount(app); + target.remove(); + }); +}); diff --git a/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js b/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js index af9520beab12..9b259f3992d9 100644 --- a/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js @@ -30,6 +30,11 @@ export class Query { /** @type {Array<(old: T) => T>} */ #overrides = $state([]); + // Plain (non-reactive) fields: batch/fork snapshotting must never + // hide a reset from #get_promise + #reset_generation = 0; + #promise_generation = 0; + /** @type {T | undefined} */ #current = $derived.by(() => { // don't reduce undefined value @@ -79,7 +84,12 @@ export class Query { } #get_promise() { - void untrack(() => (this.#promise ??= this.#run())); + void untrack(() => { + if (this.#promise === null || this.#promise_generation !== this.#reset_generation) { + this.#promise_generation = this.#reset_generation; + this.#promise = this.#run(); + } + }); return /** @type {Promise} */ (this.#promise); } @@ -211,6 +221,7 @@ export class Query { */ refresh() { delete query_responses[this.#key]; + this.#promise_generation = this.#reset_generation; return (this.#promise = this.#run()); } @@ -227,6 +238,7 @@ export class Query { this.#loading = false; this.#error = undefined; this.#raw = value; + this.#promise_generation = this.#reset_generation; this.#promise = Promise.resolve(); } @@ -245,6 +257,7 @@ export class Query { const promise = Promise.reject(error); promise.catch(noop); + this.#promise_generation = this.#reset_generation; this.#promise = promise; } @@ -275,6 +288,7 @@ export class Query { * rendered queries to get fresh data */ reset() { + this.#reset_generation += 1; this.#promise = null; delete query_responses[this.#key]; } diff --git a/packages/kit/src/runtime/client/remote-functions/query/reset-race-harness.spec.svelte b/packages/kit/src/runtime/client/remote-functions/query/reset-race-harness.spec.svelte new file mode 100644 index 000000000000..d6b64a9ab8b2 --- /dev/null +++ b/packages/kit/src/runtime/client/remote-functions/query/reset-race-harness.spec.svelte @@ -0,0 +1,23 @@ + + + + {#snippet pending()} +

layout pending

+ {/snippet} + + {@const user = await layout_query} +

{user}

+ + +{#if page.show} + + {#snippet pending()} +

page pending

+ {/snippet} + + {@const items = await page_query} +

{items}

+ +{/if} From eae18c297705c3d7fe565ce7c57b324c65512ad8 Mon Sep 17 00:00:00 2001 From: Nic Polumeyv <162764842+Nic-Polumeyv@users.noreply.github.com> Date: Mon, 20 Jul 2026 21:54:38 -0400 Subject: [PATCH 2/3] chore: collapse generation counters into a stale flag --- .../remote-functions/query/instance.svelte.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js b/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js index 9b259f3992d9..2c12eabbbd60 100644 --- a/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js @@ -30,10 +30,9 @@ export class Query { /** @type {Array<(old: T) => T>} */ #overrides = $state([]); - // Plain (non-reactive) fields: batch/fork snapshotting must never + // Plain (non-reactive) field: batch/fork snapshotting must never // hide a reset from #get_promise - #reset_generation = 0; - #promise_generation = 0; + #stale = false; /** @type {T | undefined} */ #current = $derived.by(() => { @@ -85,8 +84,7 @@ export class Query { #get_promise() { void untrack(() => { - if (this.#promise === null || this.#promise_generation !== this.#reset_generation) { - this.#promise_generation = this.#reset_generation; + if (this.#promise === null || this.#stale) { this.#promise = this.#run(); } }); @@ -109,6 +107,7 @@ export class Query { } #run() { + this.#stale = false; this.#loading = true; const { promise, resolve, reject } = with_resolvers(); @@ -221,7 +220,6 @@ export class Query { */ refresh() { delete query_responses[this.#key]; - this.#promise_generation = this.#reset_generation; return (this.#promise = this.#run()); } @@ -238,7 +236,7 @@ export class Query { this.#loading = false; this.#error = undefined; this.#raw = value; - this.#promise_generation = this.#reset_generation; + this.#stale = false; this.#promise = Promise.resolve(); } @@ -257,7 +255,7 @@ export class Query { const promise = Promise.reject(error); promise.catch(noop); - this.#promise_generation = this.#reset_generation; + this.#stale = false; this.#promise = promise; } @@ -288,7 +286,7 @@ export class Query { * rendered queries to get fresh data */ reset() { - this.#reset_generation += 1; + this.#stale = true; this.#promise = null; delete query_responses[this.#key]; } From fa645e9dd2dda34adda8e5efdd50b9a2ee61e019 Mon Sep 17 00:00:00 2001 From: Nic Polumeyv <162764842+Nic-Polumeyv@users.noreply.github.com> Date: Mon, 20 Jul 2026 22:53:02 -0400 Subject: [PATCH 3/3] chore: trim comments --- .../query/instance.reset.svelte.spec.js | 27 +++++-------------- .../remote-functions/query/instance.svelte.js | 3 +-- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/packages/kit/src/runtime/client/remote-functions/query/instance.reset.svelte.spec.js b/packages/kit/src/runtime/client/remote-functions/query/instance.reset.svelte.spec.js index e0fe7d58ae6b..e8718cbd8229 100644 --- a/packages/kit/src/runtime/client/remote-functions/query/instance.reset.svelte.spec.js +++ b/packages/kit/src/runtime/client/remote-functions/query/instance.reset.svelte.spec.js @@ -36,13 +36,8 @@ function deferred() { } describe('Query.reset', () => { - // Regression test for #16444: a remote form redirect navigation resets all - // cached queries, but the destination page's query is re-rendered from a - // different batch than the one holding the reset. While the reset batch is - // still pending (the persistent layout's re-fired `await` keeps it so), - // Svelte's time-travel overlay hands the render the pre-reset promise, and - // the lazy init in `#get_promise` used to write that stale promise back - // permanently, so the destination rendered stale data and never refetched. + // #16444: a render that evaluates while the resetting batch is still pending + // reads the pre-reset promise through Svelte's time-travel overlay test('a reset survives a render that evaluates while the resetting batch is pending', async () => { const target = document.createElement('div'); document.body.appendChild(target); @@ -77,38 +72,30 @@ describe('Query.reset', () => { 'initial render' ); - // navigate away from the page. The spec holds `page_query`, so the cached - // instance survives, as it does in a real app whenever the proxies have - // not been garbage collected yet + // the held `page_query` keeps the instance cached, like un-collected proxies do page.show = false; await tick(); expect(text('page')).toBe(undefined); - // the next layout fetch hangs until we release it, which keeps the batch - // created below pending across the page's re-render + // an unresolved layout fetch keeps the reset batch pending const gate = deferred(); user_gate = gate; - // redirect lands: reset all queries, as `_goto` does in its `accept` - // callback. Resetting `layout_query` re-fires the layout's `await`, so - // this batch stays pending on `user_gate` + // what `_goto` does in `accept` when a redirect lands layout_query.reset(); page_query.reset(); - // give the reset batch a chance to start processing, then re-render the - // destination page from a separate batch, as a real navigation does + // re-render the destination from a separate batch, as a real navigation does await Promise.resolve(); page.show = true; - // release the layout fetch so everything can settle await Promise.resolve(); gate.resolve('user-2'); await wait_for(() => text('page') === 'items-2', 'page to show refetched data'); await wait_for(() => text('layout') === 'user-2', 'layout to show refetched data'); - // exactly one refetch each: the reset must trigger a new fetch, but not - // a duplicate one + // a new fetch each, but not a duplicate one expect(items_calls).toBe(2); expect(user_calls).toBe(2); diff --git a/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js b/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js index 2c12eabbbd60..ad5f720131ac 100644 --- a/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/query/instance.svelte.js @@ -30,8 +30,7 @@ export class Query { /** @type {Array<(old: T) => T>} */ #overrides = $state([]); - // Plain (non-reactive) field: batch/fork snapshotting must never - // hide a reset from #get_promise + // plain (non-reactive) so a batch snapshot can never hide a reset from #get_promise #stale = false; /** @type {T | undefined} */