Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/rude-islands-flow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: use validated args in batch resolver in both csr and ssr
31 changes: 15 additions & 16 deletions packages/kit/src/runtime/app/server/remote/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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) {
Expand Down
49 changes: 41 additions & 8 deletions packages/kit/src/runtime/app/server/remote/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: () => {
Expand Down Expand Up @@ -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<any[]>} validate
* @param {(arg?: any[]) => MaybePromise<(arg: any, idx: number) => T>} fn
* @returns {Promise<PromiseSettledResult<T>[]>}
*/
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
Expand Down
56 changes: 33 additions & 23 deletions packages/kit/src/runtime/server/remote.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -340,3 +320,33 @@ export function get_remote_id(url) {
export function get_remote_action(url) {
return url.searchParams.get('/remote');
}

/**
* @param {PromiseSettledResult<any>[]} results
* @param {Transport} transport
* @param {RequestEvent} event
* @param {RequestState} state
* @param {SSROptions} options
* @returns {Promise<Response>}
*/
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)
});
}
2 changes: 1 addition & 1 deletion packages/kit/src/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Array<PromiseSettledResult<any>>>;
}
| {
type: 'form';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<script lang="ts">
import { reverse } from './batch.remote.js';

let phrase = $state('ecrof eht esu');
const words = $derived(phrase.split(' ').reverse());
</script>

<div id="phrase">
{#each words as word, i}
{await reverse(word)}{i === words.length - 1 ? '' : ' '}
{/each}
</div>
<button onclick={() => (phrase = 'rehtaf ruoy ma i')}>get dramatic</button>
Original file line number Diff line number Diff line change
@@ -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;
}
);
8 changes: 8 additions & 0 deletions packages/kit/test/apps/async/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Loading