diff --git a/.changeset/rude-islands-flow.md b/.changeset/rude-islands-flow.md new file mode 100644 index 000000000000..3e7813c69467 --- /dev/null +++ b/.changeset/rude-islands-flow.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: use validated args in batch resolver in both csr and ssr diff --git a/packages/kit/src/runtime/app/server/remote/query.js b/packages/kit/src/runtime/app/server/remote/query.js index d8bb50ba7392..25f36a99207f 100644 --- a/packages/kit/src/runtime/app/server/remote/query.js +++ b/packages/kit/src/runtime/app/server/remote/query.js @@ -4,7 +4,13 @@ import { get_request_store } from '@sveltejs/kit/internal/server'; import { create_remote_key, stringify_remote_arg } from '../../../shared.js'; import { prerendering } from '__sveltekit/environment'; -import { create_validator, get_cache, get_response, run_remote_function } from './shared.js'; +import { + create_validator, + get_cache, + get_response, + run_remote_batch_function, + 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. @@ -150,8 +156,7 @@ function batch(validate_or_fn, maybe_fn) { name: '', run: (args) => { const { event, state } = get_request_store(); - - return run_remote_function( + return run_remote_batch_function( event, state, false, @@ -173,7 +178,7 @@ function batch(validate_or_fn, maybe_fn) { ); } - const { event, state } = get_request_store(); + const { state } = get_request_store(); const get_remote_function_result = () => { // Collect all the calls to the same query in the same macrotask, @@ -190,20 +195,14 @@ function batch(validate_or_fn, maybe_fn) { batching = { args: [], resolvers: [] }; try { - const get_result = await run_remote_function( - event, - state, - false, - batched.args, - (array) => Promise.all(array.map(validate)), - fn - ); + const results = await __.run(batched.args); for (let i = 0; i < batched.resolvers.length; i++) { - try { - batched.resolvers[i].resolve(get_result(batched.args[i], i)); - } catch (error) { - batched.resolvers[i].reject(error); + const result = results[i]; + if (result.status === 'fulfilled') { + batched.resolvers[i].resolve(result.value); + } else { + batched.resolvers[i].reject(result.reason); } } } catch (error) { diff --git a/packages/kit/src/runtime/app/server/remote/shared.js b/packages/kit/src/runtime/app/server/remote/shared.js index 43698b76244d..9f506183879f 100644 --- a/packages/kit/src/runtime/app/server/remote/shared.js +++ b/packages/kit/src/runtime/app/server/remote/shared.js @@ -92,18 +92,13 @@ export function parse_remote_response(data, transport) { } /** - * Like `with_event` but removes things from `event` you cannot see/call in remote functions, such as `setHeaders`. - * @template T * @param {RequestEvent} event * @param {RequestState} state * @param {boolean} allow_cookies - * @param {any} arg - * @param {(arg: any) => any} validate - * @param {(arg?: any) => T} fn + * @returns {RequestStore} */ -export async function run_remote_function(event, state, allow_cookies, arg, validate, fn) { - /** @type {RequestStore} */ - const store = { +function sanitize_event_for_remote_function(event, state, allow_cookies) { + return { event: { ...event, setHeaders: () => { @@ -140,12 +135,50 @@ export async function run_remote_function(event, state, allow_cookies, arg, vali is_in_remote_function: true } }; +} +/** + * Like `with_event` but removes things from `event` you cannot see/call in remote functions, such as `setHeaders`. + * @template T + * @param {RequestEvent} event + * @param {RequestState} state + * @param {boolean} allow_cookies + * @param {any} arg + * @param {(arg: any) => any} validate + * @param {(arg?: any) => T} fn + */ +export async function run_remote_function(event, state, allow_cookies, arg, validate, fn) { + const store = sanitize_event_for_remote_function(event, state, allow_cookies); // In two parts, each with_event, so that runtimes without async local storage can still get the event at the start of the function const validated = await with_request_store(store, () => validate(arg)); return with_request_store(store, () => fn(validated)); } +/** + * Additionally-constrained version of `run_remote_function` that handles the array/validation dance of batching. + * @template T + * @param {RequestEvent} event + * @param {RequestState} state + * @param {boolean} allow_cookies + * @param {any} arg + * @param {(arg: any[]) => MaybePromise} validate + * @param {(arg?: any[]) => MaybePromise<(arg: any, idx: number) => T>} fn + * @returns {Promise[]>} + */ +export async function run_remote_batch_function(event, state, allow_cookies, arg, validate, fn) { + const store = sanitize_event_for_remote_function(event, state, allow_cookies); + // In two parts, each with_event, so that runtimes without async local storage can still get the event at the start of the function + const validated = await with_request_store(store, () => validate(arg)); + const resolver = await with_request_store(store, () => fn(validated)); + return validated.map((value, index) => { + try { + return { status: 'fulfilled', value: resolver(value, index) }; + } catch (e) { + return { status: 'rejected', reason: e }; + } + }); +} + /** * @param {RemoteInfo} info * @param {RequestState} state diff --git a/packages/kit/src/runtime/server/remote.js b/packages/kit/src/runtime/server/remote.js index d52b4be8d87b..523077cd494b 100644 --- a/packages/kit/src/runtime/server/remote.js +++ b/packages/kit/src/runtime/server/remote.js @@ -1,4 +1,4 @@ -/** @import { ActionResult, RemoteForm, RequestEvent, SSRManifest } from '@sveltejs/kit' */ +/** @import { ActionResult, RemoteForm, RequestEvent, SSRManifest, Transport } from '@sveltejs/kit' */ /** @import { RemoteFunctionResponse, RemoteInfo, RequestState, SSROptions } from 'types' */ import { json, error } from '@sveltejs/kit'; @@ -74,28 +74,8 @@ async function handle_remote_call_internal(event, state, options, manifest, id) const { payloads } = await event.request.json(); const args = payloads.map((payload) => parse_remote_arg(payload, transport)); - const get_result = await with_request_store({ event, state }, () => info.run(args)); - const results = await Promise.all( - args.map(async (arg, i) => { - try { - return { type: 'result', data: get_result(arg, i) }; - } catch (error) { - return { - type: 'error', - error: await handle_error_and_jsonify(event, state, options, error), - status: - error instanceof HttpError || error instanceof SvelteKitError ? error.status : 500 - }; - } - }) - ); - - return json( - /** @type {RemoteFunctionResponse} */ ({ - type: 'result', - result: stringify(results, transport) - }) - ); + const results = await info.run(args); + return batch_to_response(results, transport, event, state, options); } if (info.type === 'form') { @@ -340,3 +320,33 @@ export function get_remote_id(url) { export function get_remote_action(url) { return url.searchParams.get('/remote'); } + +/** + * @param {PromiseSettledResult[]} results + * @param {Transport} transport + * @param {RequestEvent} event + * @param {RequestState} state + * @param {SSROptions} options + * @returns {Promise} + */ +async function batch_to_response(results, transport, event, state, options) { + const data = await Promise.all( + results.map(async (result) => { + if (result.status === 'fulfilled') { + return { type: 'result', data: result.value }; + } else { + const err = result.reason; + return { + type: 'error', + error: await handle_error_and_jsonify(event, state, options, err), + status: err instanceof HttpError || err instanceof SvelteKitError ? err.status : 500 + }; + } + }) + ); + + return json({ + type: 'result', + result: stringify(data, transport) + }); +} diff --git a/packages/kit/src/types/internal.d.ts b/packages/kit/src/types/internal.d.ts index 6384201af551..1d305b044c4d 100644 --- a/packages/kit/src/types/internal.d.ts +++ b/packages/kit/src/types/internal.d.ts @@ -571,7 +571,7 @@ export type RemoteInfo = id: string; name: string; /** Direct access to the function without batching etc logic, for remote functions called from the client */ - run: (args: any[]) => Promise<(arg: any, idx: number) => any>; + run: (args: any[]) => Promise>>; } | { type: 'form'; diff --git a/packages/kit/test/apps/async/src/routes/remote/batch-validation/+page.svelte b/packages/kit/test/apps/async/src/routes/remote/batch-validation/+page.svelte new file mode 100644 index 000000000000..710c4f806c6a --- /dev/null +++ b/packages/kit/test/apps/async/src/routes/remote/batch-validation/+page.svelte @@ -0,0 +1,13 @@ + + +
+ {#each words as word, i} + {await reverse(word)}{i === words.length - 1 ? '' : ' '} + {/each} +
+ diff --git a/packages/kit/test/apps/async/src/routes/remote/batch-validation/batch.remote.js b/packages/kit/test/apps/async/src/routes/remote/batch-validation/batch.remote.js new file mode 100644 index 000000000000..95ff1e550aa8 --- /dev/null +++ b/packages/kit/test/apps/async/src/routes/remote/batch-validation/batch.remote.js @@ -0,0 +1,12 @@ +import { query } from '$app/server'; +import * as v from 'valibot'; + +export const reverse = query.batch( + v.pipe( + v.string(), + v.transform((val) => val.split('').reverse().join('')) + ), + () => { + return (x) => x; + } +); diff --git a/packages/kit/test/apps/async/test/client.test.js b/packages/kit/test/apps/async/test/client.test.js index 8266b5ac8338..d6d4a4da9717 100644 --- a/packages/kit/test/apps/async/test/client.test.js +++ b/packages/kit/test/apps/async/test/client.test.js @@ -301,6 +301,14 @@ test.describe('remote function mutations', () => { expect(request_count).toBe(1); // only the command request }); + test('query.batch resolver function always receives validated arguments', async ({ page }) => { + await page.goto('/remote/batch-validation'); + + await expect(page.locator('#phrase')).toHaveText('use the force'); + await page.locator('button').click(); + await expect(page.locator('#phrase')).toHaveText('i am your father'); + }); + // TODO ditto test('query works with transport', async ({ page }) => { await page.goto('/remote/transport');