From 35f376a04220ed63b7f8a94c8c9e649345f0e36a Mon Sep 17 00:00:00 2001 From: Philip Breuer Date: Thu, 30 Oct 2025 16:48:13 +0100 Subject: [PATCH 1/5] Update query.js --- .../src/runtime/app/server/remote/query.js | 57 ++++++++++++++++--- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/packages/kit/src/runtime/app/server/remote/query.js b/packages/kit/src/runtime/app/server/remote/query.js index 22eed9a10b01..fd0debb57334 100644 --- a/packages/kit/src/runtime/app/server/remote/query.js +++ b/packages/kit/src/runtime/app/server/remote/query.js @@ -72,6 +72,10 @@ export function query(validate_or_fn, maybe_fn) { const { event, state } = get_request_store(); + let immediate_refresh = true; + + queueMicrotask(() => (immediate_refresh = false)); + /** @type {Promise & Partial>} */ const promise = get_response(__, arg, state, () => run_remote_function(event, state, false, arg, validate, fn) @@ -98,7 +102,7 @@ export function query(validate_or_fn, maybe_fn) { }; promise.refresh = () => { - const { state } = get_request_store(); + const { event, state } = get_request_store(); const refreshes = state.refreshes; if (!refreshes) { @@ -107,11 +111,25 @@ export function query(validate_or_fn, maybe_fn) { ); } - const cache_key = create_remote_cache_key(__.id, stringify_remote_arg(arg, state.transport)); - refreshes[cache_key] = promise; + const key = stringify_remote_arg(arg, state.transport); + const cache_key = create_remote_cache_key(__.id, key); + + if (immediate_refresh) { + refreshes[cache_key] = promise; + + return promise.then(() => {}); + } + + const refreshed = Promise.resolve( + run_remote_function(event, state, false, arg, validate, fn) + ); + + refreshed.catch(() => {}); + + const cache = get_cache(__, state); + cache[key] = refreshes[cache_key] = refreshed; - // TODO we could probably just return promise here, but would need to update the types - return promise.then(() => {}); + return refreshed.then(() => {}); }; promise.withOverride = () => { @@ -200,6 +218,10 @@ function batch(validate_or_fn, maybe_fn) { const { event, state } = get_request_store(); + let immediate_refresh = true; + + queueMicrotask(() => (immediate_refresh = false)); + /** @type {Promise & Partial>} */ const promise = get_response(__, arg, state, () => { // Collect all the calls to the same query in the same macrotask, @@ -243,8 +265,8 @@ function batch(validate_or_fn, maybe_fn) { promise.catch(() => {}); - promise.refresh = async () => { - const { state } = get_request_store(); + promise.refresh = () => { + const { event, state } = get_request_store(); const refreshes = state.refreshes; if (!refreshes) { @@ -253,8 +275,25 @@ function batch(validate_or_fn, maybe_fn) { ); } - const cache_key = create_remote_cache_key(__.id, stringify_remote_arg(arg, state.transport)); - refreshes[cache_key] = await /** @type {Promise} */ (promise); + const key = stringify_remote_arg(arg, state.transport); + const cache_key = create_remote_cache_key(__.id, key); + + if (immediate_refresh) { + refreshes[cache_key] = promise; + + return promise.then(() => {}); + } + + const refreshed = Promise.resolve( + run_remote_function(event, state, false, arg, validate, fn) + ); + + refreshed.catch(() => {}); + + const cache = get_cache(__, state); + cache[key] = refreshes[cache_key] = refreshed; + + return refreshed.then(() => {}); }; promise.withOverride = () => { From 8547bda1e0ca85218e281511b2745b7d514402cb Mon Sep 17 00:00:00 2001 From: Philip Breuer Date: Sat, 1 Nov 2025 12:57:29 +0100 Subject: [PATCH 2/5] Remove queueMicrotask --- .../src/runtime/app/server/remote/query.js | 62 +++++++++++-------- .../src/runtime/app/server/remote/shared.js | 2 +- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/packages/kit/src/runtime/app/server/remote/query.js b/packages/kit/src/runtime/app/server/remote/query.js index fd0debb57334..69c5fbab82fa 100644 --- a/packages/kit/src/runtime/app/server/remote/query.js +++ b/packages/kit/src/runtime/app/server/remote/query.js @@ -72,14 +72,10 @@ export function query(validate_or_fn, maybe_fn) { const { event, state } = get_request_store(); - let immediate_refresh = true; - - queueMicrotask(() => (immediate_refresh = false)); + const get_remote_function_result = () => run_remote_function(event, state, false, arg, validate, fn); /** @type {Promise & Partial>} */ - const promise = get_response(__, arg, state, () => - run_remote_function(event, state, false, arg, validate, fn) - ); + const promise = get_response(__, arg, state, get_remote_function_result); promise.catch(() => {}); @@ -94,6 +90,7 @@ export function query(validate_or_fn, maybe_fn) { ); } + // TODO: Shouldn't this throw to signify that non-exported queries can't be set? Or still update the cache without touching refreshes? if (__.id) { const cache = get_cache(__, state); const key = stringify_remote_arg(arg, state.transport); @@ -102,7 +99,7 @@ export function query(validate_or_fn, maybe_fn) { }; promise.refresh = () => { - const { event, state } = get_request_store(); + const { state } = get_request_store(); const refreshes = state.refreshes; if (!refreshes) { @@ -111,22 +108,20 @@ export function query(validate_or_fn, maybe_fn) { ); } + const cache = get_cache(__, state); const key = stringify_remote_arg(arg, state.transport); const cache_key = create_remote_cache_key(__.id, key); - if (immediate_refresh) { + // First time query has ever been triggered and .refresh() was called within the same microtask + if (!cache[key]) { refreshes[cache_key] = promise; - return promise.then(() => {}); } - const refreshed = Promise.resolve( - run_remote_function(event, state, false, arg, validate, fn) - ); + const refreshed = get_remote_function_result(); refreshed.catch(() => {}); - const cache = get_cache(__, state); cache[key] = refreshes[cache_key] = refreshed; return refreshed.then(() => {}); @@ -218,12 +213,7 @@ function batch(validate_or_fn, maybe_fn) { const { event, state } = get_request_store(); - let immediate_refresh = true; - - queueMicrotask(() => (immediate_refresh = false)); - - /** @type {Promise & Partial>} */ - const promise = get_response(__, arg, state, () => { + const get_remote_function_result = () => { // Collect all the calls to the same query in the same macrotask, // then execute them as one backend request. return new Promise((resolve, reject) => { @@ -261,12 +251,32 @@ function batch(validate_or_fn, maybe_fn) { } }, 0); }); - }); + }; + + /** @type {Promise & Partial>} */ + const promise = get_response(__, arg, state, get_remote_function_result); promise.catch(() => {}); + promise.set = (value) => { + const { state } = get_request_store(); + const refreshes = state.refreshes; + + if (!refreshes) { + throw new Error( + `Cannot call set on query.batch '${__.name}' because it is not executed in the context of a command/form remote function` + ); + } + + if (__.id) { + const key = stringify_remote_arg(arg, state.transport); + const cache_key = create_remote_cache_key(__.id, key); + refreshes[cache_key] = Promise.resolve(value); + } + } + promise.refresh = () => { - const { event, state } = get_request_store(); + const { state } = get_request_store(); const refreshes = state.refreshes; if (!refreshes) { @@ -275,22 +285,20 @@ function batch(validate_or_fn, maybe_fn) { ); } + const cache = get_cache(__, state); const key = stringify_remote_arg(arg, state.transport); const cache_key = create_remote_cache_key(__.id, key); - if (immediate_refresh) { + // First time query has ever been triggered and .refresh() was called within the same microtask + if (!cache[key]) { refreshes[cache_key] = promise; - return promise.then(() => {}); } - const refreshed = Promise.resolve( - run_remote_function(event, state, false, arg, validate, fn) - ); + const refreshed = get_remote_function_result(); refreshed.catch(() => {}); - const cache = get_cache(__, state); cache[key] = refreshes[cache_key] = refreshed; return refreshed.then(() => {}); diff --git a/packages/kit/src/runtime/app/server/remote/shared.js b/packages/kit/src/runtime/app/server/remote/shared.js index c078d290db6f..a92d1d4fc722 100644 --- a/packages/kit/src/runtime/app/server/remote/shared.js +++ b/packages/kit/src/runtime/app/server/remote/shared.js @@ -69,7 +69,7 @@ export function create_validator(validate_or_fn, maybe_fn) { * @returns {Promise} */ export async function get_response(info, arg, state, get_result) { - // wait a beat, in case `myQuery().set(...)` is immediately called + // wait a beat, in case `myQuery().set(...)` or `myQuery().refresh()` is immediately called // eslint-disable-next-line @typescript-eslint/await-thenable await 0; From 5dcd6241b4979e22739e9dbaf18cf184580db38e Mon Sep 17 00:00:00 2001 From: Philip Breuer Date: Sat, 1 Nov 2025 22:55:11 +0100 Subject: [PATCH 3/5] Add tests --- .../src/runtime/app/server/remote/query.js | 14 +++--- .../basics/src/routes/remote/+page.svelte | 9 ++++ .../src/routes/remote/batch/+page.svelte | 36 +++++++++++++-- .../src/routes/remote/batch/batch.remote.js | 46 +++++++++++++++---- .../src/routes/remote/query-command.remote.js | 7 +++ .../kit/test/apps/basics/test/client.test.js | 44 ++++++++++++++++++ 6 files changed, 138 insertions(+), 18 deletions(-) diff --git a/packages/kit/src/runtime/app/server/remote/query.js b/packages/kit/src/runtime/app/server/remote/query.js index 69c5fbab82fa..74768bac4fae 100644 --- a/packages/kit/src/runtime/app/server/remote/query.js +++ b/packages/kit/src/runtime/app/server/remote/query.js @@ -72,7 +72,8 @@ export function query(validate_or_fn, maybe_fn) { const { event, state } = get_request_store(); - const get_remote_function_result = () => run_remote_function(event, state, false, arg, validate, fn); + const get_remote_function_result = () => + run_remote_function(event, state, false, arg, validate, fn); /** @type {Promise & Partial>} */ const promise = get_response(__, arg, state, get_remote_function_result); @@ -90,11 +91,11 @@ export function query(validate_or_fn, maybe_fn) { ); } - // TODO: Shouldn't this throw to signify that non-exported queries can't be set? Or still update the cache without touching refreshes? if (__.id) { - const cache = get_cache(__, state); const key = stringify_remote_arg(arg, state.transport); - refreshes[create_remote_cache_key(__.id, key)] = cache[key] = Promise.resolve(value); + const cache_key = create_remote_cache_key(__.id, key); + const cache = get_cache(__, state); + cache[key] = refreshes[cache_key] = Promise.resolve(value); } }; @@ -271,9 +272,10 @@ function batch(validate_or_fn, maybe_fn) { if (__.id) { const key = stringify_remote_arg(arg, state.transport); const cache_key = create_remote_cache_key(__.id, key); - refreshes[cache_key] = Promise.resolve(value); + const cache = get_cache(__, state); + cache[key] = refreshes[cache_key] = Promise.resolve(value); } - } + }; promise.refresh = () => { const { state } = get_request_store(); diff --git a/packages/kit/test/apps/basics/src/routes/remote/+page.svelte b/packages/kit/test/apps/basics/src/routes/remote/+page.svelte index c8f1d23f8446..a7dbab7bf6ab 100644 --- a/packages/kit/test/apps/basics/src/routes/remote/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/remote/+page.svelte @@ -6,6 +6,7 @@ get_count, set_count, set_count_server_refresh, + set_count_server_refresh_after_read, set_count_server_set, resolve_deferreds } from './query-command.remote.js'; @@ -67,6 +68,14 @@ > command (query server refresh) + + + + diff --git a/packages/kit/test/apps/basics/src/routes/remote/batch/batch.remote.js b/packages/kit/test/apps/basics/src/routes/remote/batch/batch.remote.js index 917729e1c412..716cf0f83e7a 100644 --- a/packages/kit/test/apps/basics/src/routes/remote/batch/batch.remote.js +++ b/packages/kit/test/apps/basics/src/routes/remote/batch/batch.remote.js @@ -1,15 +1,43 @@ -import { query } from '$app/server'; +import { command, query } from '$app/server'; import { error } from '@sveltejs/kit'; +/** @type {Array<[string, { id: string; title: string }]>} **/ +const INITIAL_TODOS = [ + ['1', { id: '1', title: 'Buy groceries' }], + ['2', { id: '2', title: 'Walk the dog' }] +]; + +let todos = new Map(INITIAL_TODOS); + export const get_todo = query.batch('unchecked', (ids) => { - if (JSON.stringify(ids) !== JSON.stringify(['1', '2', 'error'])) { - throw new Error(`Expected 3 IDs (deduplicated), got ${JSON.stringify(ids)}`); + if (new Set(ids).size !== ids.length) { + throw new Error(`batch queries must be deduplicated, but got ${JSON.stringify(ids)}`); } - return (id) => - id === '1' - ? { id: '1', title: 'Buy groceries' } - : id === '2' - ? { id: '2', title: 'Walk the dog' } - : error(404, { message: 'Not found' }); + return (id) => { + if (id === 'error') return error(404, { message: 'Not found' }); + + return todos.get(id); + }; +}); + +export const set_todo_title = command('unchecked', ({ id, title }) => { + const todo = { id, title }; + todos.set(id, todo); + get_todo(id).set(todo); + return todo; +}); + +export const set_todo_title_server_refresh = command('unchecked', ({ id, title }) => { + const todo = { id, title }; + todos.set(id, todo); + get_todo(id).refresh(); + return todo; +}); + +export const reset_todos = command(() => { + todos = new Map(INITIAL_TODOS); + for (const [id, todo] of todos) { + get_todo(id).set({ ...todo }); + } }); diff --git a/packages/kit/test/apps/basics/src/routes/remote/query-command.remote.js b/packages/kit/test/apps/basics/src/routes/remote/query-command.remote.js index 5771fcb3af39..53334d628f94 100644 --- a/packages/kit/test/apps/basics/src/routes/remote/query-command.remote.js +++ b/packages/kit/test/apps/basics/src/routes/remote/query-command.remote.js @@ -36,6 +36,13 @@ export const set_count_server_refresh = command('unchecked', (c) => { return c; }); +export const set_count_server_refresh_after_read = command('unchecked', async (c) => { + await get_count(); + count = c; + await get_count().refresh(); + return c; +}); + export const set_count_server_set = command('unchecked', async (c) => { get_count_called = false; count = c; diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index df4c519275e7..b185b8a3c2b5 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -1860,6 +1860,20 @@ test.describe('remote functions', () => { expect(request_count).toBe(1); // no query refreshes, since that happens as part of the command response }); + test('command refresh after reading query reruns the query', async ({ page }) => { + await page.goto('/remote'); + await expect(page.locator('#count-result')).toHaveText('0 / 0 (false)'); + + let request_count = 0; + page.on('request', (r) => (request_count += r.url().includes('/_app/remote') ? 1 : 0)); + + await page.click('#multiply-server-refresh-after-read-btn'); + await expect(page.locator('#command-result')).toHaveText('6'); + await expect(page.locator('#count-result')).toHaveText('6 / 6 (false)'); + await page.waitForTimeout(100); // allow all requests to finish (in case there are query refreshes which shouldn't happen) + expect(request_count).toBe(1); + }); + test('command does server-initiated single flight mutation (set)', async ({ page }) => { await page.goto('/remote'); await expect(page.locator('#count-result')).toHaveText('0 / 0 (false)'); @@ -2027,6 +2041,36 @@ test.describe('remote functions', () => { expect(request_count).toBe(1); }); + test('query.batch set updates cache without extra request', async ({ page }) => { + await page.goto('/remote/batch'); + await page.click('#batch-reset-btn'); + await expect(page.locator('#batch-result-1')).toHaveText('Buy groceries'); + + let request_count = 0; + const handler = (r) => (request_count += r.url().includes('/_app/remote') ? 1 : 0); + page.on('request', handler); + + await page.click('#batch-set-btn'); + await expect(page.locator('#batch-result-1')).toHaveText('Buy cat food'); + await page.waitForTimeout(100); // allow all requests to finish + expect(request_count).toBe(1); // only the command request + }); + + test('query.batch refresh in command reuses single flight', async ({ page }) => { + await page.goto('/remote/batch'); + await page.click('#batch-reset-btn'); + await expect(page.locator('#batch-result-2')).toHaveText('Walk the dog'); + + let request_count = 0; + const handler = (r) => (request_count += r.url().includes('/_app/remote') ? 1 : 0); + page.on('request', handler); + + await page.click('#batch-refresh-btn'); + await expect(page.locator('#batch-result-2')).toHaveText('Walk the dog (refreshed)'); + await page.waitForTimeout(100); // allow all requests to finish + expect(request_count).toBe(1); // only the command request + }); + // TODO ditto test('query works with transport', async ({ page }) => { await page.goto('/remote/transport'); From a696f01a90c628111f4ba838969a55981d56ca71 Mon Sep 17 00:00:00 2001 From: Philip Breuer Date: Sun, 2 Nov 2025 01:19:55 +0100 Subject: [PATCH 4/5] Reduce code repetition --- .../src/runtime/app/server/remote/query.js | 151 ++++++++---------- 1 file changed, 64 insertions(+), 87 deletions(-) diff --git a/packages/kit/src/runtime/app/server/remote/query.js b/packages/kit/src/runtime/app/server/remote/query.js index 74768bac4fae..fd18b1d7bab8 100644 --- a/packages/kit/src/runtime/app/server/remote/query.js +++ b/packages/kit/src/runtime/app/server/remote/query.js @@ -80,52 +80,13 @@ export function query(validate_or_fn, maybe_fn) { promise.catch(() => {}); - /** @param {Output} value */ - promise.set = (value) => { - const { state } = get_request_store(); - const refreshes = state.refreshes; - - if (!refreshes) { - throw new Error( - `Cannot call set on query '${__.name}' because it is not executed in the context of a command/form remote function` - ); - } - - if (__.id) { - const key = stringify_remote_arg(arg, state.transport); - const cache_key = create_remote_cache_key(__.id, key); - const cache = get_cache(__, state); - cache[key] = refreshes[cache_key] = Promise.resolve(value); - } - }; + promise.set = (value) => update_refresh_value(get_refresh_context(__, 'set', arg), value); promise.refresh = () => { - const { state } = get_request_store(); - const refreshes = state.refreshes; - - if (!refreshes) { - throw new Error( - `Cannot call refresh on query '${__.name}' because it is not executed in the context of a command/form remote function` - ); - } - - const cache = get_cache(__, state); - const key = stringify_remote_arg(arg, state.transport); - const cache_key = create_remote_cache_key(__.id, key); - - // First time query has ever been triggered and .refresh() was called within the same microtask - if (!cache[key]) { - refreshes[cache_key] = promise; - return promise.then(() => {}); - } - - const refreshed = get_remote_function_result(); - - refreshed.catch(() => {}); - - cache[key] = refreshes[cache_key] = refreshed; - - return refreshed.then(() => {}); + const refresh_context = get_refresh_context(__, 'refresh', arg); + const is_immediate_refresh = !refresh_context.cache[refresh_context.key]; + const value = is_immediate_refresh ? promise : get_remote_function_result(); + return update_refresh_value(refresh_context, value, is_immediate_refresh); }; promise.withOverride = () => { @@ -259,51 +220,13 @@ function batch(validate_or_fn, maybe_fn) { promise.catch(() => {}); - promise.set = (value) => { - const { state } = get_request_store(); - const refreshes = state.refreshes; - - if (!refreshes) { - throw new Error( - `Cannot call set on query.batch '${__.name}' because it is not executed in the context of a command/form remote function` - ); - } - - if (__.id) { - const key = stringify_remote_arg(arg, state.transport); - const cache_key = create_remote_cache_key(__.id, key); - const cache = get_cache(__, state); - cache[key] = refreshes[cache_key] = Promise.resolve(value); - } - }; + promise.set = (value) => update_refresh_value(get_refresh_context(__, 'set', arg), value); promise.refresh = () => { - const { state } = get_request_store(); - const refreshes = state.refreshes; - - if (!refreshes) { - throw new Error( - `Cannot call refresh on query.batch '${__.name}' because it is not executed in the context of a command/form remote function` - ); - } - - const cache = get_cache(__, state); - const key = stringify_remote_arg(arg, state.transport); - const cache_key = create_remote_cache_key(__.id, key); - - // First time query has ever been triggered and .refresh() was called within the same microtask - if (!cache[key]) { - refreshes[cache_key] = promise; - return promise.then(() => {}); - } - - const refreshed = get_remote_function_result(); - - refreshed.catch(() => {}); - - cache[key] = refreshes[cache_key] = refreshed; - - return refreshed.then(() => {}); + const refresh_context = get_refresh_context(__, 'refresh', arg); + const is_immediate_refresh = !refresh_context.cache[refresh_context.key]; + const value = is_immediate_refresh ? promise : get_remote_function_result(); + return update_refresh_value(refresh_context, value, is_immediate_refresh); }; promise.withOverride = () => { @@ -320,3 +243,57 @@ function batch(validate_or_fn, maybe_fn) { // Add batch as a property to the query function Object.defineProperty(query, 'batch', { value: batch, enumerable: true }); + +/** + * @param {RemoteInfo} __ + * @param {'set' | 'refresh'} action + * @param {any} [arg] + * @returns {{ __: RemoteInfo; state: any; refreshes: Record>; cache: Record>; cache_key: string; key: string }} + */ +function get_refresh_context(__, action, arg) { + const { state } = get_request_store(); + const { refreshes } = state; + + if (!refreshes) { + throw new Error( + `Cannot call ${action} on ${format_remote_name(__)} because it is not executed in the context of a command/form remote function` + ); + } + + const key = stringify_remote_arg(arg, state.transport); + const cache_key = create_remote_cache_key(__.id, key); + const cache = get_cache(__, state); + + return { __, state, refreshes, cache, cache_key, key }; +} + +/** + * @param {{ __: RemoteInfo; refreshes: Record>; cache: Record>; cache_key: string; key: string }} context + * @param {any} value + * @param {boolean} [is_immediate_refresh=false] + * @returns {Promise} + */ +function update_refresh_value( + { __, refreshes, cache, cache_key, key }, + value, + is_immediate_refresh = false +) { + const promise = Promise.resolve(value); + + if (!is_immediate_refresh) { + cache[key] = promise; + } + + if (__.id) { + refreshes[cache_key] = promise; + } + + return promise.then(() => {}); +} + +/** + * @param {RemoteInfo} __ + */ +function format_remote_name(__) { + return __.type === 'query_batch' ? `query.batch '${__.name}'` : `query '${__.name}'`; +} From ad220011f10c0e105507d23446cf8000c5f4a7f3 Mon Sep 17 00:00:00 2001 From: Philip Breuer Date: Sun, 2 Nov 2025 01:57:12 +0100 Subject: [PATCH 5/5] Add changeset --- .changeset/swift-fans-check.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/swift-fans-check.md diff --git a/.changeset/swift-fans-check.md b/.changeset/swift-fans-check.md new file mode 100644 index 000000000000..309ef654e423 --- /dev/null +++ b/.changeset/swift-fans-check.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: correct query `.set` and `.refresh` behavior in commands