fix: don't refresh queries automatically when running commands (#14170)

* fix: don't refresh queries automatically when running commands

* update docs

* unused

* DRY out, fix type error

Author: Rich-Harris

.changeset/upset-dots-change.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: don't refresh queries automatically when running commands

documentation/docs/20-core-concepts/60-remote-functions.md

@@ -503,9 +503,11 @@ Now simply call `addLike`, from (for example) an event handler:
 
 > [!NOTE] Commands cannot be called during render.
 
-### Single-flight mutations
+### Updating queries
+
+To update `getLikes(item.id)`, or any other query, we need to tell SvelteKit _which_ queries need to be refreshed (unlike `form`, which by default invalidates everything, to approximate the behaviour of a native form submission).
 
-As with forms, any queries on the page (such as `getLikes(item.id)` in the example above) will automatically be refreshed following a successful command. But we can make things more efficient by telling SvelteKit which queries will be affected by the command, either inside the command itself...
+We either do that inside the command itself...
 
 ```js
 /// file: likes.remote.js

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

@@ -61,7 +61,11 @@ export function command(id) {
 					release_overrides(updates);
 					throw new HttpError(result.status ?? 500, result.error);
 				} else {
-					refresh_queries(result.refreshes, updates);
+					console.log('refreshes', result.refreshes);
+
+					if (result.refreshes) {
+						refresh_queries(result.refreshes, updates);
+					}
 
 					return devalue.parse(result.result, app.decoders);
 				}

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

@@ -5,7 +5,14 @@ import { app_dir, base } from '__sveltekit/paths';
 import * as devalue from 'devalue';
 import { DEV } from 'esm-env';
 import { HttpError } from '@sveltejs/kit/internal';
-import { app, remote_responses, started, goto, set_nearest_error_page } from '../client.js';
+import {
+	app,
+	remote_responses,
+	started,
+	goto,
+	set_nearest_error_page,
+	invalidateAll
+} from '../client.js';
 import { tick } from 'svelte';
 import { refresh_queries, release_overrides } from './shared.svelte.js';
 
@@ -84,7 +91,11 @@ export function form(id) {
 					if (form_result.type === 'result') {
 						result = devalue.parse(form_result.result, app.decoders);
 
-						refresh_queries(form_result.refreshes, updates);
+						if (form_result.refreshes) {
+							refresh_queries(form_result.refreshes, updates);
+						} else {
+							void invalidateAll();
+						}
 					} else if (form_result.type === 'redirect') {
 						const refreshes = form_result.refreshes ?? '';
 						const invalidateAll = !refreshes && updates.length === 0;

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

@@ -2,7 +2,7 @@
 /** @import { RemoteFunctionResponse } from 'types' */
 /** @import { Query } from './query.svelte.js' */
 import * as devalue from 'devalue';
-import { app, goto, invalidateAll, query_map } from '../client.js';
+import { app, goto, query_map } from '../client.js';
 import { HttpError, Redirect } from '@sveltejs/kit/internal';
 import { tick } from 'svelte';
 import { create_remote_cache_key, stringify_remote_arg } from '../../shared.js';
@@ -125,19 +125,16 @@ export function release_overrides(updates) {
  */
 export function refresh_queries(stringified_refreshes, updates = []) {
 	const refreshes = Object.entries(devalue.parse(stringified_refreshes, app.decoders));
-	if (refreshes.length > 0) {
-		// `refreshes` is a superset of `updates`
-		for (const [key, value] of refreshes) {
-			// If there was an optimistic update, release it right before we update the query
-			const update = updates.find((u) => u._key === key);
-			if (update && 'release' in update) {
-				update.release();
-			}
-			// Update the query with the new value
-			const entry = query_map.get(key);
-			entry?.resource.set(value);
+
+	// `refreshes` is a superset of `updates`
+	for (const [key, value] of refreshes) {
+		// If there was an optimistic update, release it right before we update the query
+		const update = updates.find((u) => u._key === key);
+		if (update && 'release' in update) {
+			update.release();
 		}
-	} else {
-		void invalidateAll();
+		// Update the query with the new value
+		const entry = query_map.get(key);
+		entry?.resource.set(value);
 	}
 }

packages/kit/src/runtime/server/remote.js

@@ -62,13 +62,7 @@ export async function handle_remote_call(event, options, manifest, id) {
 				/** @type {RemoteFunctionResponse} */ ({
 					type: 'result',
 					result: stringify(data, transport),
-					refreshes: stringify(
-						{
-							...get_event_state(event).refreshes,
-							...(await apply_client_refreshes(/** @type {string[]} */ (form_client_refreshes)))
-						},
-						transport
-					)
+					refreshes: await serialize_refreshes(/** @type {string[]} */ (form_client_refreshes))
 				})
 			);
 		}
@@ -78,13 +72,12 @@ export async function handle_remote_call(event, options, manifest, id) {
 			const { payload, refreshes } = await event.request.json();
 			const arg = parse_remote_arg(payload, transport);
 			const data = await with_event(event, () => fn(arg));
-			const refreshed = await apply_client_refreshes(refreshes);
 
 			return json(
 				/** @type {RemoteFunctionResponse} */ ({
 					type: 'result',
 					result: stringify(data, transport),
-					refreshes: stringify({ ...get_event_state(event).refreshes, ...refreshed }, transport)
+					refreshes: await serialize_refreshes(refreshes)
 				})
 			);
 		}
@@ -107,14 +100,10 @@ export async function handle_remote_call(event, options, manifest, id) {
 		);
 	} catch (error) {
 		if (error instanceof Redirect) {
-			const refreshes = {
-				...(get_event_state(event).refreshes ?? {}), // could be set by form actions
-				...(await apply_client_refreshes(form_client_refreshes ?? []))
-			};
 			return json({
 				type: 'redirect',
 				location: error.location,
-				refreshes: Object.keys(refreshes).length > 0 ? stringify(refreshes, transport) : undefined
+				refreshes: await serialize_refreshes(form_client_refreshes ?? [])
 			});
 		}
 
@@ -132,26 +121,33 @@ export async function handle_remote_call(event, options, manifest, id) {
 		);
 	}
 
-	/** @param {string[]} refreshes */
-	async function apply_client_refreshes(refreshes) {
-		return Object.fromEntries(
-			await Promise.all(
-				refreshes.map(async (key) => {
-					const [hash, name, payload] = key.split('/');
-					const loader = manifest._.remotes[hash];
-
-					// TODO what do we do in this case? erroring after the mutation has happened is not great
-					if (!loader) error(400, 'Bad Request');
-
-					const module = await loader();
-					const fn = module[name];
-
-					if (!fn) error(400, 'Bad Request');
-
-					return [key, await with_event(event, () => fn(parse_remote_arg(payload, transport)))];
-				})
+	/**
+	 * @param {string[]} client_refreshes
+	 */
+	async function serialize_refreshes(client_refreshes) {
+		const refreshes = {
+			...get_event_state(event).refreshes,
+			...Object.fromEntries(
+				await Promise.all(
+					client_refreshes.map(async (key) => {
+						const [hash, name, payload] = key.split('/');
+						const loader = manifest._.remotes[hash];
+
+						// TODO what do we do in this case? erroring after the mutation has happened is not great
+						if (!loader) error(400, 'Bad Request');
+
+						const module = await loader();
+						const fn = module[name];
+
+						if (!fn) error(400, 'Bad Request');
+
+						return [key, await with_event(event, () => fn(parse_remote_arg(payload, transport)))];
+					})
+				)
 			)
-		);
+		};
+
+		return Object.keys(refreshes).length > 0 ? stringify(refreshes, transport) : undefined;
 	}
 }
 

packages/kit/src/types/internal.d.ts

@@ -302,7 +302,7 @@ export type RemoteFunctionResponse =
 			type: 'result';
 			result: string;
 			/** devalue'd Record<string, any> */
-			refreshes: string;
+			refreshes: string | undefined;
 	  };
 
 /**

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

@@ -1664,7 +1664,7 @@ test.describe('remote functions', () => {
 		expect(request_count).toBe(2);
 	});
 
-	test('command returns correct sum and refreshes all data by default', async ({ page }) => {
+	test('command returns correct sum but does not refresh data by default', async ({ page }) => {
 		await page.goto('/remote');
 		await expect(page.locator('#count-result')).toHaveText('0 / 0 (false)');
 
@@ -1673,9 +1673,9 @@ test.describe('remote functions', () => {
 
 		await page.click('#multiply-btn');
 		await expect(page.locator('#command-result')).toHaveText('2');
-		await expect(page.locator('#count-result')).toHaveText('2 / 2 (false)');
+		await expect(page.locator('#count-result')).toHaveText('0 / 0 (false)');
 		await page.waitForTimeout(100); // allow all requests to finish
-		expect(request_count).toBe(4); // 1 for the command, 3 for the refresh
+		expect(request_count).toBe(1); // 1 for the command, no refreshes
 	});
 
 	test('command returns correct sum and does client-initiated single flight mutation', async ({