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..e8718cbd8229 --- /dev/null +++ b/packages/kit/src/runtime/client/remote-functions/query/instance.reset.svelte.spec.js @@ -0,0 +1,105 @@ +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', () => { + // #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); + + 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' + ); + + // the held `page_query` keeps the instance cached, like un-collected proxies do + page.show = false; + await tick(); + expect(text('page')).toBe(undefined); + + // an unresolved layout fetch keeps the reset batch pending + const gate = deferred(); + user_gate = gate; + + // what `_goto` does in `accept` when a redirect lands + layout_query.reset(); + page_query.reset(); + + // re-render the destination from a separate batch, as a real navigation does + await Promise.resolve(); + page.show = true; + + 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'); + + // a new fetch each, 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..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,6 +30,9 @@ export class Query { /** @type {Array<(old: T) => T>} */ #overrides = $state([]); + // plain (non-reactive) so a batch snapshot can never hide a reset from #get_promise + #stale = false; + /** @type {T | undefined} */ #current = $derived.by(() => { // don't reduce undefined value @@ -79,7 +82,11 @@ export class Query { } #get_promise() { - void untrack(() => (this.#promise ??= this.#run())); + void untrack(() => { + if (this.#promise === null || this.#stale) { + this.#promise = this.#run(); + } + }); return /** @type {Promise} */ (this.#promise); } @@ -99,6 +106,7 @@ export class Query { } #run() { + this.#stale = false; this.#loading = true; const { promise, resolve, reject } = with_resolvers(); @@ -227,6 +235,7 @@ export class Query { this.#loading = false; this.#error = undefined; this.#raw = value; + this.#stale = false; this.#promise = Promise.resolve(); } @@ -245,6 +254,7 @@ export class Query { const promise = Promise.reject(error); promise.catch(noop); + this.#stale = false; this.#promise = promise; } @@ -275,6 +285,7 @@ export class Query { * rendered queries to get fresh data */ reset() { + this.#stale = true; 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}