fix: create separate cache entries for non-exported queries

.changeset/soft-emus-fry.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: create separate cache entries for non-exported remote function queries

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).

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;
 				}

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<any>} */ (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)
 			);
 

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<any> & Partial<RemoteQuery<any>>} */
-		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<any> & Partial<RemoteQuery<any>>} */
-		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) => {

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<any>} T
- * @param {string} id
+ * @param {RemoteInfo} info
  * @param {any} arg
  * @param {RequestState} state
  * @param {() => Promise<T>} get_result
  * @returns {Promise<T>}
  */
-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;
+}

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<string, any>} */
 			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

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

@@ -597,7 +597,7 @@ export interface RequestState {
 		record_span: RecordSpan;
 	};
 	form_instances?: Map<any, any>;
-	remote_data?: Record<string, MaybePromise<any>>;
+	remote_data?: Map<RemoteInfo, Record<string, MaybePromise<any>>>;
 	refreshes?: Record<string, Promise<any>>;
 	is_endpoint_request?: boolean;
 }

packages/kit/test/apps/basics/src/routes/remote/query-non-exported/+page.svelte

@@ -0,0 +1,8 @@
+<script>
+	import { total } from './data.remote.js';
+</script>
+
+<!-- TODO replace with inline await -->
+{#await total() then t}
+	<h1>{t}</h1>
+{/await}

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

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

@@ -1633,6 +1633,15 @@
 		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');
 
@@ -1738,7 +1747,7 @@
 		);
 	});
 
 	test('form scoping with for(...) works', async ({ page, javaScriptEnabled }) => {
 		await page.goto('/remote/form');
 
 		await page.fill('[data-scoped] input', 'hello');