diff --git a/.changeset/soft-emus-fry.md b/.changeset/soft-emus-fry.md new file mode 100644 index 000000000000..7a92ae52e5da --- /dev/null +++ b/.changeset/soft-emus-fry.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: create separate cache entries for non-exported remote function queries diff --git a/documentation/docs/20-core-concepts/60-remote-functions.md b/documentation/docs/20-core-concepts/60-remote-functions.md index a1c63dc7b124..a0a1a47eaa86 100644 --- a/documentation/docs/20-core-concepts/60-remote-functions.md +++ b/documentation/docs/20-core-concepts/60-remote-functions.md @@ -995,13 +995,13 @@ export const getProfile = query(async () => { }; }); -// this function could be called from multiple places -function getUser() { - const { cookies, locals } = getRequestEvent(); +// this query could be called from multiple places, but +// the function will only run once per request +const getUser = query(() => { + const { cookies } = getRequestEvent(); - locals.userPromise ??= findUser(cookies.get('session_id')); - return await locals.userPromise; -} + return await findUser(cookies.get('session_id')); +}); ``` Note that some properties of `RequestEvent` are different inside remote functions. There are no `params` or `route.id`, and you cannot set headers (other than writing cookies, and then only inside `form` and `command` functions), and `url.pathname` is always `/` (since the path that’s actually being requested by the client is purely an implementation detail). diff --git a/packages/kit/src/runtime/app/server/remote/form.js b/packages/kit/src/runtime/app/server/remote/form.js index 01e2c893df76..1fe3390b18c5 100644 --- a/packages/kit/src/runtime/app/server/remote/form.js +++ b/packages/kit/src/runtime/app/server/remote/form.js @@ -3,7 +3,7 @@ /** @import { StandardSchemaV1 } from '@standard-schema/spec' */ import { get_request_store } from '@sveltejs/kit/internal/server'; import { DEV } from 'esm-env'; -import { run_remote_function } from './shared.js'; +import { get_cache, run_remote_function } from './shared.js'; import { convert_formdata, flatten_issues } from '../../../utils.js'; /** @@ -166,7 +166,7 @@ export function form(validate_or_fn, maybe_fn) { // We don't need to care about args or deduplicating calls, because uneval results are only relevant in full page reloads // where only one form submission is active at the same time if (!event.isRemoteRequest) { - (state.remote_data ??= {})[__.id] = output; + get_cache(__, state)[''] ??= output; } return output; @@ -189,8 +189,7 @@ export function form(validate_or_fn, maybe_fn) { Object.defineProperty(instance, property, { get() { try { - const { remote_data } = get_request_store().state; - return remote_data?.[__.id]?.[property] ?? {}; + return get_cache(__)?.['']?.[property] ?? {}; } catch { return undefined; } @@ -201,8 +200,7 @@ export function form(validate_or_fn, maybe_fn) { Object.defineProperty(instance, 'result', { get() { try { - const { remote_data } = get_request_store().state; - return remote_data?.[__.id]?.result; + return get_cache(__)?.['']?.result; } catch { return undefined; } diff --git a/packages/kit/src/runtime/app/server/remote/prerender.js b/packages/kit/src/runtime/app/server/remote/prerender.js index c73fef4dbb4e..8c19732bc52c 100644 --- a/packages/kit/src/runtime/app/server/remote/prerender.js +++ b/packages/kit/src/runtime/app/server/remote/prerender.js @@ -4,10 +4,11 @@ import { error, json } from '@sveltejs/kit'; import { DEV } from 'esm-env'; import { get_request_store } from '@sveltejs/kit/internal/server'; -import { create_remote_cache_key, stringify, stringify_remote_arg } from '../../../shared.js'; +import { stringify, stringify_remote_arg } from '../../../shared.js'; import { app_dir, base } from '__sveltekit/paths'; import { create_validator, + get_cache, get_response, parse_remote_response, run_remote_function @@ -96,25 +97,29 @@ export function prerender(validate_or_fn, fn_or_options, maybe_options) { if (!state.prerendering && !DEV && !event.isRemoteRequest) { try { - return await get_response(id, arg, state, async () => { + return await get_response(__, arg, state, async () => { + const key = stringify_remote_arg(arg, state.transport); + const cache = get_cache(__, state); + // TODO adapters can provide prerendered data more efficiently than // fetching from the public internet - const response = await fetch(new URL(url, event.url.origin).href); - - if (!response.ok) { - throw new Error('Prerendered response not found'); - } + const promise = (cache[key] ??= fetch(new URL(url, event.url.origin).href).then( + async (response) => { + if (!response.ok) { + throw new Error('Prerendered response not found'); + } - const prerendered = await response.json(); + const prerendered = await response.json(); - if (prerendered.type === 'error') { - error(prerendered.status, prerendered.error); - } + if (prerendered.type === 'error') { + error(prerendered.status, prerendered.error); + } - // TODO can we redirect here? + return prerendered.result; + } + )); - (state.remote_data ??= {})[create_remote_cache_key(id, payload)] = prerendered.result; - return parse_remote_response(prerendered.result, state.transport); + return parse_remote_response(await promise, state.transport); }); } catch { // not available prerendered, fallback to normal function @@ -125,7 +130,7 @@ export function prerender(validate_or_fn, fn_or_options, maybe_options) { return /** @type {Promise} */ (state.prerendering.remote_responses.get(url)); } - const promise = get_response(id, arg, state, () => + const promise = get_response(__, arg, state, () => run_remote_function(event, state, false, arg, validate, fn) ); diff --git a/packages/kit/src/runtime/app/server/remote/query.js b/packages/kit/src/runtime/app/server/remote/query.js index bb36543f17fa..2974f7228271 100644 --- a/packages/kit/src/runtime/app/server/remote/query.js +++ b/packages/kit/src/runtime/app/server/remote/query.js @@ -4,7 +4,7 @@ import { get_request_store } from '@sveltejs/kit/internal/server'; import { create_remote_cache_key, stringify_remote_arg } from '../../../shared.js'; import { prerendering } from '__sveltekit/environment'; -import { create_validator, get_response, run_remote_function } from './shared.js'; +import { create_validator, get_cache, get_response, run_remote_function } from './shared.js'; /** * Creates a remote query. When called from the browser, the function will be invoked on the server via a `fetch` call. @@ -73,7 +73,7 @@ export function query(validate_or_fn, maybe_fn) { const { event, state } = get_request_store(); /** @type {Promise & Partial>} */ - const promise = get_response(__.id, arg, state, () => + const promise = get_response(__, arg, state, () => run_remote_function(event, state, false, arg, validate, fn) ); @@ -90,8 +90,12 @@ 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] = (state.remote_data ??= {})[cache_key] = Promise.resolve(value); + const cache = get_cache(__, state); + const key = stringify_remote_arg(arg, state.transport); + + if (__.id) { + refreshes[__.id + '/' + key] = cache[key] = Promise.resolve(value); + } }; promise.refresh = () => { @@ -198,7 +202,7 @@ function batch(validate_or_fn, maybe_fn) { const { event, state } = get_request_store(); /** @type {Promise & Partial>} */ - const promise = get_response(__.id, arg, state, () => { + const promise = get_response(__, arg, state, () => { // Collect all the calls to the same query in the same macrotask, // then execute them as one backend request. return new Promise((resolve, reject) => { diff --git a/packages/kit/src/runtime/app/server/remote/shared.js b/packages/kit/src/runtime/app/server/remote/shared.js index c0c7c19a2be4..48afb882b21b 100644 --- a/packages/kit/src/runtime/app/server/remote/shared.js +++ b/packages/kit/src/runtime/app/server/remote/shared.js @@ -1,9 +1,9 @@ /** @import { RequestEvent } from '@sveltejs/kit' */ -/** @import { ServerHooks, MaybePromise, RequestState } from 'types' */ +/** @import { ServerHooks, MaybePromise, RequestState, RemoteInfo } from 'types' */ import { parse } from 'devalue'; import { error } from '@sveltejs/kit'; import { with_request_store, get_request_store } from '@sveltejs/kit/internal/server'; -import { create_remote_cache_key, stringify_remote_arg } from '../../../shared.js'; +import { stringify_remote_arg } from '../../../shared.js'; /** * @param {any} validate_or_fn @@ -62,19 +62,20 @@ export function create_validator(validate_or_fn, maybe_fn) { * Also saves an uneval'ed version of the result for later HTML inlining for hydration. * * @template {MaybePromise} T - * @param {string} id + * @param {RemoteInfo} info * @param {any} arg * @param {RequestState} state * @param {() => Promise} get_result * @returns {Promise} */ -export async function get_response(id, arg, state, get_result) { +export async function get_response(info, arg, state, get_result) { // wait a beat, in case `myQuery().set(...)` is immediately called // eslint-disable-next-line @typescript-eslint/await-thenable await 0; - const cache_key = create_remote_cache_key(id, stringify_remote_arg(arg, state.transport)); - return ((state.remote_data ??= {})[cache_key] ??= get_result()); + const cache = get_cache(info, state); + + return (cache[stringify_remote_arg(arg, state.transport)] ??= get_result()); } /** @@ -141,3 +142,18 @@ export async function run_remote_function(event, state, allow_cookies, arg, vali const validated = await with_request_store({ event: cleansed, state }, () => validate(arg)); return with_request_store({ event: cleansed, state }, () => fn(validated)); } + +/** + * @param {RemoteInfo} info + * @param {RequestState} state + */ +export function get_cache(info, state = get_request_store().state) { + let cache = state.remote_data?.get(info); + + if (cache === undefined) { + cache = {}; + (state.remote_data ??= new Map()).set(info, cache); + } + + return cache; +} diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index 90fe3aefe3b0..bc46fcd12535 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -467,16 +467,22 @@ export async function render_response({ args.push(`{\n${indent}\t${hydrate.join(`,\n${indent}\t`)}\n${indent}}`); } - const { remote_data } = event_state; + const { remote_data: remote_cache } = event_state; let serialized_remote_data = ''; - if (remote_data) { + if (remote_cache) { /** @type {Record} */ const remote = {}; - for (const key in remote_data) { - remote[key] = await remote_data[key]; + for (const [info, cache] of remote_cache) { + // remote functions without an `id` aren't exported, and thus + // cannot be called from the client + if (!info.id) continue; + + for (const key in cache) { + remote[key ? info.id + '/' + key : info.id] = await cache[key]; + } } // TODO this is repeated in a few places — dedupe it diff --git a/packages/kit/src/types/internal.d.ts b/packages/kit/src/types/internal.d.ts index c1167bd79e8d..a9203cf1f94b 100644 --- a/packages/kit/src/types/internal.d.ts +++ b/packages/kit/src/types/internal.d.ts @@ -597,7 +597,7 @@ export interface RequestState { record_span: RecordSpan; }; form_instances?: Map; - remote_data?: Record>; + remote_data?: Map>>; refreshes?: Record>; is_endpoint_request?: boolean; } diff --git a/packages/kit/test/apps/basics/src/routes/remote/query-non-exported/+page.svelte b/packages/kit/test/apps/basics/src/routes/remote/query-non-exported/+page.svelte new file mode 100644 index 000000000000..5fc45d97d451 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/remote/query-non-exported/+page.svelte @@ -0,0 +1,8 @@ + + + +{#await total() then t} +

{t}

+{/await} diff --git a/packages/kit/test/apps/basics/src/routes/remote/query-non-exported/data.remote.ts b/packages/kit/test/apps/basics/src/routes/remote/query-non-exported/data.remote.ts new file mode 100644 index 000000000000..f444537b3892 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/remote/query-non-exported/data.remote.ts @@ -0,0 +1,8 @@ +import { query } from '$app/server'; + +const one = query(() => 1); +const two = query(() => 2); + +export const total = query(async () => { + return (await one()) + (await two()); +}); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 799220608522..ff54734dff48 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1633,6 +1633,15 @@ test.describe('remote functions', () => { await expect(page.locator('#redirected')).toHaveText('redirected'); }); + test('non-exported queries do not clobber each other', async ({ page, javaScriptEnabled }) => { + // TODO remove once async SSR exists + if (!javaScriptEnabled) return; + + await page.goto('/remote/query-non-exported'); + + await expect(page.locator('h1')).toHaveText('3'); + }); + test('form works', async ({ page, javaScriptEnabled }) => { await page.goto('/remote/form');