fix: refetch queries after a redirect when the cached query survives the navigation

.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

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',

packages/kit/package.json

@@ -67,6 +67,7 @@
 	"files": [
 		"src",
 		"!src/**/*.spec.js",
+		"!src/**/*.spec.svelte",
 		"!src/core/**/fixtures",
 		"!src/core/**/test",
 		"types",

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<any>, 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();
+	});
+});

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<T>} */ (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];
 	}

packages/kit/src/runtime/client/remote-functions/query/reset-race-harness.spec.svelte

@@ -0,0 +1,23 @@
+<script>
+	let { layout_query, page_query, page } = $props();
+</script>
+
+<svelte:boundary>
+	{#snippet pending()}
+		<p data-testid="layout-pending">layout pending</p>
+	{/snippet}
+
+	{@const user = await layout_query}
+	<p data-testid="layout">{user}</p>
+</svelte:boundary>
+
+{#if page.show}
+	<svelte:boundary>
+		{#snippet pending()}
+			<p data-testid="page-pending">page pending</p>
+		{/snippet}
+
+		{@const items = await page_query}
+		<p data-testid="page">{items}</p>
+	</svelte:boundary>
+{/if}