fix: don't fetch query when overridden but unused

.changeset/smart-badgers-yell.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: don't fetch query when overridden but unused

packages/kit/src/runtime/client/remote-functions/query.svelte.js

@@ -132,6 +132,8 @@ export class Query {
 	_key;
 
 	#init = false;
+	/** @type {undefined | true | (() => void)} */
+	#started;
 	/** @type {() => Promise<T>} */
 	#fn;
 	#loading = $state(true);
@@ -189,6 +191,13 @@ export class Query {
 		this.#promise = $state.raw(this.#run());
 	}
 
+	#resolve_started() {
+		if (typeof this.#started === 'function') {
+			this.#started();
+		}
+		this.#started = true;
+	}
+
 	#run() {
 		// Prevent state_unsafe_mutation error on first run when the resource is created within the template
 		if (this.#init) {
@@ -213,7 +222,25 @@ export class Query {
 			resolve
 		);
 
-		Promise.resolve(this.#fn())
+		Promise.resolve()
+			.then(() => {
+				// Avoid running the query if it is not used yet but withOverride was
+				// called on it already.
+				if (this.#overrides.length && !this.#started) {
+					/** @type {Promise<void>} */
+					const promise = new Promise((res) => {
+						resolve = res;
+					});
+					this.#started = resolve;
+					return promise;
+				}
+			})
+			.then(() => {
+				// Skip fetching if we waited on override and a set() or refresh() happened in the meantime
+				const idx = this.#latest.indexOf(resolve);
+				if (idx === -1) return;
+				return this.#fn();
+			})
 			.then((value) => {
 				// Skip the response if resource was refreshed with a later promise while we were waiting for this one to resolve
 				const idx = this.#latest.indexOf(resolve);
@@ -240,18 +267,29 @@ export class Query {
 		return promise;
 	}
 
+	// Hack because create_remote_function accesses .then right away,
+	// and we don't want to track that as "accessed now start" yet.
+	#first_then_invocation = false;
+
 	get then() {
+		if (this.#first_then_invocation) {
+			this.#resolve_started();
+		} else {
+			this.#first_then_invocation = true;
+		}
 		return this.#then;
 	}
 
 	get catch() {
+		this.#resolve_started();
 		this.#then;
 		return (/** @type {any} */ reject) => {
 			return this.#then(undefined, reject);
 		};
 	}
 
 	get finally() {
+		this.#resolve_started();
 		this.#then;
 		return (/** @type {any} */ fn) => {
 			return this.#then(
@@ -268,24 +306,28 @@ export class Query {
 	}
 
 	get current() {
+		this.#resolve_started();
 		return this.#current;
 	}
 
 	get error() {
+		this.#resolve_started();
 		return this.#error;
 	}
 
 	/**
 	 * Returns true if the resource is loading or reloading.
 	 */
 	get loading() {
+		this.#resolve_started();
 		return this.#loading;
 	}
 
 	/**
 	 * Returns true once the resource has been loaded for the first time.
 	 */
 	get ready() {
+		this.#resolve_started();
 		return this.#ready;
 	}
 
@@ -306,6 +348,14 @@ export class Query {
 		this.#error = undefined;
 		this.#raw = value;
 		this.#promise = Promise.resolve();
+		// any in-flight-fetches are now outdated
+		for (const latest of this.#latest) {
+			latest();
+		}
+		this.#latest = [];
+		// resolve potential pending promise to prevent memory leaks (.then() in create_remote_function would never resolve).
+		// It's fine to resolve the promise because it's a noop due to if condition in #fn
+		this.#resolve_started();
 	}
 
 	/**

packages/kit/test/apps/async/src/routes/remote/optimistic-new-query/+page.svelte

@@ -0,0 +1,12 @@
+<script>
+	import { foo, mutate } from './data.remote';
+
+	let show = $state(false);
+</script>
+
+<button onclick={() => mutate().updates(foo().withOverride(() => 'bar'))}>override</button>
+<button onclick={() => (show = true)}>show</button>
+
+{#if show}
+	<p>{await foo()}</p>
+{/if}

packages/kit/test/apps/async/src/routes/remote/optimistic-new-query/data.remote.js

@@ -0,0 +1,9 @@
+import { command, query } from '$app/server';
+
+export const foo = query(() => {
+	return 'foo';
+});
+
+export const mutate = command(() => {
+	foo().set('baz');
+});

packages/kit/test/apps/async/test/client.test.js

@@ -254,6 +254,21 @@ test.describe('remote function mutations', () => {
 		await expect(page.locator('#command-pending')).toHaveText('Command pending: 0');
 	});
 
+	test('optimistic overrides avoid unused query requests', async ({ page }) => {
+		await page.goto('/remote/optimistic-new-query');
+
+		let request_count = 0;
+		page.on('request', (r) => (request_count += r.url().includes('/_app/remote') ? 1 : 0));
+
+		await page.getByText('override').click();
+		await page.waitForTimeout(100); // allow any requests to happen
+		expect(request_count).toBe(1); // just the command
+
+		await page.getByText('show').click();
+		await expect(page.locator('p')).toHaveText('foo'); // set() from command mutation is obsolete as query was removed from cache
+		expect(request_count).toBe(2); // now the query, too
+	});
+
 	// TODO once we have async SSR adjust the test and move this into test.js
 	test('query.batch works', async ({ page }) => {
 		await page.goto('/remote/batch');