From 1c3ba7a479fe259fe97d2c990f573a61fa4f1d42 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 31 Aug 2022 15:17:56 +0200 Subject: [PATCH 01/85] basic actions method --- packages/kit/src/exports/index.js | 12 +- packages/kit/src/runtime/control.js | 13 ++ .../kit/src/runtime/server/page/actions.js | 121 ++++++++++++++ packages/kit/src/runtime/server/page/index.js | 149 ++++-------------- .../kit/src/runtime/server/page/render.js | 14 +- packages/kit/src/runtime/server/utils.js | 11 ++ .../actions/form-errors/+page.server.js | 10 +- .../post-explicit/+page.server.js | 2 +- .../post-implicit/+page.server.js | 2 +- .../mutative-endpoint/+page.server.ts | 2 +- .../shadowed/error-post/+page.server.js | 13 +- .../shadowed/missing-get/+page.server.js | 2 +- .../routes/shadowed/no-get/+page.server.js | 2 +- .../post-success-redirect/+page.server.js | 8 +- .../redirect-post-with-cookie/+page.server.js | 2 +- .../shadowed/redirect-post/+page.server.js | 2 +- .../shadowed/simple/post/+page.server.js | 2 +- .../src/routes/shadowed-post/+page.server.js | 2 +- packages/kit/types/index.d.ts | 17 +- packages/kit/types/internal.d.ts | 5 +- 20 files changed, 228 insertions(+), 163 deletions(-) create mode 100644 packages/kit/src/runtime/server/page/actions.js diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index 0a924350779a..22490b1b74ca 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -1,4 +1,4 @@ -import { HttpError, Redirect } from '../runtime/control.js'; +import { HttpError, Redirect, ValidationError } from '../runtime/control.js'; /** * Creates an `HttpError` object with an HTTP status code and an optional message. @@ -43,3 +43,13 @@ export function json(data, init) { headers }); } + +/** + * Generates a `ValidationError` object. + * @param {number} status + * @param {Record} errors + * @param {FormData} [form] + */ +export function validation(status, errors, form) { + return new ValidationError(status, errors, form); +} diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index 912572472dff..b0924a426788 100644 --- a/packages/kit/src/runtime/control.js +++ b/packages/kit/src/runtime/control.js @@ -31,3 +31,16 @@ export class Redirect { this.location = location; } } + +export class ValidationError { + /** + * @param {number} status + * @param {Record} errors + * @param {FormData} [form] + */ + constructor(status, errors, form) { + this.form = form; + this.status = status; + this.errors = errors; + } +} diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js new file mode 100644 index 000000000000..9d06ac14291c --- /dev/null +++ b/packages/kit/src/runtime/server/page/actions.js @@ -0,0 +1,121 @@ +import { error, json } from '../../../exports/index.js'; +import { negotiate } from '../../../utils/http.js'; +import { HttpError, Redirect, ValidationError } from '../../control.js'; +import { error_to_pojo } from '../utils.js'; + +/** + * @param {import('types').RequestEvent} event + */ +export function is_action_json_request(event) { + const accept = negotiate(event.request.headers.get('accept') || 'text/html', [ + 'text/html', + 'application/json' + ]); + + return ( + accept === 'application/json' && + event.request.method !== 'GET' && + event.request.method !== 'HEAD' + ); +} + +/** + * @param {import('types').RequestEvent} event + * @param {import('types').SSROptions} options + * @param {import('types').SSRNode['server']} server + */ +export async function handle_action_json_request(event, options, server) { + // TODO create POJO interface for this with + // {status: number, errors?: Record, form?: Record, result?: Record} + const handler = server.actions; + + if (!handler) { + maybe_throw_migration_error(server); + // TODO should this be a different error altogether? + return new Response('POST method not allowed. No actions exist for this page', { + status: 405, + headers: { + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405 + // "The server must generate an Allow header field in a 405 status code response" + allow: '' + } + }); + } + + try { + const result = await handler.call(null, event); + if (!result) { + // TODO json({status: 204}) instead? + return new Response(undefined, { + status: 204 + }); + } else { + return json({ status: 200, result }); + } + } catch (e) { + const error = /** @type {Redirect | HttpError | ValidationError | Error} */ (e); + + if (error instanceof Redirect) { + return json({ status: 303, location: error.location }); + } + + if (error instanceof ValidationError) { + return json({ + status: error.status, + form: error.form, + errors: error.errors + }); + } + + if (!(error instanceof HttpError)) { + options.handle_error(error, event); + } + + return json(error_to_pojo(error, options.get_stack), { + status: error instanceof HttpError ? error.status : 500 + }); + } +} + +/** + * @param {import('types').RequestEvent} event + * @param {import('types').SSRNode} leaf_node + * @returns + */ +export function is_action_request(event, leaf_node) { + return leaf_node.server && event.request.method !== 'GET' && event.request.method !== 'HEAD'; +} + +/** + * @param {import('types').RequestEvent} event + * @param {import('types').SSRNode['server']} server + * @returns {Promise | void>} + * @throws {Redirect | ValidationError | HttpError | Error} + */ +export async function handle_action_request(event, server) { + const handler = server.actions; + + if (!handler) { + maybe_throw_migration_error(server); + // TODO should this be a different error altogether? + event.setHeaders({ + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405 + // "The server must generate an Allow header field in a 405 status code response" + allow: '' + }); + throw error(405, 'POST method not allowed. No actions exist for this page'); + } + + return await handler.call(null, event); +} + +/** + * @param {import('types').SSRNode['server']} server + */ +function maybe_throw_migration_error(server) { + for (const method of ['POST', 'PUT', 'PATCH', 'DELETE']) { + if (/** @type {any} */ (server)[method]) { + throw new Error(`${method} method no longer allowed in +page.server, use actions instead.`); + } + } +} diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index 1d05961f9b90..8a88d2c34eac 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/packages/kit/src/runtime/server/page/index.js @@ -1,28 +1,19 @@ import { devalue } from 'devalue'; -import { negotiate } from '../../../utils/http.js'; -import { render_response } from './render.js'; -import { respond_with_error } from './respond_with_error.js'; -import { - method_not_allowed, - error_to_pojo, - allowed_methods, - get_option, - static_error_page -} from '../utils.js'; -import { create_fetch } from './fetch.js'; -import { HttpError, Redirect } from '../../control.js'; -import { error, json } from '../../../exports/index.js'; +import { DATA_SUFFIX } from '../../../constants.js'; import { compact } from '../../../utils/array.js'; import { normalize_error } from '../../../utils/error.js'; +import { HttpError, Redirect, ValidationError } from '../../control.js'; +import { get_option, redirect_response, static_error_page } from '../utils.js'; +import { + handle_action_json_request, + handle_action_request, + is_action_json_request, + is_action_request +} from './actions.js'; +import { create_fetch } from './fetch.js'; import { load_data, load_server_data } from './load_data.js'; -import { DATA_SUFFIX } from '../../../constants.js'; - -/** - * @typedef {import('./types.js').Loaded} Loaded - * @typedef {import('types').SSRNode} SSRNode - * @typedef {import('types').SSROptions} SSROptions - * @typedef {import('types').SSRState} SSRState - */ +import { render_response } from './render.js'; +import { respond_with_error } from './respond_with_error.js'; /** * @param {import('types').RequestEvent} event @@ -41,19 +32,10 @@ export async function render_page(event, route, page, options, state, resolve_op }); } - const accept = negotiate(event.request.headers.get('accept') || 'text/html', [ - 'text/html', - 'application/json' - ]); - - if ( - accept === 'application/json' && - event.request.method !== 'GET' && - event.request.method !== 'HEAD' - ) { + if (is_action_json_request(event)) { const node = await options.manifest._.nodes[page.leaf](); if (node.server) { - return handle_json_request(event, options, node.server); + return handle_action_json_request(event, options, node.server); } } @@ -68,42 +50,32 @@ export async function render_page(event, route, page, options, state, resolve_op let status = 200; + /** @type {Record | void} */ + let mutation_data; + /** @type {HttpError | Error} */ let mutation_error; - /** @type {Record | undefined} */ + /** @type {ValidationError | undefined} */ let validation_errors; - if (leaf_node.server && event.request.method !== 'GET' && event.request.method !== 'HEAD') { - // for non-GET requests, first call handler in +page.server.js + if (is_action_request(event, leaf_node)) { + // for action requests, first call handler in +page.server.js // (this also determines status code) try { - const method = /** @type {'POST' | 'PATCH' | 'PUT' | 'DELETE'} */ (event.request.method); - const handler = leaf_node.server[method]; - if (handler) { - const result = await handler.call(null, event); - - if (result?.errors) { - validation_errors = result.errors; - status = result.status ?? 400; - } - - if (event.request.method === 'POST' && result?.location) { - return redirect_response(303, result.location); - } - } else { - event.setHeaders({ - allow: allowed_methods(leaf_node.server).join(', ') - }); - - mutation_error = error(405, 'Method not allowed'); - } + mutation_data = await handle_action_request(event, leaf_node.server); } catch (e) { - if (e instanceof Redirect) { - return redirect_response(e.status, e.location); + const error = /** @type {Redirect | HttpError | ValidationError | Error} */ (e); + if (error instanceof Redirect) { + return redirect_response(303, error.location); + } + status = + error instanceof HttpError || error instanceof ValidationError ? error.status : 500; + if (error instanceof ValidationError) { + validation_errors = error; + } else { + mutation_error = error; } - - mutation_error = /** @type {HttpError | Error} */ (e); } } @@ -116,7 +88,7 @@ export async function render_page(event, route, page, options, state, resolve_op const should_prerender = get_option(nodes, 'prerender') ?? false; if (should_prerender) { const mod = leaf_node.server; - if (mod && (mod.POST || mod.PUT || mod.DELETE || mod.PATCH)) { + if (mod && mod.actions) { throw new Error('Cannot prerender pages that have mutative methods'); } } else if (state.prerendering) { @@ -153,7 +125,7 @@ export async function render_page(event, route, page, options, state, resolve_op }); } - /** @type {Array} */ + /** @type {Array} */ let branch = []; /** @type {Error | null} */ @@ -347,58 +319,3 @@ export async function render_page(event, route, page, options, state, resolve_op }); } } - -/** - * @param {import('types').RequestEvent} event - * @param {import('types').SSROptions} options - * @param {import('types').SSRNode['server']} mod - */ -export async function handle_json_request(event, options, mod) { - const method = /** @type {'POST' | 'PUT' | 'PATCH' | 'DELETE'} */ (event.request.method); - const handler = mod[method]; - - if (!handler) { - return method_not_allowed(mod, method); - } - - try { - // @ts-ignore - const result = await handler.call(null, event); - - if (result?.errors) { - // @ts-ignore - return json({ errors: result.errors }, { status: result.status || 400 }); - } - - return new Response(undefined, { - status: 204, - // @ts-ignore - headers: result?.location ? { location: result.location } : undefined - }); - } catch (e) { - const error = normalize_error(e); - - if (error instanceof Redirect) { - return redirect_response(error.status, error.location); - } - - if (!(error instanceof HttpError)) { - options.handle_error(error, event); - } - - return json(error_to_pojo(error, options.get_stack), { - status: error instanceof HttpError ? error.status : 500 - }); - } -} - -/** - * @param {number} status - * @param {string} location - */ -function redirect_response(status, location) { - return new Response(undefined, { - status, - headers: { location } - }); -} diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index 4ca30dc50516..621ef4107cf5 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -6,7 +6,7 @@ import { render_json_payload_script } from '../../../utils/escape.js'; import { s } from '../../../utils/misc.js'; import { Csp } from './csp.js'; import { serialize_error } from '../utils.js'; -import { HttpError } from '../../control.js'; +import { HttpError, ValidationError } from '../../control.js'; // TODO rename this function/module @@ -28,7 +28,7 @@ const updated = { * error: HttpError | Error | null; * event: import('types').RequestEvent; * resolve_opts: import('types').RequiredResolveOptions; - * validation_errors: Record | undefined; + * validation_errors: ValidationError | undefined; * }} opts */ export async function render_response({ @@ -104,7 +104,7 @@ export async function render_response({ }; if (validation_errors) { - props.errors = validation_errors; + props.errors = validation_errors.errors; } // TODO remove this for 1.0 @@ -190,7 +190,7 @@ export async function render_response({ if (validation_errors) { try { - serialized.errors = devalue(validation_errors); + serialized.errors = devalue(validation_errors.errors); } catch (e) { // If we're here, the data could not be serialized with devalue const error = /** @type {any} */ (e); @@ -296,12 +296,6 @@ export async function render_response({ ); } - if (validation_errors) { - serialized_data.push( - render_json_payload_script({ type: 'validation_errors' }, validation_errors) - ); - } - body += `\n\t${serialized_data.join('\n\t')}`; } diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js index 26c173ad86d6..19d116ca98ab 100644 --- a/packages/kit/src/runtime/server/utils.js +++ b/packages/kit/src/runtime/server/utils.js @@ -175,3 +175,14 @@ export function static_error_page(options, status, message) { status }); } + +/** + * @param {number} status + * @param {string} location + */ +export function redirect_response(status, location) { + return new Response(undefined, { + status, + headers: { location } + }); +} diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js index 29b3ed36c390..9c710f75cd9d 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js @@ -1,7 +1,5 @@ -export const POST = async () => { - return { - errors: { - message: 'an error occurred' - } - }; +import { validation } from '@sveltejs/kit'; + +export const actions = async () => { + throw validation(400, { message: 'an error occurred' }); }; diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit/+page.server.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit/+page.server.js index 749e56f85049..06d7d4c68648 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit/+page.server.js @@ -1,5 +1,5 @@ import { error } from '@sveltejs/kit'; -export const POST = () => { +export const actions = () => { throw error(400, 'oops'); }; diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit/+page.server.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit/+page.server.js index 11d6342a446f..52fdfea67b73 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit/+page.server.js @@ -1,5 +1,5 @@ import { FancyError } from '../_shared.js'; -export const POST = () => { +export const actions = () => { throw new FancyError('oops'); }; diff --git a/packages/kit/test/apps/basics/src/routes/prerendering/mutative-endpoint/+page.server.ts b/packages/kit/test/apps/basics/src/routes/prerendering/mutative-endpoint/+page.server.ts index c18506c83cf3..5e288445a189 100644 --- a/packages/kit/test/apps/basics/src/routes/prerendering/mutative-endpoint/+page.server.ts +++ b/packages/kit/test/apps/basics/src/routes/prerendering/mutative-endpoint/+page.server.ts @@ -1,3 +1,3 @@ -export const POST = () => { +export const actions = () => { return {}; }; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js index 7bc1eeac4928..73af9f7a128a 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js @@ -1,16 +1,15 @@ +import { validation } from '@sveltejs/kit'; + export function load() { return { get_message: 'hello from get' }; } -export async function POST({ request }) { +export async function actions({ request }) { const data = await request.formData(); - return { - status: 400, - errors: { - post_message: `echo: ${data.get('message')}` - } - }; + throw validation(400, { + post_message: `echo: ${data.get('message')}` + }); } diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/missing-get/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/missing-get/+page.server.js index 1f87355e2984..b0492ae1be04 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/missing-get/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/missing-get/+page.server.js @@ -1 +1 @@ -export const POST = () => {}; +export const actions = () => {}; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/no-get/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/no-get/+page.server.js index 1a24c4bbba85..7a665e58770c 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/no-get/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/no-get/+page.server.js @@ -1,3 +1,3 @@ -export function POST() { +export function actions() { return {}; } diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/post-success-redirect/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/post-success-redirect/+page.server.js index 30d0b0e3fb87..715b6f7d5fcd 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/post-success-redirect/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/post-success-redirect/+page.server.js @@ -1,5 +1,5 @@ -export function POST() { - return { - location: '/shadowed/post-success-redirect/redirected' - }; +import { redirect } from '@sveltejs/kit'; + +export function actions() { + throw redirect(303, '/shadowed/post-success-redirect/redirected'); } diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post-with-cookie/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post-with-cookie/+page.server.js index bdd7b53b5995..cc6d024f551f 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post-with-cookie/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post-with-cookie/+page.server.js @@ -1,6 +1,6 @@ import { redirect } from '@sveltejs/kit'; -export function POST({ setHeaders }) { +export function actions({ setHeaders }) { setHeaders({ 'set-cookie': 'shadow-redirect=happy' }); throw redirect(302, '/shadowed/redirected'); } diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post/+page.server.js index 2b8b1976028e..132a628414b4 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post/+page.server.js @@ -1,5 +1,5 @@ import { redirect } from '@sveltejs/kit'; -export function POST() { +export function actions() { throw redirect(302, '/shadowed/redirected'); } diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/simple/post/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/simple/post/+page.server.js index fee7fff4e694..a9a749929336 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/simple/post/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/simple/post/+page.server.js @@ -1,2 +1,2 @@ /** @type {import('./$types').Action} */ -export function POST() {} +export function actions() {} diff --git a/packages/kit/test/prerendering/basics/src/routes/shadowed-post/+page.server.js b/packages/kit/test/prerendering/basics/src/routes/shadowed-post/+page.server.js index 4b2b54377b72..62159134ad87 100644 --- a/packages/kit/test/prerendering/basics/src/routes/shadowed-post/+page.server.js +++ b/packages/kit/test/prerendering/basics/src/routes/shadowed-post/+page.server.js @@ -4,4 +4,4 @@ export function load() { }; } -export function POST() {} +export function actions() {} diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index ffe84ca00789..210c86105905 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -17,7 +17,7 @@ import { TrailingSlash } from './private.js'; import { SSRNodeLoader, SSRRoute, ValidatedConfig } from './internal.js'; -import { HttpError, Redirect } from '../src/runtime/control.js'; +import { HttpError, Redirect, ValidationError } from '../src/runtime/control.js'; export { PrerenderOption } from './private.js'; @@ -312,11 +312,7 @@ export interface ServerLoadEvent< export interface Action< Params extends Partial> = Partial> > { - (event: RequestEvent): MaybePromise< - | { status?: number; errors: Record; location?: never } - | { status?: never; errors?: never; location: string } - | void - >; + (event: RequestEvent): MaybePromise | void>; } // TODO figure out how to just re-export from '../src/index/index.js' without @@ -341,3 +337,12 @@ export function redirect(status: number, location: string): Redirect; * Generates a JSON `Response` object from the supplied data. */ export function json(data: any, init?: ResponseInit): Response; + +/** + * Generates a `ValidationError` object. + */ +export function validation( + status: number, + errors: Record, + form?: FormData +): ValidationError; diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts index c5669e7867ad..17d3d12a231c 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -280,10 +280,7 @@ export interface SSRNode { prerender?: PrerenderOption; ssr?: boolean; csr?: boolean; - POST?: Action; - PATCH?: Action; - PUT?: Action; - DELETE?: Action; + actions?: Action; }; // store this in dev so we can print serialization errors From 9ac19bc59c4c20eccfbd05e29899f741c2983f12 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 31 Aug 2022 16:29:57 +0200 Subject: [PATCH 02/85] add form store --- packages/kit/src/core/sync/write_root.js | 3 ++ packages/kit/src/runtime/app/stores.js | 19 +++++++++++- packages/kit/src/runtime/client/client.js | 13 +++++--- packages/kit/src/runtime/client/singletons.js | 5 ++- packages/kit/src/runtime/client/types.d.ts | 1 + packages/kit/src/runtime/control.js | 19 ++++++++++-- .../kit/src/runtime/server/page/actions.js | 16 ++++++---- .../kit/src/runtime/server/page/render.js | 8 +++-- .../+page.server.js | 10 ++++++ .../form-errors-persist-fields/+page.svelte | 31 +++++++++++++++++++ packages/kit/test/apps/basics/test/test.js | 19 ++++++++++++ packages/kit/types/ambient.d.ts | 8 ++++- 12 files changed, 133 insertions(+), 19 deletions(-) create mode 100644 packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js create mode 100644 packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte diff --git a/packages/kit/src/core/sync/write_root.js b/packages/kit/src/core/sync/write_root.js index 7bc1f7a79ee3..9414c5859775 100644 --- a/packages/kit/src/core/sync/write_root.js +++ b/packages/kit/src/core/sync/write_root.js @@ -51,13 +51,16 @@ export function write_root(manifest_data, output) { export let components; ${levels.map((l) => `export let data_${l} = null;`).join('\n\t\t\t\t')} + // form actions export let errors; + export let values; if (!browser) { setContext('__svelte__', stores); } $: stores.page.set(page); + $: stores.form.set({ errors, values }); afterUpdate(stores.page.notify); let mounted = false; diff --git a/packages/kit/src/runtime/app/stores.js b/packages/kit/src/runtime/app/stores.js index 0eb99f4dfa08..d27927c53475 100644 --- a/packages/kit/src/runtime/app/stores.js +++ b/packages/kit/src/runtime/app/stores.js @@ -25,7 +25,8 @@ export const getStores = () => { navigating: { subscribe: stores.navigating.subscribe }, - updated: stores.updated + updated: stores.updated, + form: stores.form }; // TODO remove this for 1.0 @@ -68,6 +69,22 @@ export const navigating = { } }; +/** @type {typeof import('$app/stores').form} */ +export const form = { + set(values) { + const store = getStores().form; + return store.set(values); + }, + update(value) { + const store = getStores().form; + return store.update(value); + }, + subscribe(fn) { + const store = getStores().form; + return store.subscribe(fn); + } +}; + function removed_session() { // TODO remove for 1.0 throw new Error( diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 6b95fbbfd1a7..7b5d3af5f3af 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -8,7 +8,7 @@ import { error } from '../../exports/index.js'; import Root from '__GENERATED__/root.svelte'; import { nodes, server_loads, dictionary, matchers } from '__GENERATED__/client-manifest.js'; -import { HttpError, Redirect } from '../control.js'; +import { HttpError, Redirect, ValidationError } from '../control.js'; import { stores } from './singletons.js'; import { DATA_SUFFIX } from '../../constants.js'; @@ -396,7 +396,7 @@ export function create_client({ target, base, trailing_slash }) { * status: number; * error: HttpError | Error | null; * routeId: string | null; - * validation_errors?: Record | null; + * validation_errors?: ValidationError | null; * }} opts */ async function get_navigation_result_from_branch({ @@ -422,7 +422,8 @@ export function create_client({ target, base, trailing_slash }) { }, props: { components: filtered.map((branch_node) => branch_node.node.component), - errors: validation_errors + errors: validation_errors?.errors ?? null, + values: validation_errors?.values ?? null } }; @@ -1288,7 +1289,8 @@ export function create_client({ target, base, trailing_slash }) { params, routeId, data: server_data_nodes, - errors: validation_errors + errors: validation_errors, + values: form_values }) => { const url = new URL(location.href); @@ -1329,7 +1331,8 @@ export function create_client({ target, base, trailing_slash }) { original_error.message ) : original_error, - validation_errors, + validation_errors: + validation_errors && new ValidationError(400, validation_errors, form_values), routeId }); } catch (e) { diff --git a/packages/kit/src/runtime/client/singletons.js b/packages/kit/src/runtime/client/singletons.js index f1ab3986cb6c..90de032f9cb7 100644 --- a/packages/kit/src/runtime/client/singletons.js +++ b/packages/kit/src/runtime/client/singletons.js @@ -17,5 +17,8 @@ export const stores = { url: notifiable_store({}), page: notifiable_store({}), navigating: writable(/** @type {import('types').Navigation | null} */ (null)), - updated: create_updated_store() + updated: create_updated_store(), + form: writable( + /** @type {{errors: Record, values?: Record} | null} */ (null) + ) }; diff --git a/packages/kit/src/runtime/client/types.d.ts b/packages/kit/src/runtime/client/types.d.ts index e86bc3ae0eb2..bdd90bafffd2 100644 --- a/packages/kit/src/runtime/client/types.d.ts +++ b/packages/kit/src/runtime/client/types.d.ts @@ -29,6 +29,7 @@ export interface Client { routeId: string | null; data: Array; errors: Record | null; + values: Record | null; }) => Promise; _start_router: () => void; } diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index b0924a426788..8140486b373e 100644 --- a/packages/kit/src/runtime/control.js +++ b/packages/kit/src/runtime/control.js @@ -36,10 +36,23 @@ export class ValidationError { /** * @param {number} status * @param {Record} errors - * @param {FormData} [form] + * @param {FormData | Record | null | undefined} [values] */ - constructor(status, errors, form) { - this.form = form; + constructor(status, errors, values) { + // NodeJS doesn't have a FormData class, so we have to check it differently + if (typeof values?.get === 'function') { + /** @type {Record} */ + const converted = {}; + for (const [key, value] of values.entries()) { + if (typeof value === 'string') { + converted[key] = value; + } + } + this.values = converted; + } else { + this.values = values; + } + this.status = status; this.errors = errors; } diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 9d06ac14291c..e31fe75abd81 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -7,6 +7,7 @@ import { error_to_pojo } from '../utils.js'; * @param {import('types').RequestEvent} event */ export function is_action_json_request(event) { + console.log(event.request.headers.get('accept')); const accept = negotiate(event.request.headers.get('accept') || 'text/html', [ 'text/html', 'application/json' @@ -26,7 +27,7 @@ export function is_action_json_request(event) { */ export async function handle_action_json_request(event, options, server) { // TODO create POJO interface for this with - // {status: number, errors?: Record, form?: Record, result?: Record} + // {status: number, errors?: Record, values?: Record, result?: Record} const handler = server.actions; if (!handler) { @@ -60,11 +61,14 @@ export async function handle_action_json_request(event, options, server) { } if (error instanceof ValidationError) { - return json({ - status: error.status, - form: error.form, - errors: error.errors - }); + return json( + { + status: error.status, + values: error.values, + errors: error.errors + }, + { status: error.status } + ); } if (!(error instanceof HttpError)) { diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index 621ef4107cf5..fcc87b6bb6b5 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -80,6 +80,7 @@ export async function render_response({ stores: { page: writable(null), navigating: writable(null), + form: writable(null), updated }, components: await Promise.all(branch.map(({ node }) => node.component())) @@ -105,6 +106,7 @@ export async function render_response({ if (validation_errors) { props.errors = validation_errors.errors; + props.values = validation_errors.values; } // TODO remove this for 1.0 @@ -174,7 +176,7 @@ export async function render_response({ /** @param {string} path */ const prefixed = (path) => (path.startsWith('/') ? path : `${assets}/${path}`); - const serialized = { data: '', errors: 'null' }; + const serialized = { data: '', errors: 'null', values: 'null' }; try { serialized.data = devalue(branch.map(({ server_data }) => server_data)); @@ -191,6 +193,7 @@ export async function render_response({ if (validation_errors) { try { serialized.errors = devalue(validation_errors.errors); + serialized.values = devalue(validation_errors.values); } catch (e) { // If we're here, the data could not be serialized with devalue const error = /** @type {any} */ (e); @@ -212,7 +215,8 @@ export async function render_response({ params: ${devalue(event.params)}, routeId: ${s(event.routeId)}, data: ${serialized.data}, - errors: ${serialized.errors} + errors: ${serialized.errors}, + values: ${serialized.values}, }` : 'null'}, paths: ${s(options.paths)}, target: document.querySelector('[data-sveltekit-hydrate="${target}"]').parentNode, diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js new file mode 100644 index 000000000000..632f1a436519 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js @@ -0,0 +1,10 @@ +import { validation } from '@sveltejs/kit'; + +/** + * @type {import('./$types').Action} + */ +export const actions = async ({ request }) => { + const data = await request.formData(); + data.delete('password'); + throw validation(400, { message: 'invalid credentials' }, data); +}; diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte new file mode 100644 index 000000000000..28f21b6f1990 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte @@ -0,0 +1,31 @@ + + +
+ + + +
+ + +{#if hydrated_form_values} +
{JSON.stringify(hydrated_form_values)}
+{/if} diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 6a9bc4cf0d8e..e18818fd3c1a 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1734,4 +1734,23 @@ test.describe('Actions', () => { expect(await page.textContent('p.client')).toBe('hydrated: an error occurred'); } }); + + test('Form fields are persisted', async ({ page, javaScriptEnabled }) => { + await page.goto('/actions/form-errors-persist-fields'); + await page.type('input[name="username"]', 'foo'); + await page.type('input[name="password"]', 'bar'); + await Promise.all([ + page.waitForRequest((request) => + request.url().includes('/actions/form-errors-persist-fields') + ), + page.click('button') + ]); + expect(await page.inputValue('input[name="username"]')).toBe('foo'); + if (javaScriptEnabled) { + expect(await page.inputValue('input[name="password"]')).toBe('bar'); + expect(await page.textContent('pre')).toBe(JSON.stringify({ username: 'foo' })); + } else { + expect(await page.inputValue('input[name="password"]')).toBe(''); + } + }); }); diff --git a/packages/kit/types/ambient.d.ts b/packages/kit/types/ambient.d.ts index 15d7421507bb..d5ef1a47855b 100644 --- a/packages/kit/types/ambient.d.ts +++ b/packages/kit/types/ambient.d.ts @@ -185,7 +185,7 @@ declare module '$app/paths' { * In the browser, we don't need to worry about this, and stores can be accessed from anywhere. Code that will only ever run on the browser can refer to (or subscribe to) any of these stores at any time. */ declare module '$app/stores' { - import { Readable } from 'svelte/store'; + import { Readable, Writable } from 'svelte/store'; import { Navigation, Page } from '@sveltejs/kit'; /** @@ -202,6 +202,11 @@ declare module '$app/stores' { * A readable store whose initial value is `false`. If [`version.pollInterval`](https://kit.svelte.dev/docs/configuration#version) is a non-zero value, SvelteKit will poll for new versions of the app and update the store value to `true` when it detects one. `updated.check()` will force an immediate check, regardless of polling. */ export const updated: Readable & { check: () => boolean }; + /** + * A writable store whose value contain the errors and values of the last form submission. + * It is updated automatically when using the `
` component, else you need to take care of it yourself. + */ + export const form: Writable<{ errors: Record; values: Record }>; /** * A function that returns all of the contextual stores. On the server, this must be called during component initialization. @@ -211,6 +216,7 @@ declare module '$app/stores' { navigating: typeof navigating; page: typeof page; updated: typeof updated; + form: typeof form; }; } From 7b71fa357784dbd28dfaee998d42f19d31840287 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 31 Aug 2022 18:17:15 +0200 Subject: [PATCH 03/85] remove export let errors in favor of form store --- packages/kit/src/core/sync/write_root.js | 12 +++++------- packages/kit/src/runtime/client/client.js | 15 ++++++--------- packages/kit/src/runtime/client/start.js | 10 +--------- packages/kit/src/runtime/client/types.d.ts | 5 ++--- packages/kit/src/runtime/server/page/actions.js | 4 ++-- packages/kit/src/runtime/server/page/render.js | 14 +++++++------- .../form-errors-persist-fields/+page.svelte | 2 -- .../src/routes/actions/form-errors/+page.svelte | 7 +++---- .../src/routes/shadowed/error-post/+page.svelte | 7 ++----- packages/kit/types/ambient.d.ts | 4 ++-- packages/kit/types/index.d.ts | 6 ++++++ 11 files changed, 36 insertions(+), 50 deletions(-) diff --git a/packages/kit/src/core/sync/write_root.js b/packages/kit/src/core/sync/write_root.js index 9414c5859775..28694883acfa 100644 --- a/packages/kit/src/core/sync/write_root.js +++ b/packages/kit/src/core/sync/write_root.js @@ -21,16 +21,16 @@ export function write_root(manifest_data, output) { let l = max_depth; - let pyramid = ``; + let pyramid = ``; while (l--) { pyramid = ` {#if components[${l + 1}]} - + ${pyramid.replace(/\n/g, '\n\t\t\t\t\t')} {:else} - + {/if} ` .replace(/^\t\t\t/gm, '') @@ -51,16 +51,14 @@ export function write_root(manifest_data, output) { export let components; ${levels.map((l) => `export let data_${l} = null;`).join('\n\t\t\t\t')} - // form actions - export let errors; - export let values; + export let form; if (!browser) { setContext('__svelte__', stores); } $: stores.page.set(page); - $: stores.form.set({ errors, values }); + $: stores.form.set(form); afterUpdate(stores.page.notify); let mounted = false; diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 7b5d3af5f3af..3cd49247cae8 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -8,7 +8,7 @@ import { error } from '../../exports/index.js'; import Root from '__GENERATED__/root.svelte'; import { nodes, server_loads, dictionary, matchers } from '__GENERATED__/client-manifest.js'; -import { HttpError, Redirect, ValidationError } from '../control.js'; +import { HttpError, Redirect } from '../control.js'; import { stores } from './singletons.js'; import { DATA_SUFFIX } from '../../constants.js'; @@ -396,7 +396,7 @@ export function create_client({ target, base, trailing_slash }) { * status: number; * error: HttpError | Error | null; * routeId: string | null; - * validation_errors?: ValidationError | null; + * form_state?: import('types').FormState | null; * }} opts */ async function get_navigation_result_from_branch({ @@ -406,7 +406,7 @@ export function create_client({ target, base, trailing_slash }) { status, error, routeId, - validation_errors + form_state }) { const filtered = /** @type {import('./types').BranchNode[] } */ (branch.filter(Boolean)); @@ -422,8 +422,7 @@ export function create_client({ target, base, trailing_slash }) { }, props: { components: filtered.map((branch_node) => branch_node.node.component), - errors: validation_errors?.errors ?? null, - values: validation_errors?.values ?? null + form: form_state } }; @@ -1289,8 +1288,7 @@ export function create_client({ target, base, trailing_slash }) { params, routeId, data: server_data_nodes, - errors: validation_errors, - values: form_values + form: form_state }) => { const url = new URL(location.href); @@ -1331,8 +1329,7 @@ export function create_client({ target, base, trailing_slash }) { original_error.message ) : original_error, - validation_errors: - validation_errors && new ValidationError(400, validation_errors, form_values), + form_state, routeId }); } catch (e) { diff --git a/packages/kit/src/runtime/client/start.js b/packages/kit/src/runtime/client/start.js index ac27e445dc2a..635bcf3dc10e 100644 --- a/packages/kit/src/runtime/client/start.js +++ b/packages/kit/src/runtime/client/start.js @@ -6,15 +6,7 @@ import { set_public_env } from '../env-public.js'; /** * @param {{ * env: Record; - * hydrate: { - * status: number; - * error: Error | (import('../server/page/types').SerializedHttpError); - * node_ids: number[]; - * params: Record; - * routeId: string | null; - * data: Array; - * errors: Record | null; - * }; + * hydrate: Parameters[0]; * paths: { * assets: string; * base: string; diff --git a/packages/kit/src/runtime/client/types.d.ts b/packages/kit/src/runtime/client/types.d.ts index bdd90bafffd2..c07b28743f2e 100644 --- a/packages/kit/src/runtime/client/types.d.ts +++ b/packages/kit/src/runtime/client/types.d.ts @@ -6,7 +6,7 @@ import { prefetch, prefetchRoutes } from '$app/navigation'; -import { CSRPageNode, CSRPageNodeLoader, CSRRoute, Uses } from 'types'; +import { CSRPageNode, CSRPageNodeLoader, CSRRoute, FormState, Uses } from 'types'; import { HttpError } from '../control.js'; import { SerializedHttpError } from '../server/page/types.js'; @@ -28,8 +28,7 @@ export interface Client { params: Record; routeId: string | null; data: Array; - errors: Record | null; - values: Record | null; + form: FormState | null; }) => Promise; _start_router: () => void; } diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index e31fe75abd81..c1f0e5c8350d 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -26,8 +26,8 @@ export function is_action_json_request(event) { * @param {import('types').SSRNode['server']} server */ export async function handle_action_json_request(event, options, server) { - // TODO create POJO interface for this with - // {status: number, errors?: Record, values?: Record, result?: Record} + // TODO create POJO interface for this? Something like + // {status: number, errors?: Record, location?: string, values?: Record, result?: Record} const handler = server.actions; if (!handler) { diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index fcc87b6bb6b5..37e902162059 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -105,8 +105,7 @@ export async function render_response({ }; if (validation_errors) { - props.errors = validation_errors.errors; - props.values = validation_errors.values; + props.form = { errors: validation_errors.errors, values: validation_errors.values }; } // TODO remove this for 1.0 @@ -176,7 +175,7 @@ export async function render_response({ /** @param {string} path */ const prefixed = (path) => (path.startsWith('/') ? path : `${assets}/${path}`); - const serialized = { data: '', errors: 'null', values: 'null' }; + const serialized = { data: '', form: 'null' }; try { serialized.data = devalue(branch.map(({ server_data }) => server_data)); @@ -192,8 +191,10 @@ export async function render_response({ if (validation_errors) { try { - serialized.errors = devalue(validation_errors.errors); - serialized.values = devalue(validation_errors.values); + serialized.form = devalue({ + errors: validation_errors.errors, + values: validation_errors.values + }); } catch (e) { // If we're here, the data could not be serialized with devalue const error = /** @type {any} */ (e); @@ -215,8 +216,7 @@ export async function render_response({ params: ${devalue(event.params)}, routeId: ${s(event.routeId)}, data: ${serialized.data}, - errors: ${serialized.errors}, - values: ${serialized.values}, + form: ${serialized.form} }` : 'null'}, paths: ${s(options.paths)}, target: document.querySelector('[data-sveltekit-hydrate="${target}"]').parentNode, diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte index 28f21b6f1990..0f1e8c32a9a8 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte @@ -2,8 +2,6 @@ import { browser } from '$app/environment'; import { form } from '$app/stores'; - export let errors - $: hydrated_form_values = browser ? $form?.values : ''; async function submit() { diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte index 9ca9d4396bb2..17528838daed 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte @@ -1,16 +1,15 @@ -

{errors?.message}

+

{$form?.errors?.message}

{#if hydrated_error_message} diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte index 246542429da8..be3156b44683 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte @@ -1,12 +1,9 @@ -

{data.get_message} / {errors.post_message}

+

{data.get_message} / {$form?.errors?.post_message}

status: {$page.status}

diff --git a/packages/kit/types/ambient.d.ts b/packages/kit/types/ambient.d.ts index d5ef1a47855b..7cdcb72da452 100644 --- a/packages/kit/types/ambient.d.ts +++ b/packages/kit/types/ambient.d.ts @@ -186,7 +186,7 @@ declare module '$app/paths' { */ declare module '$app/stores' { import { Readable, Writable } from 'svelte/store'; - import { Navigation, Page } from '@sveltejs/kit'; + import { FormState, Navigation, Page } from '@sveltejs/kit'; /** * A readable store whose value contains page data. @@ -206,7 +206,7 @@ declare module '$app/stores' { * A writable store whose value contain the errors and values of the last form submission. * It is updated automatically when using the `
` component, else you need to take care of it yourself. */ - export const form: Writable<{ errors: Record; values: Record }>; + export const form: Writable; /** * A function that returns all of the contextual stores. On the server, this must be called during component initialization. diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 210c86105905..ca962e755775 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -315,6 +315,12 @@ export interface Action< (event: RequestEvent): MaybePromise | void>; } +export interface FormState extends Partial> { + error?: Record; + values?: Record; + result?: Record; +} + // TODO figure out how to just re-export from '../src/index/index.js' without // breaking the site From d8800965d268fc1f1bd84f91e57d41f7bb75fcfe Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 31 Aug 2022 22:00:56 +0200 Subject: [PATCH 04/85] make actions an object --- .../kit/src/core/sync/write_types/index.js | 1 + packages/kit/src/exports/node/polyfills.js | 5 +- packages/kit/src/runtime/control.js | 3 +- .../kit/src/runtime/server/page/actions.js | 84 +++++++++++++++++-- .../+page.server.js | 11 +-- .../actions/form-errors/+page.server.js | 7 +- .../post-explicit/+page.server.js | 7 +- .../post-implicit/+page.server.js | 7 +- .../shadowed/error-post/+page.server.js | 15 ++-- .../shadowed/missing-get/+page.server.js | 5 +- .../routes/shadowed/no-get/+page.server.js | 9 +- .../post-success-redirect/+page.server.js | 9 +- .../redirect-post-with-cookie/+page.server.js | 11 ++- .../shadowed/redirect-post/+page.server.js | 9 +- .../shadowed/simple/post/+page.server.js | 6 +- .../kit/test/apps/basics/test/server.test.js | 1 + packages/kit/test/apps/basics/test/test.js | 2 +- .../src/routes/shadowed-post/+page.server.js | 5 +- packages/kit/types/index.d.ts | 47 ++++++++++- packages/kit/types/internal.d.ts | 5 +- 20 files changed, 200 insertions(+), 49 deletions(-) diff --git a/packages/kit/src/core/sync/write_types/index.js b/packages/kit/src/core/sync/write_types/index.js index 9bfb8febc281..05e095abbd3d 100644 --- a/packages/kit/src/core/sync/write_types/index.js +++ b/packages/kit/src/core/sync/write_types/index.js @@ -193,6 +193,7 @@ function update_types(config, routes, route) { if (route.leaf.server) { exports.push(`export type Action = Kit.Action`); + exports.push(`export type Actions = Kit.Actions`); } } diff --git a/packages/kit/src/exports/node/polyfills.js b/packages/kit/src/exports/node/polyfills.js index 0e7481441e62..25e3fd5ebc90 100644 --- a/packages/kit/src/exports/node/polyfills.js +++ b/packages/kit/src/exports/node/polyfills.js @@ -1,7 +1,7 @@ import { fetch, Response, Request, Headers } from 'undici'; import { ReadableStream, TransformStream, WritableStream } from 'stream/web'; import { Readable } from 'stream'; -import { Request as NodeFetchRequest } from 'node-fetch'; +import { Request as NodeFetchRequest, FormData } from 'node-fetch'; import { webcrypto as crypto } from 'crypto'; /** @type {Record} */ @@ -24,7 +24,8 @@ const globals = { Headers, ReadableStream, TransformStream, - WritableStream + WritableStream, + FormData }; // exported for dev/preview and node environments diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index 8140486b373e..7b5d2b571b86 100644 --- a/packages/kit/src/runtime/control.js +++ b/packages/kit/src/runtime/control.js @@ -39,8 +39,7 @@ export class ValidationError { * @param {FormData | Record | null | undefined} [values] */ constructor(status, errors, values) { - // NodeJS doesn't have a FormData class, so we have to check it differently - if (typeof values?.get === 'function') { + if (values instanceof FormData) { /** @type {Record} */ const converted = {}; for (const [key, value] of values.entries()) { diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index c1f0e5c8350d..b48e6f9a005e 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -28,9 +28,9 @@ export function is_action_json_request(event) { export async function handle_action_json_request(event, options, server) { // TODO create POJO interface for this? Something like // {status: number, errors?: Record, location?: string, values?: Record, result?: Record} - const handler = server.actions; + const actions = server.actions; - if (!handler) { + if (!actions) { maybe_throw_migration_error(server); // TODO should this be a different error altogether? return new Response('POST method not allowed. No actions exist for this page', { @@ -44,7 +44,7 @@ export async function handle_action_json_request(event, options, server) { } try { - const result = await handler.call(null, event); + const result = await call_action(event, actions); if (!result) { // TODO json({status: 204}) instead? return new Response(undefined, { @@ -97,9 +97,9 @@ export function is_action_request(event, leaf_node) { * @throws {Redirect | ValidationError | HttpError | Error} */ export async function handle_action_request(event, server) { - const handler = server.actions; + const actions = server.actions; - if (!handler) { + if (!actions) { maybe_throw_migration_error(server); // TODO should this be a different error altogether? event.setHeaders({ @@ -110,7 +110,47 @@ export async function handle_action_request(event, server) { throw error(405, 'POST method not allowed. No actions exist for this page'); } - return await handler.call(null, event); + return call_action(event, actions); +} + +/** + * @param {import('types').RequestEvent} event + * @param {NonNullable} actions + * @throws {Redirect | ValidationError | HttpError | Error} + */ +export async function call_action(event, actions) { + const url = new URL(event.request.url); + + let name = 'default'; + for (const param of url.searchParams) { + throw param; + if (param[0].startsWith('/')) { + name = param[0].slice(1); + break; + } + } + + const action = actions[name]; + if (!action) { + throw new Error(`No action with name '${name}' found`); + } + + if (event.request.headers.get('content-type') === 'application/json') { + throw new Error('Actions expect form-encoded data, JSON is not supported'); + } + + const form = await event.request.formData(); + const fields = new FormData(); + const files = new FilesFormData(); + for (const [key, value] of form) { + if (typeof value === 'string') { + fields.append(key, value); + } else { + files.append(key, value); + } + } + + return action({ ...event, fields, files }); } /** @@ -123,3 +163,35 @@ function maybe_throw_migration_error(server) { } } } + +/** @typedef {import('types').FilesFormData} FFD */ + +/** @implements {FFD} */ +export class FilesFormData extends Map { + constructor() { + super([]); + } + // @ts-ignore + set(key, file) { + super.set(key, [file]); + } + // @ts-ignore + get(key) { + return super.get(key)[0]; + } + // @ts-ignore + append(key, value) { + const files = super.get(key) || []; + files.push(value); + super.set(key, files); + } + // @ts-ignore + getAll(key) { + return super.get(key) || []; + } + // @ts-ignore + forEach(callback) { + super.forEach((value, key) => callback(value, key, this)); + } + // TODO iteration is wrong because FormData returns entries with multiple values each as a [key, value] pair +} diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js index 632f1a436519..079be981af08 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js @@ -1,10 +1,11 @@ import { validation } from '@sveltejs/kit'; /** - * @type {import('./$types').Action} + * @type {import('./$types').Actions} */ -export const actions = async ({ request }) => { - const data = await request.formData(); - data.delete('password'); - throw validation(400, { message: 'invalid credentials' }, data); +export const actions = { + default: async ({ fields }) => { + fields.delete('password'); + throw validation(400, { message: 'invalid credentials' }, fields); + } }; diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js index 9c710f75cd9d..bc27dda83467 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js @@ -1,5 +1,8 @@ import { validation } from '@sveltejs/kit'; -export const actions = async () => { - throw validation(400, { message: 'an error occurred' }); +/** @type {import('./$types').Actions} */ +export const actions = { + default: async () => { + throw validation(400, { message: 'an error occurred' }); + } }; diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit/+page.server.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit/+page.server.js index 06d7d4c68648..56ccc82ff691 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-explicit/+page.server.js @@ -1,5 +1,8 @@ import { error } from '@sveltejs/kit'; -export const actions = () => { - throw error(400, 'oops'); +/** @type {import('./$types').Actions} */ +export const actions = { + default: () => { + throw error(400, 'oops'); + } }; diff --git a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit/+page.server.js b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit/+page.server.js index 52fdfea67b73..fe1fc1d78fc8 100644 --- a/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/errors/page-endpoint/post-implicit/+page.server.js @@ -1,5 +1,8 @@ import { FancyError } from '../_shared.js'; -export const actions = () => { - throw new FancyError('oops'); +/** @type {import('./$types').Actions} */ +export const actions = { + default: () => { + throw new FancyError('oops'); + } }; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js index 73af9f7a128a..efbc57eb84a2 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js @@ -6,10 +6,11 @@ export function load() { }; } -export async function actions({ request }) { - const data = await request.formData(); - - throw validation(400, { - post_message: `echo: ${data.get('message')}` - }); -} +/** @type {import('./$types').Actions} */ +export const actions = { + default: async ({ fields }) => { + throw validation(400, { + post_message: `echo: ${fields.get('message')}` + }); + } +}; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/missing-get/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/missing-get/+page.server.js index b0492ae1be04..2f3f537b31bb 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/missing-get/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/missing-get/+page.server.js @@ -1 +1,4 @@ -export const actions = () => {}; +/** @type {import('./$types').Actions} */ +export const actions = { + default: () => {} +}; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/no-get/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/no-get/+page.server.js index 7a665e58770c..14f357bfb176 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/no-get/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/no-get/+page.server.js @@ -1,3 +1,6 @@ -export function actions() { - return {}; -} +/** @type {import('./$types').Actions} */ +export const actions = { + default: () => { + return {}; + } +}; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/post-success-redirect/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/post-success-redirect/+page.server.js index 715b6f7d5fcd..ac6e97a468fc 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/post-success-redirect/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/post-success-redirect/+page.server.js @@ -1,5 +1,8 @@ import { redirect } from '@sveltejs/kit'; -export function actions() { - throw redirect(303, '/shadowed/post-success-redirect/redirected'); -} +/** @type {import('./$types').Actions} */ +export const actions = { + default: () => { + throw redirect(303, '/shadowed/post-success-redirect/redirected'); + } +}; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post-with-cookie/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post-with-cookie/+page.server.js index cc6d024f551f..a35cb6812e49 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post-with-cookie/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post-with-cookie/+page.server.js @@ -1,6 +1,9 @@ import { redirect } from '@sveltejs/kit'; -export function actions({ setHeaders }) { - setHeaders({ 'set-cookie': 'shadow-redirect=happy' }); - throw redirect(302, '/shadowed/redirected'); -} +/** @type {import('./$types').Actions} */ +export const actions = { + default: ({ setHeaders }) => { + setHeaders({ 'set-cookie': 'shadow-redirect=happy' }); + throw redirect(302, '/shadowed/redirected'); + } +}; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post/+page.server.js index 132a628414b4..c152db3348d3 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-post/+page.server.js @@ -1,5 +1,8 @@ import { redirect } from '@sveltejs/kit'; -export function actions() { - throw redirect(302, '/shadowed/redirected'); -} +/** @type {import('./$types').Actions} */ +export const actions = { + default: () => { + throw redirect(302, '/shadowed/redirected'); + } +}; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/simple/post/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/simple/post/+page.server.js index a9a749929336..2f3f537b31bb 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/simple/post/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/simple/post/+page.server.js @@ -1,2 +1,4 @@ -/** @type {import('./$types').Action} */ -export function actions() {} +/** @type {import('./$types').Actions} */ +export const actions = { + default: () => {} +}; diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index 1cafac3badb6..d992a6b6682e 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -199,6 +199,7 @@ test.describe('Routing', () => { test.describe('Shadowed pages', () => { test('Action can return undefined', async ({ request }) => { const response = await request.post('/shadowed/simple/post', { + form: {}, headers: { accept: 'application/json' } diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index e18818fd3c1a..e4a07065d7da 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -621,7 +621,7 @@ test.describe('Errors', () => { if (process.env.DEV) { const lines = stack.split('\n'); - expect(lines[1]).toContain('+page.server.js:4:8'); + expect(lines[1]).toContain('+page.server.js:6:9'); } const error = read_errors('/errors/page-endpoint/post-implicit'); diff --git a/packages/kit/test/prerendering/basics/src/routes/shadowed-post/+page.server.js b/packages/kit/test/prerendering/basics/src/routes/shadowed-post/+page.server.js index 62159134ad87..8f9a2be06223 100644 --- a/packages/kit/test/prerendering/basics/src/routes/shadowed-post/+page.server.js +++ b/packages/kit/test/prerendering/basics/src/routes/shadowed-post/+page.server.js @@ -4,4 +4,7 @@ export function load() { }; } -export function actions() {} +/** @type {import('./$types').Actions} */ +export const actions = { + default: () => {} +}; diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index ca962e755775..47f85d30fb16 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -312,7 +312,52 @@ export interface ServerLoadEvent< export interface Action< Params extends Partial> = Partial> > { - (event: RequestEvent): MaybePromise | void>; + (event: ActionEvent): MaybePromise | void>; +} + +export type Actions< + Params extends Partial> = Partial> +> = Record>; + +export interface ActionEvent< + Params extends Partial> = Partial> +> extends RequestEvent { + fields: FieldsFormData; + files: FilesFormData; +} + +export class FieldsFormData extends FormData { + [Symbol.iterator](): IterableIterator<[Fields, string]>; + entries(): IterableIterator<[Fields, string]>; + keys(): IterableIterator; + values(): IterableIterator; + + delete(name: Fields): void; + get(name: Fields): string | null; + getAll(name: Fields): string[]; + has(name: Fields): boolean; + set(name: string, value: string): void; + forEach( + callbackfn: (value: string, key: string, parent: FieldsFormData) => void, + thisArg?: any + ): void; +} + +export class FilesFormData { + [Symbol.iterator](): IterableIterator<[Fields, FileFileType]>; + entries(): IterableIterator<[Fields, FileFileType]>; + keys(): IterableIterator; + values(): IterableIterator; + + delete(name: Fields): void; + get(name: Fields): FileFileType | null; + getAll(name: Fields): FileFileType[]; + has(name: Fields): boolean; + set(name: string, value: string | Blob, fileName?: string): void; + forEach( + callbackfn: (value: FileFileType, key: string, parent: FilesFormData) => void, + thisArg?: any + ): void; } export interface FormState extends Partial> { diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts index 17d3d12a231c..4617b314d012 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -14,7 +14,8 @@ import { ResolveOptions, Server, ServerInitOptions, - SSRManifest + SSRManifest, + Actions } from './index.js'; import { HttpMethod, @@ -280,7 +281,7 @@ export interface SSRNode { prerender?: PrerenderOption; ssr?: boolean; csr?: boolean; - actions?: Action; + actions?: Actions; }; // store this in dev so we can print serialization errors From 102851b6c14276ff8a357164844a721bedc253d3 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 12:49:40 +0200 Subject: [PATCH 05/85] implement FilesFormData --- .../kit/src/runtime/server/page/actions.js | 120 +++++++++++++++--- packages/kit/types/index.d.ts | 5 +- 2 files changed, 102 insertions(+), 23 deletions(-) diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index b48e6f9a005e..4a39bacbaf1e 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -1,3 +1,4 @@ +import { FieldsFormData } from 'types'; import { error, json } from '../../../exports/index.js'; import { negotiate } from '../../../utils/http.js'; import { HttpError, Redirect, ValidationError } from '../../control.js'; @@ -123,7 +124,6 @@ export async function call_action(event, actions) { let name = 'default'; for (const param of url.searchParams) { - throw param; if (param[0].startsWith('/')) { name = param[0].slice(1); break; @@ -150,7 +150,7 @@ export async function call_action(event, actions) { } } - return action({ ...event, fields, files }); + return action({ ...event, fields: /** @type {FieldsFormData} */ (fields), files }); } /** @@ -164,34 +164,116 @@ function maybe_throw_migration_error(server) { } } -/** @typedef {import('types').FilesFormData} FFD */ +/** + * (written like this because JSDoc doesn't support import syntax in `@implements`) + * @typedef {import('types').FilesFormData} FFD + */ /** @implements {FFD} */ -export class FilesFormData extends Map { +export class FilesFormData { constructor() { - super([]); + /** @type {Map} */ + this.map = new Map(); } - // @ts-ignore + /** + * @param {string} key + * @param {any} file + */ set(key, file) { - super.set(key, [file]); + this.map.set(key, [file]); } - // @ts-ignore + /** + * @param {string} key + */ get(key) { - return super.get(key)[0]; + return this.map.get(key)?.[0]; + } + /** + * @param {string} key + * @param {any} file + */ + append(key, file) { + const files = this.map.get(key) || []; + files.push(file); + this.map.set(key, files); } - // @ts-ignore - append(key, value) { - const files = super.get(key) || []; - files.push(value); - super.set(key, files); + /** + * @param {string} key + */ + delete(key) { + this.map.delete(key); } - // @ts-ignore + /** + * @param {string} key + */ + has(key) { + return this.map.has(key); + } + /** + * @param {string} key + */ getAll(key) { - return super.get(key) || []; + return this.map.get(key) || []; } - // @ts-ignore + /** + * @param {Parameters[0]} callback + */ forEach(callback) { - super.forEach((value, key) => callback(value, key, this)); + this.map.forEach((values, key) => values.forEach((value) => callback(value, key, this))); + } + entries() { + return new Iterator(this.map, true, true); + } + keys() { + return new Iterator(this.map, true, false); + } + values() { + return new Iterator(this.map, false, true); + } + + [Symbol.iterator]() { + return new Iterator(this.map, true, true); + } +} + +/** @implements {IterableIterator} */ +class Iterator { + /** + * @param {Map} map + * @param {boolean} key + * @param {boolean} value + * */ + constructor(map, key, value) { + this.key = key; + this.value = value; + this.map = map; + this.entries = this.map.entries(); + this.index = 0; + this.current = undefined; + } + next() { + this.current = this.current || this.entries.next(); + if (this.current.done) { + return { value: undefined, done: true }; + } + + const value = + this.key && this.value + ? [this.current.value[0], this.current.value[1][this.index]] + : this.key + ? this.current.value[0] + : this.current.value[1][this.index]; + const next = { value, done: false }; + + this.index++; + if (this.index >= this.current.length) { + this.index = 0; + this.current = undefined; + } + + return next; + } + [Symbol.iterator]() { + return this; } - // TODO iteration is wrong because FormData returns entries with multiple values each as a [key, value] pair } diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 47f85d30fb16..d4670612ccbe 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -354,10 +354,7 @@ export class FilesFormData getAll(name: Fields): FileFileType[]; has(name: Fields): boolean; set(name: string, value: string | Blob, fileName?: string): void; - forEach( - callbackfn: (value: FileFileType, key: string, parent: FilesFormData) => void, - thisArg?: any - ): void; + forEach(callbackfn: (value: FileFileType, key: string, parent: FilesFormData) => void): void; } export interface FormState extends Partial> { From e650cb3846c7d7eb90d0728b61b62e75ab624e01 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 13:06:38 +0200 Subject: [PATCH 06/85] restrict values type, fix conversion, switch argument order --- packages/kit/src/exports/index.js | 6 +++--- packages/kit/src/runtime/control.js | 16 ++++++++++++---- packages/kit/src/runtime/server/page/actions.js | 8 +++++--- .../form-errors-persist-fields/+page.server.js | 2 +- .../routes/actions/form-errors/+page.server.js | 2 +- .../routes/shadowed/error-post/+page.server.js | 2 +- packages/kit/types/index.d.ts | 4 ++-- 7 files changed, 25 insertions(+), 15 deletions(-) diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index 22490b1b74ca..4d7358c4c5d4 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -47,9 +47,9 @@ export function json(data, init) { /** * Generates a `ValidationError` object. * @param {number} status + * @param {FormData | Record | null | undefined} values * @param {Record} errors - * @param {FormData} [form] */ -export function validation(status, errors, form) { - return new ValidationError(status, errors, form); +export function validation(status, values, errors) { + return new ValidationError(status, values, errors); } diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index 7b5d2b571b86..b199e29cd661 100644 --- a/packages/kit/src/runtime/control.js +++ b/packages/kit/src/runtime/control.js @@ -35,16 +35,24 @@ export class Redirect { export class ValidationError { /** * @param {number} status + * @param {FormData | Record | null | undefined} values * @param {Record} errors - * @param {FormData | Record | null | undefined} [values] */ - constructor(status, errors, values) { + constructor(status, values, errors) { if (values instanceof FormData) { - /** @type {Record} */ + /** @type {Record} */ const converted = {}; for (const [key, value] of values.entries()) { if (typeof value === 'string') { - converted[key] = value; + if (converted[key] !== undefined) { + const array = /** @type {string[]} */ ( + Array.isArray(converted[key]) ? converted[key] : [converted[key]] + ); + array.push(value); + converted[key] = array; + } else { + converted[key] = value; + } } } this.values = converted; diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 4a39bacbaf1e..22fdd0d4b297 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -1,4 +1,3 @@ -import { FieldsFormData } from 'types'; import { error, json } from '../../../exports/index.js'; import { negotiate } from '../../../utils/http.js'; import { HttpError, Redirect, ValidationError } from '../../control.js'; @@ -8,7 +7,6 @@ import { error_to_pojo } from '../utils.js'; * @param {import('types').RequestEvent} event */ export function is_action_json_request(event) { - console.log(event.request.headers.get('accept')); const accept = negotiate(event.request.headers.get('accept') || 'text/html', [ 'text/html', 'application/json' @@ -150,7 +148,11 @@ export async function call_action(event, actions) { } } - return action({ ...event, fields: /** @type {FieldsFormData} */ (fields), files }); + return action({ + ...event, + fields: /** @type {import('types').FieldsFormData} */ (fields), + files + }); } /** diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js index 079be981af08..73fc45076450 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js @@ -6,6 +6,6 @@ import { validation } from '@sveltejs/kit'; export const actions = { default: async ({ fields }) => { fields.delete('password'); - throw validation(400, { message: 'invalid credentials' }, fields); + throw validation(400, fields, { message: 'invalid credentials' }); } }; diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js index bc27dda83467..af8f61d086e5 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js @@ -3,6 +3,6 @@ import { validation } from '@sveltejs/kit'; /** @type {import('./$types').Actions} */ export const actions = { default: async () => { - throw validation(400, { message: 'an error occurred' }); + throw validation(400, undefined, { message: 'an error occurred' }); } }; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js index efbc57eb84a2..15f8d4baf82b 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js @@ -9,7 +9,7 @@ export function load() { /** @type {import('./$types').Actions} */ export const actions = { default: async ({ fields }) => { - throw validation(400, { + throw validation(400, undefined, { post_message: `echo: ${fields.get('message')}` }); } diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index d4670612ccbe..1cd1b0eafb62 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -391,6 +391,6 @@ export function json(data: any, init?: ResponseInit): Response; */ export function validation( status: number, - errors: Record, - form?: FormData + form: FormData | Record | undefined | null, + errors: Record ): ValidationError; From 5ece9c00b0c5800dc70b967d2c1b3f81df8f4ce8 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 13:10:39 +0200 Subject: [PATCH 07/85] validation->invalid --- packages/kit/src/exports/index.js | 2 +- .../routes/actions/form-errors-persist-fields/+page.server.js | 4 ++-- .../basics/src/routes/actions/form-errors/+page.server.js | 4 ++-- .../basics/src/routes/shadowed/error-post/+page.server.js | 4 ++-- packages/kit/types/index.d.ts | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index 4d7358c4c5d4..521943b25264 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -50,6 +50,6 @@ export function json(data, init) { * @param {FormData | Record | null | undefined} values * @param {Record} errors */ -export function validation(status, values, errors) { +export function invalid(status, values, errors) { return new ValidationError(status, values, errors); } diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js index 73fc45076450..21eba4744cc7 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js @@ -1,4 +1,4 @@ -import { validation } from '@sveltejs/kit'; +import { invalid } from '@sveltejs/kit'; /** * @type {import('./$types').Actions} @@ -6,6 +6,6 @@ import { validation } from '@sveltejs/kit'; export const actions = { default: async ({ fields }) => { fields.delete('password'); - throw validation(400, fields, { message: 'invalid credentials' }); + throw invalid(400, fields, { message: 'invalid credentials' }); } }; diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js index af8f61d086e5..ebe1021a6bba 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js @@ -1,8 +1,8 @@ -import { validation } from '@sveltejs/kit'; +import { invalid } from '@sveltejs/kit'; /** @type {import('./$types').Actions} */ export const actions = { default: async () => { - throw validation(400, undefined, { message: 'an error occurred' }); + throw invalid(400, undefined, { message: 'an error occurred' }); } }; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js index 15f8d4baf82b..690a22f31829 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js @@ -1,4 +1,4 @@ -import { validation } from '@sveltejs/kit'; +import { invalid } from '@sveltejs/kit'; export function load() { return { @@ -9,7 +9,7 @@ export function load() { /** @type {import('./$types').Actions} */ export const actions = { default: async ({ fields }) => { - throw validation(400, undefined, { + throw invalid(400, undefined, { post_message: `echo: ${fields.get('message')}` }); } diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 1cd1b0eafb62..47b4f763bb28 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -389,7 +389,7 @@ export function json(data: any, init?: ResponseInit): Response; /** * Generates a `ValidationError` object. */ -export function validation( +export function invalid( status: number, form: FormData | Record | undefined | null, errors: Record From 9c9dfbe917991776844e59fa1a27018ed9198f00 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 14:32:36 +0200 Subject: [PATCH 08/85] start writing docs --- documentation/docs/03-routing.md | 67 +----- documentation/docs/06-form-actions.md | 215 ++++++++++++++++++ .../docs/{06-hooks.md => 07-hooks.md} | 0 .../docs/{07-modules.md => 08-modules.md} | 0 ...rvice-workers.md => 09-service-workers.md} | 0 ...{09-link-options.md => 10-link-options.md} | 0 .../docs/{10-adapters.md => 11-adapters.md} | 0 ...{11-page-options.md => 12-page-options.md} | 0 .../docs/{12-packaging.md => 13-packaging.md} | 0 documentation/docs/{13-cli.md => 14-cli.md} | 0 ...4-configuration.md => 15-configuration.md} | 0 .../docs/{15-types.md => 16-types.md} | 0 documentation/docs/{16-seo.md => 17-seo.md} | 0 .../docs/{17-assets.md => 18-assets.md} | 0 ...8-accessibility.md => 19-accessibility.md} | 0 15 files changed, 216 insertions(+), 66 deletions(-) create mode 100644 documentation/docs/06-form-actions.md rename documentation/docs/{06-hooks.md => 07-hooks.md} (100%) rename documentation/docs/{07-modules.md => 08-modules.md} (100%) rename documentation/docs/{08-service-workers.md => 09-service-workers.md} (100%) rename documentation/docs/{09-link-options.md => 10-link-options.md} (100%) rename documentation/docs/{10-adapters.md => 11-adapters.md} (100%) rename documentation/docs/{11-page-options.md => 12-page-options.md} (100%) rename documentation/docs/{12-packaging.md => 13-packaging.md} (100%) rename documentation/docs/{13-cli.md => 14-cli.md} (100%) rename documentation/docs/{14-configuration.md => 15-configuration.md} (100%) rename documentation/docs/{15-types.md => 16-types.md} (100%) rename documentation/docs/{16-seo.md => 17-seo.md} (100%) rename documentation/docs/{17-assets.md => 18-assets.md} (100%) rename documentation/docs/{18-accessibility.md => 19-accessibility.md} (100%) diff --git a/documentation/docs/03-routing.md b/documentation/docs/03-routing.md index 92f6f270e9f6..045afbfee757 100644 --- a/documentation/docs/03-routing.md +++ b/documentation/docs/03-routing.md @@ -114,72 +114,7 @@ Like `+page.js`, `+page.server.js` can export [page options](/docs/page-options) #### Actions -`+page.server.js` can also declare _actions_, which correspond to the `POST`, `PATCH`, `PUT` and `DELETE` HTTP methods. A request made to the page with one of these methods will invoke the corresponding action before rendering the page. - -An action can return a `{ status?, errors }` object if there are validation errors (`status` defaults to `400`), or an optional `{ location }` object to redirect the user to another page: - -```js -/// file: src/routes/login/+page.server.js - -// @filename: ambient.d.ts -declare global { - const createSessionCookie: (userid: string) => string; - const hash: (content: string) => string; - const db: { - findUser: (name: string) => Promise<{ - id: string; - username: string; - password: string; - }> - } -} - -export {}; - -// @filename: index.js -// ---cut--- -import { error } from '@sveltejs/kit'; - -/** @type {import('./$types').Action} */ -export async function POST({ request, setHeaders, url }) { - const values = await request.formData(); - - const username = /** @type {string} */ (values.get('username')); - const password = /** @type {string} */ (values.get('password')); - - const user = await db.findUser(username); - - if (!user) { - return { - status: 403, - errors: { - username: 'No user with this username' - } - }; - } - - if (user.password !== hash(password)) { - return { - status: 403, - errors: { - password: 'Incorrect password' - } - }; - } - - setHeaders({ - 'set-cookie': createSessionCookie(user.id) - }); - - return { - location: url.searchParams.get('redirectTo') ?? '/' - }; -} -``` - -If validation `errors` are returned, they will be available inside `+page.svelte` as `export let errors`. - -> The actions API will likely change in the near future: https://github.com/sveltejs/kit/discussions/5875 +`+page.server.js` can also declare _actions_ which are specifically designed for form interactions. It enables things like preserving user input in case of a full page reload with validation errors while making progressive enhancement through JavaScript possible. You can learn more about them in [form actions](/docs/form-actions). ### +error diff --git a/documentation/docs/06-form-actions.md b/documentation/docs/06-form-actions.md new file mode 100644 index 000000000000..95e8331f45fc --- /dev/null +++ b/documentation/docs/06-form-actions.md @@ -0,0 +1,215 @@ +--- +title: Form Actions +--- + +`+page.server.js` can declare _actions_ which are specifically designed for form interactions. It enables things like preserving user input in case of a full page reload with validation errors while making progressive enhancement through JavaScript possible. + +## Defining actions by name + +Actions are defined through `export const actions = {...}`, with each key being the name of the action and the value being the function that is invoked when the form with that action is submitted. A `POST` request made to the page will invoke the corresponding action using a query parameter that start's with a `/` - so for example `POST todos?/addTodo` will invoke the `addTodo` action. The `default` action is called when no such query parameter is given. + +```svelte +/// file: src/routes/todos/+page.svelte + + + + + + + +
    + {#each data.todos as todo} +
  • +
    + + + +
    +
  • + {/each} +
+``` + +```js +/// file: src/routes/todos/+page.server.js +/** @type {import('./$type').Actions} */ +export const actions = { + addTodo: (event) => { + // ... + }, + editTodo: (event) => { + // ... + } +}; +``` + +## Files and strings are separated + +Since `actions` are meant to be used with forms, we can make your life easier by awaiting the `FormData` and separating the form fields which contain strings from those who contain files and running the latter through the `handleFile` hook before passing it as `fields` and `files` into the action function. + +```js +/// file: src/routes/todos/+page.server.js +/** @type {import('./$type').Actions} */ +export const actions = { + default: ({ fields, files }) => { + const name = fields.get('name'); // typed as string + const image = files.get('image'); // typed as the return type of the handleFile hook + // ... + } +}; +``` + +## Validation + +A core part of form submissions is validation. For this, an action can `throw` the `invalid` helper method exported from `@sveltejs/kit` if there are validation errors. `invalid` expects a `status`, possibly the form `values` (make sure to remove any user sensitive information such as passwords) and an `error` object. In case of a native form submit they populate the `$form` store which is available inside your components so you can preserve user input. + +```js +/// file: src/routes/login/+page.server.js + +// @filename: ambient.d.ts +declare global { + const db: { + findUser: (name: string) => Promise<{ + id: string; + username: string; + password: string; + }> + } +} + +export {}; + +// @filename: index.js +// ---cut--- +import { invalid } from '@sveltejs/kit'; + +/** @type {import('./$types').Actions} */ +export const actions = { + default: async ({ fields, setHeaders, url }) => { + const username = fields.get('username'); + const password = fields.get('password'); + + const user = await db.findUser(username); + + if (!user) { + throw invalid(403, { username }, { + username: 'No user with this username' + }); + } + + // ... + } +}; +``` + +```svelte +/// file: src/routes/login/+page.svelte + + +
+ + {#if $form?.errors?.username} + {$form?.errors?.username} + {/if} + + +
+``` + +## Success + +If everything is valid, an action can either return a JSON object with data that is merged with the returned data from `load`, or it can `throw` a `redirect` to redirect the user to another page. + +```js +/// file: src/routes/login/+page.server.js +import { redirect } from '@sveltejs/kit'; + +/** @type {import('./$types').Actions} */ +export const actions = { + default: async ({ url }) => { + // ... + + if (url.searchParams.get('redirectTo')) { + throw redirect(303, url.searchParams.get('redirectTo')); + } else { + return { + success: true + }; + } + } +}; +``` + +## Progressive enhancement + +So far, all the code examples run native form submissions - that is, when the user pressed the submit button, the page is reloaded. It's good that this use case is supported since JavaScript may not be loaded all the time. When it is though, it might be a better user experience to use the powers JavaScript gives us to provide a better user experience - this is called progressive enhancement. + +First we need to ensure that the page is _not_ reloaded on submission. For this, we prevent the default behavior. Afterwards, we run our JavaScript code instead which does the form submission through `fetch` instead. + +```svelte +/// file: src/routes/login/+page.svelte + + +
+ + {#if $form?.errors?.username} + {$form?.errors?.username} + {/if} + + +
+``` + +## `
` component + +As you can see, progressive enhancement is doable, but it may become a little cumbersome over time. That's we provide an opinionated wrapper component which does all the heavy lifting for you. Here is the same login page using the `` component from `@sveltejs/kit`: + +```svelte +/// file: src/routes/login/+page.svelte + + + + + {#if errors?.username} + {errors?.username} + {/if} + + +
+``` + +TODO explain the API in more detail diff --git a/documentation/docs/06-hooks.md b/documentation/docs/07-hooks.md similarity index 100% rename from documentation/docs/06-hooks.md rename to documentation/docs/07-hooks.md diff --git a/documentation/docs/07-modules.md b/documentation/docs/08-modules.md similarity index 100% rename from documentation/docs/07-modules.md rename to documentation/docs/08-modules.md diff --git a/documentation/docs/08-service-workers.md b/documentation/docs/09-service-workers.md similarity index 100% rename from documentation/docs/08-service-workers.md rename to documentation/docs/09-service-workers.md diff --git a/documentation/docs/09-link-options.md b/documentation/docs/10-link-options.md similarity index 100% rename from documentation/docs/09-link-options.md rename to documentation/docs/10-link-options.md diff --git a/documentation/docs/10-adapters.md b/documentation/docs/11-adapters.md similarity index 100% rename from documentation/docs/10-adapters.md rename to documentation/docs/11-adapters.md diff --git a/documentation/docs/11-page-options.md b/documentation/docs/12-page-options.md similarity index 100% rename from documentation/docs/11-page-options.md rename to documentation/docs/12-page-options.md diff --git a/documentation/docs/12-packaging.md b/documentation/docs/13-packaging.md similarity index 100% rename from documentation/docs/12-packaging.md rename to documentation/docs/13-packaging.md diff --git a/documentation/docs/13-cli.md b/documentation/docs/14-cli.md similarity index 100% rename from documentation/docs/13-cli.md rename to documentation/docs/14-cli.md diff --git a/documentation/docs/14-configuration.md b/documentation/docs/15-configuration.md similarity index 100% rename from documentation/docs/14-configuration.md rename to documentation/docs/15-configuration.md diff --git a/documentation/docs/15-types.md b/documentation/docs/16-types.md similarity index 100% rename from documentation/docs/15-types.md rename to documentation/docs/16-types.md diff --git a/documentation/docs/16-seo.md b/documentation/docs/17-seo.md similarity index 100% rename from documentation/docs/16-seo.md rename to documentation/docs/17-seo.md diff --git a/documentation/docs/17-assets.md b/documentation/docs/18-assets.md similarity index 100% rename from documentation/docs/17-assets.md rename to documentation/docs/18-assets.md diff --git a/documentation/docs/18-accessibility.md b/documentation/docs/19-accessibility.md similarity index 100% rename from documentation/docs/18-accessibility.md rename to documentation/docs/19-accessibility.md From b11c57811fd62be3adb5969b0e65bf421f19addd Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 15:55:31 +0200 Subject: [PATCH 09/85] implement handleFile --- documentation/docs/06-form-actions.md | 16 +++++++-------- documentation/docs/07-hooks.md | 17 ++++++++++++++++ packages/kit/src/exports/vite/dev/index.js | 1 + .../kit/src/runtime/server/page/actions.js | 20 ++++++++++++++----- packages/kit/src/runtime/server/page/index.js | 2 +- packages/kit/test/apps/basics/src/hooks.js | 5 +++++ .../actions/handle-file/+page.server.js | 10 ++++++++++ .../routes/actions/handle-file/+page.svelte | 10 ++++++++++ packages/kit/types/index.d.ts | 4 ++++ packages/kit/types/internal.d.ts | 4 +++- 10 files changed, 74 insertions(+), 15 deletions(-) create mode 100644 packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js create mode 100644 packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte diff --git a/documentation/docs/06-form-actions.md b/documentation/docs/06-form-actions.md index 95e8331f45fc..fccd5c0f6d9f 100644 --- a/documentation/docs/06-form-actions.md +++ b/documentation/docs/06-form-actions.md @@ -11,11 +11,11 @@ Actions are defined through `export const actions = {...}`, with each key being ```svelte /// file: src/routes/todos/+page.svelte -
+
@@ -23,7 +23,7 @@ Actions are defined through `export const actions = {...}`, with each key being
    {#each data.todos as todo}
  • -
    + @@ -35,7 +35,7 @@ Actions are defined through `export const actions = {...}`, with each key being ```js /// file: src/routes/todos/+page.server.js -/** @type {import('./$type').Actions} */ +/** @type {import('./$types').Actions} */ export const actions = { addTodo: (event) => { // ... @@ -48,11 +48,11 @@ export const actions = { ## Files and strings are separated -Since `actions` are meant to be used with forms, we can make your life easier by awaiting the `FormData` and separating the form fields which contain strings from those who contain files and running the latter through the `handleFile` hook before passing it as `fields` and `files` into the action function. +Since `actions` are meant to be used with forms, we can make your life easier by awaiting the `FormData` and separating the form fields which contain strings from those who contain files and running the latter through the [`handleFile`](/docs/hooks#handleFile) hook before passing it as `fields` and `files` into the action function. ```js /// file: src/routes/todos/+page.server.js -/** @type {import('./$type').Actions} */ +/** @type {import('./$types').Actions} */ export const actions = { default: ({ fields, files }) => { const name = fields.get('name'); // typed as string @@ -111,7 +111,7 @@ export const actions = { import { form } from '$app/stores'; - + {#if $form?.errors?.username} {$form?.errors?.username} @@ -177,7 +177,7 @@ First we need to ensure that the page is _not_ reloaded on submission. For this, } - + {#if $form?.errors?.username} {$form?.errors?.username} diff --git a/documentation/docs/07-hooks.md b/documentation/docs/07-hooks.md index 7bf25b938010..178ab0f4f71e 100644 --- a/documentation/docs/07-hooks.md +++ b/documentation/docs/07-hooks.md @@ -101,6 +101,23 @@ export function handleError({ error, event }) { > `handleError` is only called for _unexpected_ errors. It is not called for errors created with the [`error`](/docs/modules#sveltejs-kit-error) function imported from `@sveltejs/kit`, as these are _expected_ errors. +### handleFile + +Any files encountered during processing of [form actions](/docs/form-actions) are routed through this handler before being passed into the action function. The return value of `handleFile` becomes the type of each entry of the property `files` on the action; promises are awaited. + +```js +/// file: src/hooks.js +// @filename: ambient.d.ts +const ImageService: any; + +// @filename: index.js +// ---cut--- +/** @type {import('@sveltejs/kit').HandleFile} */ +export function handleFile({ file }) { + return ImageService.upload(file); +} +``` + ### externalFetch This function allows you to modify (or replace) a `fetch` request for an external resource that happens inside a `load` function that runs on the server (or during pre-rendering). diff --git a/packages/kit/src/exports/vite/dev/index.js b/packages/kit/src/exports/vite/dev/index.js index cc6b0bd23eb1..55474ed849db 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -331,6 +331,7 @@ export async function dev(vite, vite_config, svelte_config, illegal_imports) { console.error(colors.gray(error.stack)); } }), + handleFile: user_hooks.handleFile || (({ file }) => file), externalFetch: user_hooks.externalFetch || fetch }; diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 22fdd0d4b297..c966b3ae292b 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -43,7 +43,7 @@ export async function handle_action_json_request(event, options, server) { } try { - const result = await call_action(event, actions); + const result = await call_action(event, options, actions); if (!result) { // TODO json({status: 204}) instead? return new Response(undefined, { @@ -91,11 +91,12 @@ export function is_action_request(event, leaf_node) { /** * @param {import('types').RequestEvent} event + * @param {import('types').SSROptions} options * @param {import('types').SSRNode['server']} server * @returns {Promise | void>} * @throws {Redirect | ValidationError | HttpError | Error} */ -export async function handle_action_request(event, server) { +export async function handle_action_request(event, options, server) { const actions = server.actions; if (!actions) { @@ -109,15 +110,16 @@ export async function handle_action_request(event, server) { throw error(405, 'POST method not allowed. No actions exist for this page'); } - return call_action(event, actions); + return call_action(event, options, actions); } /** * @param {import('types').RequestEvent} event + * @param {import('types').SSROptions} options * @param {NonNullable} actions * @throws {Redirect | ValidationError | HttpError | Error} */ -export async function call_action(event, actions) { +export async function call_action(event, options, actions) { const url = new URL(event.request.url); let name = 'default'; @@ -140,14 +142,22 @@ export async function call_action(event, actions) { const form = await event.request.formData(); const fields = new FormData(); const files = new FilesFormData(); + const promises = []; + for (const [key, value] of form) { if (typeof value === 'string') { fields.append(key, value); } else { - files.append(key, value); + promises.push( + Promise.resolve() + .then(() => options.hooks.handleFile({ event, field: key, file: value })) + .then((/** @type {any} */ value) => ({ key, value })) + ); } } + (await Promise.all(promises)).map(({ key, value }) => files.append(key, value)); + return action({ ...event, fields: /** @type {import('types').FieldsFormData} */ (fields), diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index ba79caea15b8..9f0a2e71a698 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/packages/kit/src/runtime/server/page/index.js @@ -63,7 +63,7 @@ export async function render_page(event, route, page, options, state, resolve_op // for action requests, first call handler in +page.server.js // (this also determines status code) try { - mutation_data = await handle_action_request(event, leaf_node.server); + mutation_data = await handle_action_request(event, options, leaf_node.server); } catch (e) { const error = /** @type {Redirect | HttpError | ValidationError | Error} */ (e); if (error instanceof Redirect) { diff --git a/packages/kit/test/apps/basics/src/hooks.js b/packages/kit/test/apps/basics/src/hooks.js index b1131b608e07..0a6c60e980f5 100644 --- a/packages/kit/test/apps/basics/src/hooks.js +++ b/packages/kit/test/apps/basics/src/hooks.js @@ -43,6 +43,11 @@ export const handle = sequence( } ); +/** @type {import('@sveltejs/kit').HandleFile} */ +export const handleFile = ({ field, file }) => { + return field + ':' + file.name; +}; + /** @type {import('@sveltejs/kit').ExternalFetch} */ export async function externalFetch(request) { let newRequest = request; diff --git a/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js new file mode 100644 index 000000000000..a591efd7083d --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js @@ -0,0 +1,10 @@ +import { invalid } from '@sveltejs/kit'; + +/** @type {import('./$types').Actions} */ +export const actions = { + default: ({ files }) => { + throw invalid(400, undefined, { + result: files.get('file') + }); + } +}; diff --git a/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte new file mode 100644 index 000000000000..302d3671429d --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte @@ -0,0 +1,10 @@ + + +

    Result: {$form?.errors.result}

    + + + + + diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 47b4f763bb28..34fb607aaea3 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -188,6 +188,10 @@ export interface HandleError { (input: { error: Error & { frame?: string }; event: RequestEvent }): void; } +export interface HandleFile { + (input: { event: RequestEvent; field: string; file: File }): MaybePromise; +} + /** * The generic form of `PageLoad` and `LayoutLoad`. You should import those from `./$types` (see [generated types](https://kit.svelte.dev/docs/types#generated-types)) * rather than using `Load` directly. diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts index 50805b8564f7..d0108c1d93d9 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -15,7 +15,8 @@ import { Server, ServerInitOptions, SSRManifest, - Actions + Actions, + HandleFile } from './index.js'; import { HttpMethod, @@ -94,6 +95,7 @@ export interface Hooks { externalFetch: ExternalFetch; handle: Handle; handleError: HandleError; + handleFile: HandleFile; } export interface ImportNode { From 3450eb47033e72e55228b61353a2b92d604c201c Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 15:58:49 +0200 Subject: [PATCH 10/85] fix tests --- .../src/core/sync/write_types/test/layout/_expected/$types.d.ts | 1 + .../test/simple-page-server-and-shared/_expected/$types.d.ts | 1 + .../test/simple-page-server-only/_expected/$types.d.ts | 1 + 3 files changed, 3 insertions(+) diff --git a/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts index c8dae2dadcdd..931fdd9fb548 100644 --- a/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts @@ -41,6 +41,7 @@ export type PageData = Omit< Awaited> >; export type Action = Kit.Action; +export type Actions = Kit.Actions; export type LayoutServerLoad< OutputData extends (Partial & Record) | void = | (Partial & Record) diff --git a/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts index abe81c1b58ae..2219085f7e33 100644 --- a/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts @@ -40,5 +40,6 @@ export type PageData = Omit< Awaited> >; export type Action = Kit.Action; +export type Actions = Kit.Actions; export type LayoutServerData = null; export type LayoutData = LayoutParentData; diff --git a/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts index 4f38245b4052..3c29aa24c45b 100644 --- a/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts @@ -26,5 +26,6 @@ export type PageServerData = Kit.AwaitedProperties< >; export type PageData = Omit & PageServerData; export type Action = Kit.Action; +export type Actions = Kit.Actions; export type LayoutServerData = null; export type LayoutData = LayoutParentData; From 38a253c91f7c5025a1c1a0765c2a1a207364e5e6 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 17:11:19 +0200 Subject: [PATCH 11/85] test for files --- packages/kit/test/apps/basics/test/test.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 728ec799756a..c39f6c745fa5 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1743,4 +1743,25 @@ test.describe('Actions', () => { expect(await page.inputValue('input[name="password"]')).toBe(''); } }); + + test('handleFile is called', async ({ page }) => { + await page.goto('/actions/handle-file'); + + const [fileChooser] = await Promise.all([ + page.waitForEvent('filechooser'), + page.locator('input[name="file"]').click() + ]); + await fileChooser.setFiles({ + name: 'test.txt', + mimeType: 'text/plain', + buffer: Buffer.from('test') + }); + + await Promise.all([ + page.waitForRequest((request) => request.url().includes('/actions/handle-file')), + page.click('button') + ]); + + expect(await page.textContent('p')).toBe('Result: file:test.txt'); + }); }); From 2da72d9cb31d6d392b127373b10753ff02aceb29 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 17:48:28 +0200 Subject: [PATCH 12/85] complete migration message --- packages/kit/src/runtime/server/page/actions.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index c966b3ae292b..6c4d5a6a918f 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -171,7 +171,9 @@ export async function call_action(event, options, actions) { function maybe_throw_migration_error(server) { for (const method of ['POST', 'PUT', 'PATCH', 'DELETE']) { if (/** @type {any} */ (server)[method]) { - throw new Error(`${method} method no longer allowed in +page.server, use actions instead.`); + throw new Error( + `${method} method no longer allowed in +page.server, use actions instead. See the PR for more info: https://github.com/sveltejs/kit/pull/6469` + ); } } } From 62fa93cbec4d6ba7c5f1bf797bb1ab71beef21ca Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 17:50:20 +0200 Subject: [PATCH 13/85] support validation error thrown in endpoints --- packages/kit/src/runtime/server/endpoint.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index 6eebd7c71f7c..a86c6a7c7069 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -1,4 +1,5 @@ -import { Redirect } from '../control.js'; +import { json } from '../../exports/index.js'; +import { Redirect, ValidationError } from '../control.js'; import { check_method_names, method_not_allowed } from './utils.js'; /** @@ -56,6 +57,15 @@ export async function render_endpoint(event, mod, state) { status: error.status, headers: { location: error.location } }); + } else if (error instanceof ValidationError) { + return json( + { + status: error.status, + errors: error.errors, + values: error.values + }, + { status: error.status } + ); } throw error; From 225be5c4df8f7e973e22fe0b1ddb57ac9aa7d686 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 18:11:57 +0200 Subject: [PATCH 14/85] infer file type from handleFile hook --- packages/kit/src/core/sync/write_types/index.js | 9 +++++++-- .../write_types/test/layout/_expected/$types.d.ts | 7 +++++-- .../_expected/$types.d.ts | 7 +++++-- .../simple-page-server-only/_expected/$types.d.ts | 7 +++++-- packages/kit/test/apps/basics/src/hooks.js | 2 +- packages/kit/types/index.d.ts | 15 +++++++++------ 6 files changed, 32 insertions(+), 15 deletions(-) diff --git a/packages/kit/src/core/sync/write_types/index.js b/packages/kit/src/core/sync/write_types/index.js index 05e095abbd3d..1bdfbeb4b7f3 100644 --- a/packages/kit/src/core/sync/write_types/index.js +++ b/packages/kit/src/core/sync/write_types/index.js @@ -192,8 +192,13 @@ function update_types(config, routes, route) { for (const file of written_proxies) to_delete.delete(file); if (route.leaf.server) { - exports.push(`export type Action = Kit.Action`); - exports.push(`export type Actions = Kit.Actions`); + declarations.push( + `type FileType = typeof import('${posixify( + config.kit.files.hooks + )}.js') extends { handleFile: infer T } ? ReturnType : File;` + ); + exports.push(`export type Action = Kit.Action`); + exports.push(`export type Actions = Kit.Actions`); } } diff --git a/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts index 931fdd9fb548..d5af12a0e2c9 100644 --- a/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts @@ -13,6 +13,9 @@ type OutputDataShape = MaybeWithVoid< type EnsureParentData = T extends null | undefined ? {} : T; type PageServerParentData = EnsureParentData; type PageParentData = EnsureParentData; +type FileType = typeof import('src/hooks.js') extends { handleFile: infer T } + ? ReturnType + : File; type LayoutParams = RouteParams & {}; type LayoutServerParentData = EnsureParentData<{}>; type LayoutParentData = EnsureParentData<{}>; @@ -40,8 +43,8 @@ export type PageData = Omit< Kit.AwaitedProperties< Awaited> >; -export type Action = Kit.Action; -export type Actions = Kit.Actions; +export type Action = Kit.Action; +export type Actions = Kit.Actions; export type LayoutServerLoad< OutputData extends (Partial & Record) | void = | (Partial & Record) diff --git a/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts index 2219085f7e33..dc967251746b 100644 --- a/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts @@ -13,6 +13,9 @@ type OutputDataShape = MaybeWithVoid< type EnsureParentData = T extends null | undefined ? {} : T; type PageServerParentData = EnsureParentData; type PageParentData = EnsureParentData; +type FileType = typeof import('src/hooks.js') extends { handleFile: infer T } + ? ReturnType + : File; type LayoutParams = RouteParams & {}; type LayoutParentData = EnsureParentData<{}>; @@ -39,7 +42,7 @@ export type PageData = Omit< Kit.AwaitedProperties< Awaited> >; -export type Action = Kit.Action; -export type Actions = Kit.Actions; +export type Action = Kit.Action; +export type Actions = Kit.Actions; export type LayoutServerData = null; export type LayoutData = LayoutParentData; diff --git a/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts index 3c29aa24c45b..4a2f7c4fc904 100644 --- a/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts @@ -13,6 +13,9 @@ type OutputDataShape = MaybeWithVoid< type EnsureParentData = T extends null | undefined ? {} : T; type PageServerParentData = EnsureParentData; type PageParentData = EnsureParentData; +type FileType = typeof import('src/hooks.js') extends { handleFile: infer T } + ? ReturnType + : File; type LayoutParams = RouteParams & {}; type LayoutParentData = EnsureParentData<{}>; @@ -25,7 +28,7 @@ export type PageServerData = Kit.AwaitedProperties< Awaited> >; export type PageData = Omit & PageServerData; -export type Action = Kit.Action; -export type Actions = Kit.Actions; +export type Action = Kit.Action; +export type Actions = Kit.Actions; export type LayoutServerData = null; export type LayoutData = LayoutParentData; diff --git a/packages/kit/test/apps/basics/src/hooks.js b/packages/kit/test/apps/basics/src/hooks.js index 0a6c60e980f5..d415d275efa5 100644 --- a/packages/kit/test/apps/basics/src/hooks.js +++ b/packages/kit/test/apps/basics/src/hooks.js @@ -43,7 +43,7 @@ export const handle = sequence( } ); -/** @type {import('@sveltejs/kit').HandleFile} */ +/** @type {import('@sveltejs/kit').HandleFile} */ export const handleFile = ({ field, file }) => { return field + ':' + file.name; }; diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 34fb607aaea3..35721b01c26c 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -314,20 +314,23 @@ export interface ServerLoadEvent< } export interface Action< - Params extends Partial> = Partial> + Params extends Partial> = Partial>, + FileType = any > { - (event: ActionEvent): MaybePromise | void>; + (event: ActionEvent): MaybePromise | void>; } export type Actions< - Params extends Partial> = Partial> -> = Record>; + Params extends Partial> = Partial>, + FileType = any +> = Record>; export interface ActionEvent< - Params extends Partial> = Partial> + Params extends Partial> = Partial>, + FileType = any > extends RequestEvent { fields: FieldsFormData; - files: FilesFormData; + files: FilesFormData; } export class FieldsFormData extends FormData { From 7ce58eadb14d3639a227628b0caf66f459f9eb15 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 18:34:17 +0200 Subject: [PATCH 15/85] add handleFile to build --- packages/kit/src/exports/vite/build/build_server.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/src/exports/vite/build/build_server.js b/packages/kit/src/exports/vite/build/build_server.js index 231243312c7c..8571f0606160 100644 --- a/packages/kit/src/exports/vite/build/build_server.js +++ b/packages/kit/src/exports/vite/build/build_server.js @@ -110,6 +110,7 @@ export class Server { this.options.hooks = { handle: module.handle || (({ event, resolve }) => resolve(event)), handleError: module.handleError || (({ error }) => console.error(error.stack)), + handleFile: module.handleFile || (({ file }) => file), externalFetch: module.externalFetch || fetch }; } From 9dcb6becec303d8a0f5f146c345eaf849396500c Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 20:28:44 +0200 Subject: [PATCH 16/85] types, cleanup --- documentation/docs/06-form-actions.md | 49 +++++++++++++++++-- packages/kit/src/runtime/client/client.js | 2 +- packages/kit/src/runtime/client/types.d.ts | 4 +- .../kit/src/runtime/server/page/actions.js | 15 +++--- packages/kit/src/runtime/server/page/index.js | 5 +- .../actions/success-data/+page.server.js | 15 ++++++ .../routes/actions/success-data/+page.svelte | 32 ++++++++++++ packages/kit/types/ambient.d.ts | 4 +- packages/kit/types/index.d.ts | 12 ++++- 9 files changed, 117 insertions(+), 21 deletions(-) create mode 100644 packages/kit/test/apps/basics/src/routes/actions/success-data/+page.server.js create mode 100644 packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte diff --git a/documentation/docs/06-form-actions.md b/documentation/docs/06-form-actions.md index fccd5c0f6d9f..48d425f0273b 100644 --- a/documentation/docs/06-form-actions.md +++ b/documentation/docs/06-form-actions.md @@ -123,7 +123,7 @@ export const actions = { ## Success -If everything is valid, an action can either return a JSON object with data that is merged with the returned data from `load`, or it can `throw` a `redirect` to redirect the user to another page. +If everything is valid, an action can return a JSON object with data that is part of the JSON response in the case of a JavaScript fetch - it's discarded in case of a full page reload. Alternatively it can `throw` a `redirect` to redirect the user to another page. ```js /// file: src/routes/login/+page.server.js @@ -179,8 +179,8 @@ First we need to ensure that the page is _not_ reloaded on submission. For this,
    - {#if $form?.errors?.username} - {$form?.errors?.username} + {#if $form?.errors.username} + {$form.errors.username} {/if} @@ -205,7 +205,7 @@ As you can see, progressive enhancement is doable, but it may become a little cu {#if errors?.username} - {errors?.username} + {errors.username} {/if} @@ -213,3 +213,44 @@ As you can see, progressive enhancement is doable, but it may become a little cu ``` TODO explain the API in more detail + +## Alternatives + +In case you don't need your forms to work without JavaScript, you want to use HTTP verbs other than `POST`, or you want to send arbitrary JSON instead of being restricted to `FormData`, then you can resort to interacting with your API through `+server.js` endpoints (which will be possible to place next to `+page` files, soon). + +```svelte + + + + + {#if errors.username} + {errors.username} + {/if} + + +
    +``` diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index b940c1bf6a8a..3d8c24f3fdfe 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -410,7 +410,7 @@ export function create_client({ target, base, trailing_slash }) { * status: number; * error: HttpError | Error | null; * routeId: string | null; - * form_state?: import('types').FormState | null; + * form_state?: import('types').SubmittedState | null; * }} opts */ async function get_navigation_result_from_branch({ diff --git a/packages/kit/src/runtime/client/types.d.ts b/packages/kit/src/runtime/client/types.d.ts index 20d258174055..e7d4c5d60fa9 100644 --- a/packages/kit/src/runtime/client/types.d.ts +++ b/packages/kit/src/runtime/client/types.d.ts @@ -7,7 +7,7 @@ import { prefetch, prefetchRoutes } from '$app/navigation'; -import { CSRPageNode, CSRPageNodeLoader, CSRRoute, FormState, Uses } from 'types'; +import { CSRPageNode, CSRPageNodeLoader, CSRRoute, SubmittedState, Uses } from 'types'; import { HttpError } from '../control.js'; import { SerializedHttpError } from '../server/page/types.js'; @@ -30,7 +30,7 @@ export interface Client { params: Record; routeId: string | null; data: Array; - form: FormState | null; + form: SubmittedState | null; }) => Promise; _start_router: () => void; } diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 6c4d5a6a918f..2e0fe22f5421 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -25,8 +25,6 @@ export function is_action_json_request(event) { * @param {import('types').SSRNode['server']} server */ export async function handle_action_json_request(event, options, server) { - // TODO create POJO interface for this? Something like - // {status: number, errors?: Record, location?: string, values?: Record, result?: Record} const actions = server.actions; if (!actions) { @@ -50,22 +48,27 @@ export async function handle_action_json_request(event, options, server) { status: 204 }); } else { - return json({ status: 200, result }); + return json(/** @type {import('types').FormFetchResponse} */ ({ status: 200, result })); } } catch (e) { const error = /** @type {Redirect | HttpError | ValidationError | Error} */ (e); if (error instanceof Redirect) { - return json({ status: 303, location: error.location }); + return json( + /** @type {import('types').FormFetchResponse} */ ({ + status: error.status, + location: error.location + }) + ); } if (error instanceof ValidationError) { return json( - { + /** @type {import('types').FormFetchResponse} */ ({ status: error.status, values: error.values, errors: error.errors - }, + }), { status: error.status } ); } diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index 9f0a2e71a698..6b762621529e 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/packages/kit/src/runtime/server/page/index.js @@ -50,9 +50,6 @@ export async function render_page(event, route, page, options, state, resolve_op let status = 200; - /** @type {Record | void} */ - let mutation_data; - /** @type {HttpError | Error} */ let mutation_error; @@ -63,7 +60,7 @@ export async function render_page(event, route, page, options, state, resolve_op // for action requests, first call handler in +page.server.js // (this also determines status code) try { - mutation_data = await handle_action_request(event, options, leaf_node.server); + await handle_action_request(event, options, leaf_node.server); } catch (e) { const error = /** @type {Redirect | HttpError | ValidationError | Error} */ (e); if (error instanceof Redirect) { diff --git a/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.server.js new file mode 100644 index 000000000000..9f8b97f5a846 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.server.js @@ -0,0 +1,15 @@ +/** @type {import('./$types').PageServerLoad} */ +export function load() { + return { + initial: 'initial' + }; +} + +/** @type {import('./$types').Actions} */ +export const actions = { + default: ({ files }) => { + return { + result: files.get('username') + }; + } +}; diff --git a/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte new file mode 100644 index 000000000000..892fe7082b9e --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte @@ -0,0 +1,32 @@ + + +
    {JSON.stringify(data)}
    + + +{#if hydrated} +
    {JSON.stringify(hydrated)}
    +{/if} + +
    + + +
    diff --git a/packages/kit/types/ambient.d.ts b/packages/kit/types/ambient.d.ts index ad92e4e7053d..f2c450cdcba9 100644 --- a/packages/kit/types/ambient.d.ts +++ b/packages/kit/types/ambient.d.ts @@ -201,7 +201,7 @@ declare module '$app/paths' { */ declare module '$app/stores' { import { Readable, Writable } from 'svelte/store'; - import { FormState, Navigation, Page } from '@sveltejs/kit'; + import { SubmittedState, Navigation, Page } from '@sveltejs/kit'; /** * A readable store whose value contains page data. @@ -221,7 +221,7 @@ declare module '$app/stores' { * A writable store whose value contain the errors and values of the last form submission. * It is updated automatically when using the `
    ` component, else you need to take care of it yourself. */ - export const form: Writable; + export const form: Writable; /** * A function that returns all of the contextual stores. On the server, this must be called during component initialization. diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 35721b01c26c..5505ae49a2a3 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -364,10 +364,18 @@ export class FilesFormData forEach(callbackfn: (value: FileFileType, key: string, parent: FilesFormData) => void): void; } -export interface FormState extends Partial> { +export interface SubmittedState { + error: Record; + values: Record; +} + +// TODO make this a union type with type=success/redirect/error? +export interface FormFetchResponse { + status: number; error?: Record; - values?: Record; + values?: Record; result?: Record; + location?: string; } // TODO figure out how to just re-export from '../src/index/index.js' without From f1941a59d1f4ebb20264714b1d776213a88e8aaa Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 20:51:05 +0200 Subject: [PATCH 17/85] $form -> $submitted --- documentation/docs/06-form-actions.md | 26 +++++++++---------- packages/kit/src/core/sync/write_root.js | 4 +-- packages/kit/src/runtime/app/stores.js | 10 +++---- packages/kit/src/runtime/client/client.js | 10 +++---- packages/kit/src/runtime/client/singletons.js | 4 +-- packages/kit/src/runtime/client/types.d.ts | 2 +- .../kit/src/runtime/server/page/render.js | 10 +++---- .../form-errors-persist-fields/+page.svelte | 8 +++--- .../routes/actions/form-errors/+page.svelte | 6 ++--- .../routes/actions/handle-file/+page.svelte | 4 +-- packages/kit/types/ambient.d.ts | 4 +-- 11 files changed, 42 insertions(+), 46 deletions(-) diff --git a/documentation/docs/06-form-actions.md b/documentation/docs/06-form-actions.md index 48d425f0273b..2d2b59075440 100644 --- a/documentation/docs/06-form-actions.md +++ b/documentation/docs/06-form-actions.md @@ -64,7 +64,7 @@ export const actions = { ## Validation -A core part of form submissions is validation. For this, an action can `throw` the `invalid` helper method exported from `@sveltejs/kit` if there are validation errors. `invalid` expects a `status`, possibly the form `values` (make sure to remove any user sensitive information such as passwords) and an `error` object. In case of a native form submit they populate the `$form` store which is available inside your components so you can preserve user input. +A core part of form submissions is validation. For this, an action can `throw` the `invalid` helper method exported from `@sveltejs/kit` if there are validation errors. `invalid` expects a `status`, possibly the form `values` (make sure to remove any user sensitive information such as passwords) and an `error` object. In case of a native form submit they populate the `$submitted` store which is available inside your components so you can preserve user input. ```js /// file: src/routes/login/+page.server.js @@ -108,13 +108,13 @@ export const actions = { ```svelte /// file: src/routes/login/+page.svelte - - {#if $form?.errors?.username} - {$form?.errors?.username} + + {#if $submitted?.errors?.username} + {$submitted?.errors?.username} {/if} @@ -154,7 +154,7 @@ First we need to ensure that the page is _not_ reloaded on submission. For this, ```svelte /// file: src/routes/login/+page.svelte - - {#if $form?.errors.username} - {$form.errors.username} + + {#if $submitted?.errors.username} + {$submitted.errors.username} {/if} @@ -189,7 +189,7 @@ First we need to ensure that the page is _not_ reloaded on submission. For this, ## `` component -As you can see, progressive enhancement is doable, but it may become a little cumbersome over time. That's we provide an opinionated wrapper component which does all the heavy lifting for you. Here is the same login page using the `` component from `@sveltejs/kit`: +As you can see, progressive enhancement is doable, but it may become a little cumbersome over time. That's why we will soon provide an opinionated wrapper component which does all the heavy lifting for you. Here's how the same login page would look like using the `` component: ```svelte /// file: src/routes/login/+page.svelte @@ -212,8 +212,6 @@ As you can see, progressive enhancement is doable, but it may become a little cu ``` -TODO explain the API in more detail - ## Alternatives In case you don't need your forms to work without JavaScript, you want to use HTTP verbs other than `POST`, or you want to send arbitrary JSON instead of being restricted to `FormData`, then you can resort to interacting with your API through `+server.js` endpoints (which will be possible to place next to `+page` files, soon). diff --git a/packages/kit/src/core/sync/write_root.js b/packages/kit/src/core/sync/write_root.js index 28694883acfa..8112252f84a9 100644 --- a/packages/kit/src/core/sync/write_root.js +++ b/packages/kit/src/core/sync/write_root.js @@ -51,14 +51,14 @@ export function write_root(manifest_data, output) { export let components; ${levels.map((l) => `export let data_${l} = null;`).join('\n\t\t\t\t')} - export let form; + export let submitted; if (!browser) { setContext('__svelte__', stores); } $: stores.page.set(page); - $: stores.form.set(form); + $: stores.submitted.set(submitted); afterUpdate(stores.page.notify); let mounted = false; diff --git a/packages/kit/src/runtime/app/stores.js b/packages/kit/src/runtime/app/stores.js index d27927c53475..0bb433c7229b 100644 --- a/packages/kit/src/runtime/app/stores.js +++ b/packages/kit/src/runtime/app/stores.js @@ -26,7 +26,7 @@ export const getStores = () => { subscribe: stores.navigating.subscribe }, updated: stores.updated, - form: stores.form + submitted: stores.submitted }; // TODO remove this for 1.0 @@ -69,18 +69,18 @@ export const navigating = { } }; -/** @type {typeof import('$app/stores').form} */ +/** @type {typeof import('$app/stores').submitted} */ export const form = { set(values) { - const store = getStores().form; + const store = getStores().submitted; return store.set(values); }, update(value) { - const store = getStores().form; + const store = getStores().submitted; return store.update(value); }, subscribe(fn) { - const store = getStores().form; + const store = getStores().submitted; return store.subscribe(fn); } }; diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 3d8c24f3fdfe..081151bfc808 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -410,7 +410,7 @@ export function create_client({ target, base, trailing_slash }) { * status: number; * error: HttpError | Error | null; * routeId: string | null; - * form_state?: import('types').SubmittedState | null; + * submitted?: import('types').SubmittedState | null; * }} opts */ async function get_navigation_result_from_branch({ @@ -420,7 +420,7 @@ export function create_client({ target, base, trailing_slash }) { status, error, routeId, - form_state + submitted }) { const filtered = /** @type {import('./types').BranchNode[] } */ (branch.filter(Boolean)); @@ -436,7 +436,7 @@ export function create_client({ target, base, trailing_slash }) { }, props: { components: filtered.map((branch_node) => branch_node.node.component), - form: form_state + submitted } }; @@ -1301,7 +1301,7 @@ export function create_client({ target, base, trailing_slash }) { params, routeId, data: server_data_nodes, - form: form_state + submitted }) => { const url = new URL(location.href); @@ -1342,7 +1342,7 @@ export function create_client({ target, base, trailing_slash }) { original_error.message ) : original_error, - form_state, + submitted, routeId }); } catch (e) { diff --git a/packages/kit/src/runtime/client/singletons.js b/packages/kit/src/runtime/client/singletons.js index 90de032f9cb7..c471bb9d7431 100644 --- a/packages/kit/src/runtime/client/singletons.js +++ b/packages/kit/src/runtime/client/singletons.js @@ -18,7 +18,5 @@ export const stores = { page: notifiable_store({}), navigating: writable(/** @type {import('types').Navigation | null} */ (null)), updated: create_updated_store(), - form: writable( - /** @type {{errors: Record, values?: Record} | null} */ (null) - ) + submitted: writable(/** @type {import('types').SubmittedState | null} */ (null)) }; diff --git a/packages/kit/src/runtime/client/types.d.ts b/packages/kit/src/runtime/client/types.d.ts index e7d4c5d60fa9..c8fb7bbb2126 100644 --- a/packages/kit/src/runtime/client/types.d.ts +++ b/packages/kit/src/runtime/client/types.d.ts @@ -30,7 +30,7 @@ export interface Client { params: Record; routeId: string | null; data: Array; - form: SubmittedState | null; + submitted: SubmittedState | null; }) => Promise; _start_router: () => void; } diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index 56c32cf1d0a3..8fa5f9b62606 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -80,7 +80,7 @@ export async function render_response({ stores: { page: writable(null), navigating: writable(null), - form: writable(null), + submitted: writable(null), updated }, components: await Promise.all(branch.map(({ node }) => node.component())) @@ -105,7 +105,7 @@ export async function render_response({ }; if (validation_errors) { - props.form = { errors: validation_errors.errors, values: validation_errors.values }; + props.submitted = { errors: validation_errors.errors, values: validation_errors.values }; } // TODO remove this for 1.0 @@ -175,7 +175,7 @@ export async function render_response({ /** @param {string} path */ const prefixed = (path) => (path.startsWith('/') ? path : `${assets}/${path}`); - const serialized = { data: '', form: 'null' }; + const serialized = { data: '', submitted: 'null' }; try { serialized.data = devalue(branch.map(({ server_data }) => server_data)); @@ -191,7 +191,7 @@ export async function render_response({ if (validation_errors) { try { - serialized.form = devalue({ + serialized.submitted = devalue({ errors: validation_errors.errors, values: validation_errors.values }); @@ -216,7 +216,7 @@ export async function render_response({ params: ${devalue(event.params)}, routeId: ${s(event.routeId)}, data: ${serialized.data}, - form: ${serialized.form} + submitted: ${serialized.submitted} }` : 'null'}, paths: ${s(options.paths)}, target: document.querySelector('[data-sveltekit-hydrate="${target}"]').parentNode, diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte index 0f1e8c32a9a8..0c3769b7b62c 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte @@ -1,8 +1,8 @@
    - +
    diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte index 17528838daed..99aa8695875f 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte @@ -1,15 +1,15 @@
    -

    {$form?.errors?.message}

    +

    {$submitted?.errors?.message}

    {#if hydrated_error_message} diff --git a/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte index 302d3671429d..506725eef44e 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte @@ -1,8 +1,8 @@ -

    Result: {$form?.errors.result}

    +

    Result: {$submitted?.errors.result}

    diff --git a/packages/kit/types/ambient.d.ts b/packages/kit/types/ambient.d.ts index f2c450cdcba9..85f5527c8898 100644 --- a/packages/kit/types/ambient.d.ts +++ b/packages/kit/types/ambient.d.ts @@ -221,7 +221,7 @@ declare module '$app/stores' { * A writable store whose value contain the errors and values of the last form submission. * It is updated automatically when using the `` component, else you need to take care of it yourself. */ - export const form: Writable; + export const submitted: Writable; /** * A function that returns all of the contextual stores. On the server, this must be called during component initialization. @@ -231,7 +231,7 @@ declare module '$app/stores' { navigating: typeof navigating; page: typeof page; updated: typeof updated; - form: typeof form; + submitted: typeof submitted; }; } From 71ed3523a39b92d2d1f3654e99dead49ed1383a7 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 1 Sep 2022 20:55:20 +0200 Subject: [PATCH 18/85] woops --- packages/kit/src/runtime/app/stores.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/app/stores.js b/packages/kit/src/runtime/app/stores.js index 0bb433c7229b..17be206884f1 100644 --- a/packages/kit/src/runtime/app/stores.js +++ b/packages/kit/src/runtime/app/stores.js @@ -70,7 +70,7 @@ export const navigating = { }; /** @type {typeof import('$app/stores').submitted} */ -export const form = { +export const submitted = { set(values) { const store = getStores().submitted; return store.set(values); From ba0f204d5a714dab51ad5c7f754bf3265b753605 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 2 Sep 2022 15:47:56 +0200 Subject: [PATCH 19/85] allow arbitrary data on invalid, persist data in success case --- documentation/docs/06-form-actions.md | 24 ++++++-- packages/kit/src/exports/index.js | 7 +-- packages/kit/src/runtime/client/client.js | 2 +- packages/kit/src/runtime/client/singletons.js | 2 +- packages/kit/src/runtime/client/types.d.ts | 4 +- packages/kit/src/runtime/control.js | 31 ++-------- packages/kit/src/runtime/server/endpoint.js | 9 +-- packages/kit/src/runtime/server/index.js | 1 - .../kit/src/runtime/server/page/actions.js | 24 +++----- packages/kit/src/runtime/server/page/index.js | 18 +++--- .../kit/src/runtime/server/page/render.js | 17 +++--- .../runtime/server/page/respond_with_error.js | 3 +- .../+page.server.js | 7 ++- .../actions/form-errors/+page.server.js | 2 +- .../actions/handle-file/+page.server.js | 4 +- .../actions/success-data/+page.server.js | 4 +- .../routes/actions/success-data/+page.svelte | 19 ++---- .../shadowed/error-post/+page.server.js | 4 +- .../routes/shadowed/error-post/+page.svelte | 4 +- packages/kit/test/apps/basics/test/test.js | 14 +++++ packages/kit/types/ambient.d.ts | 4 +- packages/kit/types/index.d.ts | 61 ++++++++----------- 22 files changed, 119 insertions(+), 146 deletions(-) diff --git a/documentation/docs/06-form-actions.md b/documentation/docs/06-form-actions.md index 2d2b59075440..dbd2008f2ddf 100644 --- a/documentation/docs/06-form-actions.md +++ b/documentation/docs/06-form-actions.md @@ -64,7 +64,7 @@ export const actions = { ## Validation -A core part of form submissions is validation. For this, an action can `throw` the `invalid` helper method exported from `@sveltejs/kit` if there are validation errors. `invalid` expects a `status`, possibly the form `values` (make sure to remove any user sensitive information such as passwords) and an `error` object. In case of a native form submit they populate the `$submitted` store which is available inside your components so you can preserve user input. +A core part of form submissions is validation. For this, an action can `throw` the `invalid` helper method exported from `@sveltejs/kit` if there are validation errors. `invalid` expects a `status` as a required argument, and optionally anything else you want to return as a second argument. This could be the form value (make sure to remove any user sensitive information such as passwords) and an `error` object. In case of a native form submit the second argument to `invalid` populates the `$submitted` store which is available inside your components. You can use this to preserve user input. ```js /// file: src/routes/login/+page.server.js @@ -95,8 +95,9 @@ export const actions = { const user = await db.findUser(username); if (!user) { - throw invalid(403, { username }, { - username: 'No user with this username' + throw invalid(403, { + values: { username }, + errors: { username: 'No user with this username' } }); } @@ -123,7 +124,7 @@ export const actions = { ## Success -If everything is valid, an action can return a JSON object with data that is part of the JSON response in the case of a JavaScript fetch - it's discarded in case of a full page reload. Alternatively it can `throw` a `redirect` to redirect the user to another page. +If everything is valid, an action can return a JSON object with data, which will be available through the `$submitted` store. Alternatively it can `throw` a `redirect` to redirect the user to another page. ```js /// file: src/routes/login/+page.server.js @@ -145,6 +146,21 @@ export const actions = { }; ``` +```svelte +/// file: src/routes/login/+page.svelte + + +{#if $submitted.success} + Login successful +{/if} + + + ... + +``` + ## Progressive enhancement So far, all the code examples run native form submissions - that is, when the user pressed the submit button, the page is reloaded. It's good that this use case is supported since JavaScript may not be loaded all the time. When it is though, it might be a better user experience to use the powers JavaScript gives us to provide a better user experience - this is called progressive enhancement. diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index 521943b25264..ab5344732c22 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -47,9 +47,8 @@ export function json(data, init) { /** * Generates a `ValidationError` object. * @param {number} status - * @param {FormData | Record | null | undefined} values - * @param {Record} errors + * @param {Record | undefined} [data] */ -export function invalid(status, values, errors) { - return new ValidationError(status, values, errors); +export function invalid(status, data) { + return new ValidationError(status, data); } diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index 081151bfc808..a5e398441e94 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -410,7 +410,7 @@ export function create_client({ target, base, trailing_slash }) { * status: number; * error: HttpError | Error | null; * routeId: string | null; - * submitted?: import('types').SubmittedState | null; + * submitted?: Record | null; * }} opts */ async function get_navigation_result_from_branch({ diff --git a/packages/kit/src/runtime/client/singletons.js b/packages/kit/src/runtime/client/singletons.js index c471bb9d7431..ef31ad29af4f 100644 --- a/packages/kit/src/runtime/client/singletons.js +++ b/packages/kit/src/runtime/client/singletons.js @@ -18,5 +18,5 @@ export const stores = { page: notifiable_store({}), navigating: writable(/** @type {import('types').Navigation | null} */ (null)), updated: create_updated_store(), - submitted: writable(/** @type {import('types').SubmittedState | null} */ (null)) + submitted: writable(/** @type {Record | null} */ (null)) }; diff --git a/packages/kit/src/runtime/client/types.d.ts b/packages/kit/src/runtime/client/types.d.ts index c8fb7bbb2126..89076672d7b6 100644 --- a/packages/kit/src/runtime/client/types.d.ts +++ b/packages/kit/src/runtime/client/types.d.ts @@ -7,7 +7,7 @@ import { prefetch, prefetchRoutes } from '$app/navigation'; -import { CSRPageNode, CSRPageNodeLoader, CSRRoute, SubmittedState, Uses } from 'types'; +import { CSRPageNode, CSRPageNodeLoader, CSRRoute, Uses } from 'types'; import { HttpError } from '../control.js'; import { SerializedHttpError } from '../server/page/types.js'; @@ -30,7 +30,7 @@ export interface Client { params: Record; routeId: string | null; data: Array; - submitted: SubmittedState | null; + submitted: Record | null; }) => Promise; _start_router: () => void; } diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index b199e29cd661..7ef4be6781d1 100644 --- a/packages/kit/src/runtime/control.js +++ b/packages/kit/src/runtime/control.js @@ -32,35 +32,16 @@ export class Redirect { } } +/** + * @template {Record | undefined} [T=undefined] + */ export class ValidationError { /** * @param {number} status - * @param {FormData | Record | null | undefined} values - * @param {Record} errors + * @param {T} [data] */ - constructor(status, values, errors) { - if (values instanceof FormData) { - /** @type {Record} */ - const converted = {}; - for (const [key, value] of values.entries()) { - if (typeof value === 'string') { - if (converted[key] !== undefined) { - const array = /** @type {string[]} */ ( - Array.isArray(converted[key]) ? converted[key] : [converted[key]] - ); - array.push(value); - converted[key] = array; - } else { - converted[key] = value; - } - } - } - this.values = converted; - } else { - this.values = values; - } - + constructor(status, data) { this.status = status; - this.errors = errors; + this.data = data; } } diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index a86c6a7c7069..5e3aba0840e4 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -58,14 +58,7 @@ export async function render_endpoint(event, mod, state) { headers: { location: error.location } }); } else if (error instanceof ValidationError) { - return json( - { - status: error.status, - errors: error.errors, - values: error.values - }, - { status: error.status } - ); + return json(error.data, { status: error.status }); } throw error; diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 413903640056..88ccde8cc019 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -225,7 +225,6 @@ export async function respond(request, options, state) { error: null, branch: [], fetched: [], - validation_errors: undefined, cookies: [], resolve_opts }); diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 2e0fe22f5421..ee44dab0b740 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -43,34 +43,26 @@ export async function handle_action_json_request(event, options, server) { try { const result = await call_action(event, options, actions); if (!result) { - // TODO json({status: 204}) instead? return new Response(undefined, { status: 204 }); } else { - return json(/** @type {import('types').FormFetchResponse} */ ({ status: 200, result })); + return json(result); } } catch (e) { const error = /** @type {Redirect | HttpError | ValidationError | Error} */ (e); if (error instanceof Redirect) { - return json( - /** @type {import('types').FormFetchResponse} */ ({ - status: error.status, - location: error.location - }) - ); + return json({ + status: error.status, + location: error.location + }); } if (error instanceof ValidationError) { - return json( - /** @type {import('types').FormFetchResponse} */ ({ - status: error.status, - values: error.values, - errors: error.errors - }), - { status: error.status } - ); + return json(error.data, { + status: error.status + }); } if (!(error instanceof HttpError)) { diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index 6b762621529e..7a730307ad08 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/packages/kit/src/runtime/server/page/index.js @@ -50,17 +50,20 @@ export async function render_page(event, route, page, options, state, resolve_op let status = 200; + /** @type {Record | void} */ + let mutation_data; + /** @type {HttpError | Error} */ let mutation_error; /** @type {ValidationError | undefined} */ - let validation_errors; + let validation_error; if (is_action_request(event, leaf_node)) { // for action requests, first call handler in +page.server.js // (this also determines status code) try { - await handle_action_request(event, options, leaf_node.server); + mutation_data = await handle_action_request(event, options, leaf_node.server); } catch (e) { const error = /** @type {Redirect | HttpError | ValidationError | Error} */ (e); if (error instanceof Redirect) { @@ -69,7 +72,7 @@ export async function render_page(event, route, page, options, state, resolve_op status = error instanceof HttpError || error instanceof ValidationError ? error.status : 500; if (error instanceof ValidationError) { - validation_errors = error; + validation_error = error; } else { mutation_error = error; } @@ -106,7 +109,6 @@ export async function render_page(event, route, page, options, state, resolve_op if (get_option(nodes, 'ssr') === false) { return await render_response({ branch: [], - validation_errors: undefined, fetched, cookies, page_config: { @@ -250,8 +252,7 @@ export async function render_page(event, route, page, options, state, resolve_op server_data: null }), fetched, - cookies, - validation_errors: undefined + cookies }); } } @@ -283,8 +284,6 @@ export async function render_page(event, route, page, options, state, resolve_op }); } - // TODO use validation_errors - return await render_response({ event, options, @@ -297,7 +296,8 @@ export async function render_page(event, route, page, options, state, resolve_op status, error: null, branch: compact(branch), - validation_errors, + validation_error, + mutation_data, fetched, cookies }); diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index 8fa5f9b62606..b01679743bbb 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -28,7 +28,8 @@ const updated = { * error: HttpError | Error | null; * event: import('types').RequestEvent; * resolve_opts: import('types').RequiredResolveOptions; - * validation_errors: ValidationError | undefined; + * validation_error?: ValidationError | undefined; + * mutation_data?: Record | void; * }} opts */ export async function render_response({ @@ -42,7 +43,8 @@ export async function render_response({ error = null, event, resolve_opts, - validation_errors + validation_error, + mutation_data }) { if (state.prerendering) { if (options.csp.mode === 'nonce') { @@ -104,9 +106,7 @@ export async function render_response({ data }; - if (validation_errors) { - props.submitted = { errors: validation_errors.errors, values: validation_errors.values }; - } + props.submitted = (mutation_data || validation_error?.data) ?? null; // TODO remove this for 1.0 /** @@ -189,12 +189,9 @@ export async function render_response({ throw error; } - if (validation_errors) { + if (validation_error?.data || mutation_data) { try { - serialized.submitted = devalue({ - errors: validation_errors.errors, - values: validation_errors.values - }); + serialized.submitted = devalue(mutation_data || validation_error?.data); } catch (e) { // If we're here, the data could not be serialized with devalue const error = /** @type {any} */ (e); diff --git a/packages/kit/src/runtime/server/page/respond_with_error.js b/packages/kit/src/runtime/server/page/respond_with_error.js index 739bc06b3674..a0ac29d6b700 100644 --- a/packages/kit/src/runtime/server/page/respond_with_error.js +++ b/packages/kit/src/runtime/server/page/respond_with_error.js @@ -79,8 +79,7 @@ export async function respond_with_error({ event, options, state, status, error, fetched, cookies, event, - resolve_opts, - validation_errors: undefined + resolve_opts }); } catch (err) { const error = coalesce_to_error(err); diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js index 21eba4744cc7..1b37bc363826 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js @@ -6,6 +6,11 @@ import { invalid } from '@sveltejs/kit'; export const actions = { default: async ({ fields }) => { fields.delete('password'); - throw invalid(400, fields, { message: 'invalid credentials' }); + throw invalid(400, { + values: Object.fromEntries(fields), + errors: { + message: 'invalid credentials' + } + }); } }; diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js index ebe1021a6bba..3aef59d06a55 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js @@ -3,6 +3,6 @@ import { invalid } from '@sveltejs/kit'; /** @type {import('./$types').Actions} */ export const actions = { default: async () => { - throw invalid(400, undefined, { message: 'an error occurred' }); + throw invalid(400, { errors: { message: 'an error occurred' } }); } }; diff --git a/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js index a591efd7083d..44397a58ff4f 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js @@ -3,8 +3,8 @@ import { invalid } from '@sveltejs/kit'; /** @type {import('./$types').Actions} */ export const actions = { default: ({ files }) => { - throw invalid(400, undefined, { - result: files.get('file') + throw invalid(400, { + errors: { result: files.get('file') } }); } }; diff --git a/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.server.js index 9f8b97f5a846..6927e8d43fbc 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.server.js @@ -7,9 +7,9 @@ export function load() { /** @type {import('./$types').Actions} */ export const actions = { - default: ({ files }) => { + default: ({ fields }) => { return { - result: files.get('username') + result: fields.get('username') }; } }; diff --git a/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte index 892fe7082b9e..c4ad4f8dcca6 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte @@ -1,10 +1,8 @@ -
    {JSON.stringify(data)}
    - - -{#if hydrated} -
    {JSON.stringify(hydrated)}
    -{/if} +
    {JSON.stringify($submitted)}
    diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js index 690a22f31829..4dda3f5b5d33 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js @@ -9,8 +9,8 @@ export function load() { /** @type {import('./$types').Actions} */ export const actions = { default: async ({ fields }) => { - throw invalid(400, undefined, { - post_message: `echo: ${fields.get('message')}` + throw invalid(400, { + errors: { post_message: `echo: ${fields.get('message')}` } }); } }; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte index be3156b44683..f49b8280da2a 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte @@ -1,9 +1,9 @@ -

    {data.get_message} / {$form?.errors?.post_message}

    +

    {data.get_message} / {$submitted?.errors?.post_message}

    status: {$page.status}

    diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index c39f6c745fa5..d2dd57c31c73 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1764,4 +1764,18 @@ test.describe('Actions', () => { expect(await page.textContent('p')).toBe('Result: file:test.txt'); }); + + test('Success data is returned', async ({ page }) => { + await page.goto('/actions/success-data'); + + expect(await page.textContent('pre')).toBe(JSON.stringify(null)); + + await page.type('input[name="username"]', 'foo'); + await Promise.all([ + page.waitForRequest((request) => request.url().includes('/actions/success-data')), + page.click('button') + ]); + + await expect(page.locator('pre')).toHaveText(JSON.stringify({ result: 'foo' })); + }); }); diff --git a/packages/kit/types/ambient.d.ts b/packages/kit/types/ambient.d.ts index 85f5527c8898..6cb254a20340 100644 --- a/packages/kit/types/ambient.d.ts +++ b/packages/kit/types/ambient.d.ts @@ -201,7 +201,7 @@ declare module '$app/paths' { */ declare module '$app/stores' { import { Readable, Writable } from 'svelte/store'; - import { SubmittedState, Navigation, Page } from '@sveltejs/kit'; + import { Navigation, Page } from '@sveltejs/kit'; /** * A readable store whose value contains page data. @@ -221,7 +221,7 @@ declare module '$app/stores' { * A writable store whose value contain the errors and values of the last form submission. * It is updated automatically when using the `` component, else you need to take care of it yourself. */ - export const submitted: Writable; + export const submitted: Writable | null>; /** * A function that returns all of the contextual stores. On the server, this must be called during component initialization. diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 5505ae49a2a3..f168fddcef95 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -330,52 +330,40 @@ export interface ActionEvent< FileType = any > extends RequestEvent { fields: FieldsFormData; - files: FilesFormData; + files: FilesFormData; } -export class FieldsFormData extends FormData { - [Symbol.iterator](): IterableIterator<[Fields, string]>; - entries(): IterableIterator<[Fields, string]>; - keys(): IterableIterator; +export class FieldsFormData extends FormData { + [Symbol.iterator](): IterableIterator<[string, string]>; + entries(): IterableIterator<[string, string]>; + keys(): IterableIterator; values(): IterableIterator; - delete(name: Fields): void; - get(name: Fields): string | null; - getAll(name: Fields): string[]; - has(name: Fields): boolean; + delete(name: string): void; + get(name: string): string | null; + getAll(name: string): string[]; + has(name: string): boolean; set(name: string, value: string): void; forEach( - callbackfn: (value: string, key: string, parent: FieldsFormData) => void, + callbackfn: (value: string, key: string, parent: FieldsFormData) => void, thisArg?: any ): void; } -export class FilesFormData { - [Symbol.iterator](): IterableIterator<[Fields, FileFileType]>; - entries(): IterableIterator<[Fields, FileFileType]>; - keys(): IterableIterator; +export class FilesFormData { + [Symbol.iterator](): IterableIterator<[string, FileFileType]>; + entries(): IterableIterator<[string, FileFileType]>; + keys(): IterableIterator; values(): IterableIterator; - delete(name: Fields): void; - get(name: Fields): FileFileType | null; - getAll(name: Fields): FileFileType[]; - has(name: Fields): boolean; + delete(name: string): void; + get(name: string): FileFileType | null; + getAll(name: string): FileFileType[]; + has(name: string): boolean; set(name: string, value: string | Blob, fileName?: string): void; - forEach(callbackfn: (value: FileFileType, key: string, parent: FilesFormData) => void): void; -} - -export interface SubmittedState { - error: Record; - values: Record; -} - -// TODO make this a union type with type=success/redirect/error? -export interface FormFetchResponse { - status: number; - error?: Record; - values?: Record; - result?: Record; - location?: string; + forEach( + callbackfn: (value: FileFileType, key: string, parent: FilesFormData) => void + ): void; } // TODO figure out how to just re-export from '../src/index/index.js' without @@ -404,8 +392,7 @@ export function json(data: any, init?: ResponseInit): Response; /** * Generates a `ValidationError` object. */ -export function invalid( +export function invalid | undefined>( status: number, - form: FormData | Record | undefined | null, - errors: Record -): ValidationError; + data?: T +): ValidationError; From f6be2c53b153a80dcdc0e3bd8a9bab3bc8278ce7 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 2 Sep 2022 15:48:15 +0200 Subject: [PATCH 20/85] fix infered FileType --- packages/kit/src/core/sync/write_types/index.js | 2 +- .../core/sync/write_types/test/layout/_expected/$types.d.ts | 4 ++-- .../test/simple-page-server-and-shared/_expected/$types.d.ts | 4 ++-- .../test/simple-page-server-only/_expected/$types.d.ts | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/kit/src/core/sync/write_types/index.js b/packages/kit/src/core/sync/write_types/index.js index 1bdfbeb4b7f3..66b939fb8a32 100644 --- a/packages/kit/src/core/sync/write_types/index.js +++ b/packages/kit/src/core/sync/write_types/index.js @@ -195,7 +195,7 @@ function update_types(config, routes, route) { declarations.push( `type FileType = typeof import('${posixify( config.kit.files.hooks - )}.js') extends { handleFile: infer T } ? ReturnType : File;` + )}.js') extends { handleFile: (...args: any) => infer T } ? Awaited : File;` ); exports.push(`export type Action = Kit.Action`); exports.push(`export type Actions = Kit.Actions`); diff --git a/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts index d5af12a0e2c9..7553cf5b6ac4 100644 --- a/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts @@ -13,8 +13,8 @@ type OutputDataShape = MaybeWithVoid< type EnsureParentData = T extends null | undefined ? {} : T; type PageServerParentData = EnsureParentData; type PageParentData = EnsureParentData; -type FileType = typeof import('src/hooks.js') extends { handleFile: infer T } - ? ReturnType +type FileType = typeof import('src/hooks.js') extends { handleFile: (...args: any) => infer T } + ? Awaited> : File; type LayoutParams = RouteParams & {}; type LayoutServerParentData = EnsureParentData<{}>; diff --git a/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts index dc967251746b..a65fb29c8c36 100644 --- a/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts @@ -13,8 +13,8 @@ type OutputDataShape = MaybeWithVoid< type EnsureParentData = T extends null | undefined ? {} : T; type PageServerParentData = EnsureParentData; type PageParentData = EnsureParentData; -type FileType = typeof import('src/hooks.js') extends { handleFile: infer T } - ? ReturnType +type FileType = typeof import('src/hooks.js') extends { handleFile: (...args: any) => infer T } + ? Awaited> : File; type LayoutParams = RouteParams & {}; type LayoutParentData = EnsureParentData<{}>; diff --git a/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts index 4a2f7c4fc904..f2b2fa0f28d0 100644 --- a/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts @@ -13,8 +13,8 @@ type OutputDataShape = MaybeWithVoid< type EnsureParentData = T extends null | undefined ? {} : T; type PageServerParentData = EnsureParentData; type PageParentData = EnsureParentData; -type FileType = typeof import('src/hooks.js') extends { handleFile: infer T } - ? ReturnType +type FileType = typeof import('src/hooks.js') extends { handleFile: (...args: any) => infer T } + ? Awaited> : File; type LayoutParams = RouteParams & {}; type LayoutParentData = EnsureParentData<{}>; From d8e71376b84f44f24278ae984a7dcfb66318ff91 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 2 Sep 2022 17:25:19 +0200 Subject: [PATCH 21/85] give JSON response a well-defined shape --- documentation/docs/06-form-actions.md | 23 +++++++-------- .../kit/src/runtime/server/page/actions.js | 28 +++++++++++-------- .../form-errors-persist-fields/+page.svelte | 2 +- .../routes/actions/success-data/+page.svelte | 4 +-- .../kit/test/apps/basics/test/server.test.js | 4 +-- packages/kit/types/index.d.ts | 8 ++++++ 6 files changed, 41 insertions(+), 28 deletions(-) diff --git a/documentation/docs/06-form-actions.md b/documentation/docs/06-form-actions.md index dbd2008f2ddf..e0be7e552c3a 100644 --- a/documentation/docs/06-form-actions.md +++ b/documentation/docs/06-form-actions.md @@ -165,7 +165,7 @@ export const actions = { So far, all the code examples run native form submissions - that is, when the user pressed the submit button, the page is reloaded. It's good that this use case is supported since JavaScript may not be loaded all the time. When it is though, it might be a better user experience to use the powers JavaScript gives us to provide a better user experience - this is called progressive enhancement. -First we need to ensure that the page is _not_ reloaded on submission. For this, we prevent the default behavior. Afterwards, we run our JavaScript code instead which does the form submission through `fetch` instead. +First we need to ensure that the page is _not_ reloaded on submission. For this, we prevent the default behavior. Afterwards, we run our JavaScript code instead which does the form submission through `fetch` instead. The result always contains a `status` and `type` property, which is either `"success"`, `"invalid"` or `"redirect"`. In case of `"redirect"`, a location is given. In case of `"invalid"` and `"success"`, the data returned from the action function is available through the `data` property. ```svelte /// file: src/routes/login/+page.svelte @@ -174,20 +174,22 @@ First we need to ensure that the page is _not_ reloaded on submission. For this, import { invalidateAll, goto } from '$app/navigation'; async function login(event) { - event.preventDefault(); // prevent native form submission - const data = new FormData(this); // create FormData - const response = await fetch(this.action, { // call action using fetch + const data = new FormData(this); + const response = await fetch(this.action, { method: 'POST', headers: { accept: 'application/json' }, body: data }); - const { errors, location, values } = await response.json(); // destructure response object - if (response.ok) { // success, redirect + const result = await response.json(); + if (result.type === 'success' || result.type === 'redirect') { invalidateAll(); - goto(location); - } else { // validation error, update $submitted store + } + if (result.type === 'redirect') { + goto(result.location) + } + if (result.type === 'success' || result.type === 'invalid') { $submitted = { errors, values } }; } } @@ -239,9 +241,8 @@ In case you don't need your forms to work without JavaScript, you want to use HT let errors = {}; async function login(event) { - event.preventDefault(); // prevent native form submission - const data = Object.fromEntries(new FormData(this)); // create JSON from FormData - const response = await fetch('/api/login', { // call your API using fetch + const data = Object.fromEntries(new FormData(this)); + const response = await fetch('/api/login', { method: 'POST', headers: { accept: 'application/json' diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index ee44dab0b740..bd82783d1465 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -41,28 +41,25 @@ export async function handle_action_json_request(event, options, server) { } try { - const result = await call_action(event, options, actions); - if (!result) { - return new Response(undefined, { - status: 204 - }); - } else { - return json(result); - } + const data = await call_action(event, options, actions); + return action_json({ + type: 'success', + status: data ? 200 : 204, + data: /** @type {Record | undefined} */ (data) + }); } catch (e) { const error = /** @type {Redirect | HttpError | ValidationError | Error} */ (e); if (error instanceof Redirect) { - return json({ + return action_json({ + type: 'redirect', status: error.status, location: error.location }); } if (error instanceof ValidationError) { - return json(error.data, { - status: error.status - }); + return action_json({ type: 'invalid', status: error.status, data: error.data }); } if (!(error instanceof HttpError)) { @@ -75,6 +72,13 @@ export async function handle_action_json_request(event, options, server) { } } +/** + * @param {import('types').FormFetchResponse} data + */ +function action_json(data) { + return json(data); +} + /** * @param {import('types').RequestEvent} event * @param {import('types').SSRNode} leaf_node diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte index 0c3769b7b62c..679f9942941b 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte @@ -12,7 +12,7 @@ 'accept': 'application/json' } }); - const { errors, values } = await res.json(); + const { data: { errors, values } } = await res.json(); $submitted = { errors, values }; } diff --git a/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte index c4ad4f8dcca6..ee842c48892e 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte @@ -10,8 +10,8 @@ 'accept': 'application/json' } }); - const result = await res.json(); - $submitted = result; + const { data } = await res.json(); + $submitted = data; } diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index 6a47157f2ad9..c9fda674d0ee 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -265,8 +265,8 @@ test.describe('Shadowed pages', () => { } }); - expect(response.status()).toBe(204); - expect(await response.text()).toEqual(''); + expect(response.status()).toBe(200); + expect(await response.json()).toEqual({ type: 'success', status: 204 }); }); }); diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index f168fddcef95..efb4c2f3f930 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -366,6 +366,14 @@ export class FilesFormData { ): void; } +/** + * When calling a form action via fetch, the response will be one of these shapes. + */ +export type FormFetchResponse = + | { type: 'success'; status: number; data?: Record } + | { type: 'invalid'; status: number; data?: Record } + | { type: 'redirect'; status: number; location: string }; + // TODO figure out how to just re-export from '../src/index/index.js' without // breaking the site From b2e30b92fcc9b2975a0296e2cb2f05b7b806bfaf Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 2 Sep 2022 21:05:34 +0200 Subject: [PATCH 22/85] provide form state through form prop and $page.form --- documentation/docs/06-form-actions.md | 30 +++++++++++-------- packages/kit/src/core/sync/write_root.js | 8 ++--- packages/kit/src/runtime/app/stores.js | 19 +----------- packages/kit/src/runtime/client/client.js | 14 ++++----- packages/kit/src/runtime/client/singletons.js | 3 +- packages/kit/src/runtime/client/types.d.ts | 2 +- .../kit/src/runtime/server/page/render.js | 12 ++++---- .../form-errors-persist-fields/+page.svelte | 10 ++++--- .../routes/actions/form-errors/+page.svelte | 16 ++++++---- .../routes/actions/handle-file/+page.svelte | 9 +++--- .../routes/actions/success-data/+page.svelte | 8 ++--- .../routes/shadowed/error-post/+page.svelte | 7 +++-- packages/kit/test/apps/basics/test/test.js | 6 ++-- packages/kit/types/ambient.d.ts | 6 ---- packages/kit/types/index.d.ts | 1 + 15 files changed, 70 insertions(+), 81 deletions(-) diff --git a/documentation/docs/06-form-actions.md b/documentation/docs/06-form-actions.md index e0be7e552c3a..7bf4ab1b509a 100644 --- a/documentation/docs/06-form-actions.md +++ b/documentation/docs/06-form-actions.md @@ -64,7 +64,7 @@ export const actions = { ## Validation -A core part of form submissions is validation. For this, an action can `throw` the `invalid` helper method exported from `@sveltejs/kit` if there are validation errors. `invalid` expects a `status` as a required argument, and optionally anything else you want to return as a second argument. This could be the form value (make sure to remove any user sensitive information such as passwords) and an `error` object. In case of a native form submit the second argument to `invalid` populates the `$submitted` store which is available inside your components. You can use this to preserve user input. +A core part of form submissions is validation. For this, an action can `throw` the `invalid` helper method exported from `@sveltejs/kit` if there are validation errors. `invalid` expects a `status` as a required argument, and optionally anything else you want to return as a second argument. This could be the form value (make sure to remove any user sensitive information such as passwords) and an `error` object. In case of a native form submit the second argument to `invalid` populates the `$page.form` store and the `form` prop which is available inside your components. You can use this to preserve user input. ```js /// file: src/routes/login/+page.server.js @@ -109,13 +109,14 @@ export const actions = { ```svelte /// file: src/routes/login/+page.svelte - - {#if $submitted?.errors?.username} - {$submitted?.errors?.username} + + {#if form?.errors?.username} + {form?.errors?.username} {/if} @@ -124,7 +125,7 @@ export const actions = { ## Success -If everything is valid, an action can return a JSON object with data, which will be available through the `$submitted` store. Alternatively it can `throw` a `redirect` to redirect the user to another page. +If everything is valid, an action can return a JSON object with data, which will be available through the `$page.form` store and the `form` prop. Alternatively it can `throw` a `redirect` to redirect the user to another page. ```js /// file: src/routes/login/+page.server.js @@ -149,10 +150,11 @@ export const actions = { ```svelte /// file: src/routes/login/+page.svelte -{#if $submitted.success} +{#if form?.success} Login successful {/if} @@ -170,9 +172,11 @@ First we need to ensure that the page is _not_ reloaded on submission. For this, ```svelte /// file: src/routes/login/+page.svelte - - {#if $submitted?.errors.username} - {$submitted.errors.username} + + {#if form?.errors.username} + {form.errors.username} {/if} diff --git a/packages/kit/src/core/sync/write_root.js b/packages/kit/src/core/sync/write_root.js index 8112252f84a9..32a0e6a635ed 100644 --- a/packages/kit/src/core/sync/write_root.js +++ b/packages/kit/src/core/sync/write_root.js @@ -21,16 +21,16 @@ export function write_root(manifest_data, output) { let l = max_depth; - let pyramid = ``; + let pyramid = ``; while (l--) { pyramid = ` {#if components[${l + 1}]} - + ${pyramid.replace(/\n/g, '\n\t\t\t\t\t')} {:else} - + {/if} ` .replace(/^\t\t\t/gm, '') @@ -51,14 +51,12 @@ export function write_root(manifest_data, output) { export let components; ${levels.map((l) => `export let data_${l} = null;`).join('\n\t\t\t\t')} - export let submitted; if (!browser) { setContext('__svelte__', stores); } $: stores.page.set(page); - $: stores.submitted.set(submitted); afterUpdate(stores.page.notify); let mounted = false; diff --git a/packages/kit/src/runtime/app/stores.js b/packages/kit/src/runtime/app/stores.js index 17be206884f1..0eb99f4dfa08 100644 --- a/packages/kit/src/runtime/app/stores.js +++ b/packages/kit/src/runtime/app/stores.js @@ -25,8 +25,7 @@ export const getStores = () => { navigating: { subscribe: stores.navigating.subscribe }, - updated: stores.updated, - submitted: stores.submitted + updated: stores.updated }; // TODO remove this for 1.0 @@ -69,22 +68,6 @@ export const navigating = { } }; -/** @type {typeof import('$app/stores').submitted} */ -export const submitted = { - set(values) { - const store = getStores().submitted; - return store.set(values); - }, - update(value) { - const store = getStores().submitted; - return store.update(value); - }, - subscribe(fn) { - const store = getStores().submitted; - return store.subscribe(fn); - } -}; - function removed_session() { // TODO remove for 1.0 throw new Error( diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index a5e398441e94..6ef0af6aca3c 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/packages/kit/src/runtime/client/client.js @@ -410,7 +410,7 @@ export function create_client({ target, base, trailing_slash }) { * status: number; * error: HttpError | Error | null; * routeId: string | null; - * submitted?: Record | null; + * form?: Record | null; * }} opts */ async function get_navigation_result_from_branch({ @@ -420,7 +420,7 @@ export function create_client({ target, base, trailing_slash }) { status, error, routeId, - submitted + form }) { const filtered = /** @type {import('./types').BranchNode[] } */ (branch.filter(Boolean)); @@ -435,8 +435,7 @@ export function create_client({ target, base, trailing_slash }) { session_id }, props: { - components: filtered.map((branch_node) => branch_node.node.component), - submitted + components: filtered.map((branch_node) => branch_node.node.component) } }; @@ -470,7 +469,8 @@ export function create_client({ target, base, trailing_slash }) { status, url, // The whole page store is updated, but this way the object reference stays the same - data: data_changed ? data : page.data + data: data_changed ? data : page.data, + form }; // TODO remove this for 1.0 @@ -1301,7 +1301,7 @@ export function create_client({ target, base, trailing_slash }) { params, routeId, data: server_data_nodes, - submitted + form }) => { const url = new URL(location.href); @@ -1342,7 +1342,7 @@ export function create_client({ target, base, trailing_slash }) { original_error.message ) : original_error, - submitted, + form, routeId }); } catch (e) { diff --git a/packages/kit/src/runtime/client/singletons.js b/packages/kit/src/runtime/client/singletons.js index ef31ad29af4f..f1ab3986cb6c 100644 --- a/packages/kit/src/runtime/client/singletons.js +++ b/packages/kit/src/runtime/client/singletons.js @@ -17,6 +17,5 @@ export const stores = { url: notifiable_store({}), page: notifiable_store({}), navigating: writable(/** @type {import('types').Navigation | null} */ (null)), - updated: create_updated_store(), - submitted: writable(/** @type {Record | null} */ (null)) + updated: create_updated_store() }; diff --git a/packages/kit/src/runtime/client/types.d.ts b/packages/kit/src/runtime/client/types.d.ts index 89076672d7b6..0b33655afbb7 100644 --- a/packages/kit/src/runtime/client/types.d.ts +++ b/packages/kit/src/runtime/client/types.d.ts @@ -30,7 +30,7 @@ export interface Client { params: Record; routeId: string | null; data: Array; - submitted: Record | null; + form: Record | null; }) => Promise; _start_router: () => void; } diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index b01679743bbb..1df0b5ad9f44 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -82,7 +82,6 @@ export async function render_response({ stores: { page: writable(null), navigating: writable(null), - submitted: writable(null), updated }, components: await Promise.all(branch.map(({ node }) => node.component())) @@ -103,11 +102,10 @@ export async function render_response({ routeId: event.routeId, status, url: event.url, - data + data, + form: mutation_data ?? validation_error?.data ?? null }; - props.submitted = (mutation_data || validation_error?.data) ?? null; - // TODO remove this for 1.0 /** * @param {string} property @@ -175,7 +173,7 @@ export async function render_response({ /** @param {string} path */ const prefixed = (path) => (path.startsWith('/') ? path : `${assets}/${path}`); - const serialized = { data: '', submitted: 'null' }; + const serialized = { data: '', form: 'null' }; try { serialized.data = devalue(branch.map(({ server_data }) => server_data)); @@ -191,7 +189,7 @@ export async function render_response({ if (validation_error?.data || mutation_data) { try { - serialized.submitted = devalue(mutation_data || validation_error?.data); + serialized.form = devalue(mutation_data || validation_error?.data); } catch (e) { // If we're here, the data could not be serialized with devalue const error = /** @type {any} */ (e); @@ -213,7 +211,7 @@ export async function render_response({ params: ${devalue(event.params)}, routeId: ${s(event.routeId)}, data: ${serialized.data}, - submitted: ${serialized.submitted} + form: ${serialized.form} }` : 'null'}, paths: ${s(options.paths)}, target: document.querySelector('[data-sveltekit-hydrate="${target}"]').parentNode, diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte index 679f9942941b..c67f32f30d3f 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.svelte @@ -1,8 +1,10 @@ - + diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte index 99aa8695875f..1c51efda8c4b 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.svelte @@ -1,17 +1,23 @@
    -

    {$submitted?.errors?.message}

    +

    {form?.errors?.message}

    +

    {$page.form?.errors?.message}

    -{#if hydrated_error_message} -

    {hydrated_error_message}

    +{#if hydrated_error_message1} +

    {hydrated_error_message1}

    +

    {hydrated_error_message2}

    {/if} diff --git a/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte index 506725eef44e..5556f3d6ae1f 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.svelte @@ -1,10 +1,11 @@ -

    Result: {$submitted?.errors.result}

    +

    Result: {form?.errors.result}

    - - + +
    diff --git a/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte index ee842c48892e..a285b2bf056e 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/actions/success-data/+page.svelte @@ -1,8 +1,8 @@ -
    {JSON.stringify($submitted)}
    +
    {JSON.stringify(form)}
    diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte index f49b8280da2a..f1370e59596e 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte @@ -1,9 +1,10 @@ -

    {data.get_message} / {$submitted?.errors?.post_message}

    +

    {data.get_message} / {form?.errors?.post_message}

    status: {$page.status}

    diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index d2dd57c31c73..3d86631aae60 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1719,9 +1719,11 @@ test.describe('Actions', () => { test('Error props are returned', async ({ page, javaScriptEnabled }) => { await page.goto('/actions/form-errors'); await page.click('button'); - expect(await page.textContent('p.server')).toBe('an error occurred'); + expect(await page.textContent('p.server-prop')).toBe('an error occurred'); + expect(await page.textContent('p.server-store')).toBe('an error occurred'); if (javaScriptEnabled) { - expect(await page.textContent('p.client')).toBe('hydrated: an error occurred'); + expect(await page.textContent('p.client-prop')).toBe('hydrated: an error occurred'); + expect(await page.textContent('p.client-store')).toBe('hydrated: an error occurred'); } }); diff --git a/packages/kit/types/ambient.d.ts b/packages/kit/types/ambient.d.ts index 6cb254a20340..bff7dffb7920 100644 --- a/packages/kit/types/ambient.d.ts +++ b/packages/kit/types/ambient.d.ts @@ -217,11 +217,6 @@ declare module '$app/stores' { * A readable store whose initial value is `false`. If [`version.pollInterval`](https://kit.svelte.dev/docs/configuration#version) is a non-zero value, SvelteKit will poll for new versions of the app and update the store value to `true` when it detects one. `updated.check()` will force an immediate check, regardless of polling. */ export const updated: Readable & { check: () => boolean }; - /** - * A writable store whose value contain the errors and values of the last form submission. - * It is updated automatically when using the `` component, else you need to take care of it yourself. - */ - export const submitted: Writable | null>; /** * A function that returns all of the contextual stores. On the server, this must be called during component initialization. @@ -231,7 +226,6 @@ declare module '$app/stores' { navigating: typeof navigating; page: typeof page; updated: typeof updated; - submitted: typeof submitted; }; } diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index efb4c2f3f930..2effc800ceb2 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -232,6 +232,7 @@ export interface Page = Record; + form: Record | null; } export interface ParamMatcher { From 29294d06268ca341ff13aba7f1e6fd398000f33f Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 2 Sep 2022 21:09:29 +0200 Subject: [PATCH 23/85] return invalid instead of throwing it --- documentation/docs/06-form-actions.md | 4 ++-- .../kit/src/runtime/server/page/actions.js | 21 ++++++++++--------- packages/kit/src/runtime/server/page/index.js | 9 ++++++-- .../+page.server.js | 2 +- .../actions/form-errors/+page.server.js | 2 +- .../actions/handle-file/+page.server.js | 2 +- .../shadowed/error-post/+page.server.js | 2 +- 7 files changed, 24 insertions(+), 18 deletions(-) diff --git a/documentation/docs/06-form-actions.md b/documentation/docs/06-form-actions.md index 7bf4ab1b509a..25862dea860b 100644 --- a/documentation/docs/06-form-actions.md +++ b/documentation/docs/06-form-actions.md @@ -64,7 +64,7 @@ export const actions = { ## Validation -A core part of form submissions is validation. For this, an action can `throw` the `invalid` helper method exported from `@sveltejs/kit` if there are validation errors. `invalid` expects a `status` as a required argument, and optionally anything else you want to return as a second argument. This could be the form value (make sure to remove any user sensitive information such as passwords) and an `error` object. In case of a native form submit the second argument to `invalid` populates the `$page.form` store and the `form` prop which is available inside your components. You can use this to preserve user input. +A core part of form submissions is validation. For this, an action can `return` using the `invalid` helper method exported from `@sveltejs/kit` if there are validation errors. `invalid` expects a `status` as a required argument, and optionally anything else you want to return as a second argument. This could be the form value (make sure to remove any user sensitive information such as passwords) and an `error` object. In case of a native form submit the second argument to `invalid` populates the `$page.form` store and the `form` prop which is available inside your components. You can use this to preserve user input. ```js /// file: src/routes/login/+page.server.js @@ -95,7 +95,7 @@ export const actions = { const user = await db.findUser(username); if (!user) { - throw invalid(403, { + return invalid(403, { values: { username }, errors: { username: 'No user with this username' } }); diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index bd82783d1465..5cddba91c81a 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -42,13 +42,18 @@ export async function handle_action_json_request(event, options, server) { try { const data = await call_action(event, options, actions); - return action_json({ - type: 'success', - status: data ? 200 : 204, - data: /** @type {Record | undefined} */ (data) - }); + + if (data instanceof ValidationError) { + return action_json({ type: 'invalid', status: data.status, data: data.data }); + } else { + return action_json({ + type: 'success', + status: data ? 200 : 204, + data: /** @type {Record | undefined} */ (data) + }); + } } catch (e) { - const error = /** @type {Redirect | HttpError | ValidationError | Error} */ (e); + const error = /** @type {Redirect | HttpError | Error} */ (e); if (error instanceof Redirect) { return action_json({ @@ -58,10 +63,6 @@ export async function handle_action_json_request(event, options, server) { }); } - if (error instanceof ValidationError) { - return action_json({ type: 'invalid', status: error.status, data: error.data }); - } - if (!(error instanceof HttpError)) { options.handle_error(error, event); } diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index 7a730307ad08..6e50f4dca191 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/packages/kit/src/runtime/server/page/index.js @@ -63,9 +63,14 @@ export async function render_page(event, route, page, options, state, resolve_op // for action requests, first call handler in +page.server.js // (this also determines status code) try { - mutation_data = await handle_action_request(event, options, leaf_node.server); + const result = await handle_action_request(event, options, leaf_node.server); + if (result instanceof ValidationError) { + validation_error = result; + } else { + mutation_data = result; + } } catch (e) { - const error = /** @type {Redirect | HttpError | ValidationError | Error} */ (e); + const error = /** @type {Redirect | HttpError | Error} */ (e); if (error instanceof Redirect) { return redirect_response(303, error.location); } diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js index 1b37bc363826..e604aebb7100 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors-persist-fields/+page.server.js @@ -6,7 +6,7 @@ import { invalid } from '@sveltejs/kit'; export const actions = { default: async ({ fields }) => { fields.delete('password'); - throw invalid(400, { + return invalid(400, { values: Object.fromEntries(fields), errors: { message: 'invalid credentials' diff --git a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js index 3aef59d06a55..cfd99c843e4b 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/form-errors/+page.server.js @@ -3,6 +3,6 @@ import { invalid } from '@sveltejs/kit'; /** @type {import('./$types').Actions} */ export const actions = { default: async () => { - throw invalid(400, { errors: { message: 'an error occurred' } }); + return invalid(400, { errors: { message: 'an error occurred' } }); } }; diff --git a/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js index 44397a58ff4f..584538653047 100644 --- a/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/actions/handle-file/+page.server.js @@ -3,7 +3,7 @@ import { invalid } from '@sveltejs/kit'; /** @type {import('./$types').Actions} */ export const actions = { default: ({ files }) => { - throw invalid(400, { + return invalid(400, { errors: { result: files.get('file') } }); } diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js index 4dda3f5b5d33..f84c7497c2a6 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js +++ b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.server.js @@ -9,7 +9,7 @@ export function load() { /** @type {import('./$types').Actions} */ export const actions = { default: async ({ fields }) => { - throw invalid(400, { + return invalid(400, { errors: { post_message: `echo: ${fields.get('message')}` } }); } From ba4aa8fb428e0ac4715868249c7ec9a3a995d785 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 2 Sep 2022 22:29:27 +0200 Subject: [PATCH 24/85] types for actions --- .../kit/src/core/sync/write_types/index.js | 127 ++++++++++++++---- .../src/core/sync/write_types/index.spec.js | 98 +++++++++++--- .../test/layout/_expected/$types.d.ts | 5 +- .../_expected/$types.d.ts | 5 +- .../simple-page-server-only/+page.server.js | 4 + .../_expected/$types.d.ts | 7 +- .../routes/shadowed/error-post/+page.svelte | 2 + packages/kit/types/index.d.ts | 10 +- 8 files changed, 201 insertions(+), 57 deletions(-) diff --git a/packages/kit/src/core/sync/write_types/index.js b/packages/kit/src/core/sync/write_types/index.js index 66b939fb8a32..46f6f031f64c 100644 --- a/packages/kit/src/core/sync/write_types/index.js +++ b/packages/kit/src/core/sync/write_types/index.js @@ -21,9 +21,6 @@ try { const cwd = process.cwd(); -const shared_names = new Set(['load']); -const server_names = new Set(['load', 'POST', 'PUT', 'PATCH', 'DELETE']); // TODO replace with a single `action` - /** * Creates types for the whole manifest * @param {import('types').ValidatedConfig} config @@ -199,6 +196,7 @@ function update_types(config, routes, route) { ); exports.push(`export type Action = Kit.Action`); exports.push(`export type Actions = Kit.Actions`); + exports.push(`export type ActionEvent = Kit.ActionEvent`); } } @@ -276,7 +274,7 @@ function process_node(node, outdir, is_page, all_pages_have_load = true) { if (node.server) { const content = fs.readFileSync(node.server, 'utf8'); - const proxy = tweak_types(content, server_names); + const proxy = tweak_types(content, true); const basename = path.basename(node.server); if (proxy?.modified) { fs.writeFileSync(`${outdir}/proxy${basename}`, proxy.code); @@ -301,23 +299,19 @@ function process_node(node, outdir, is_page, all_pages_have_load = true) { exports.push(`export type ${prefix}ServerLoadEvent = Parameters<${prefix}ServerLoad>[0];`); if (is_page) { - let errors = 'unknown'; + let type = 'unknown'; if (proxy) { - const types = []; - for (const method of ['POST', 'PUT', 'PATCH', 'DELETE']) { - if (proxy.exports.includes(method)) { - // If the file wasn't tweaked, we can use the return type of the original file. - // The advantage is that type updates are reflected without saving. - const from = proxy.modified - ? `./proxy${replace_ext_with_js(basename)}` - : path_to_original(outdir, node.server); - - types.push(`Kit.AwaitedErrors`); - } + if (proxy.exports.includes('actions')) { + // If the file wasn't tweaked, we can use the return type of the original file. + // The advantage is that type updates are reflected without saving. + const from = proxy.modified + ? `./proxy${replace_ext_with_js(basename)}` + : path_to_original(outdir, node.server); + + type = `Kit.AwaitedActions`; } - errors = types.length ? types.join(' | ') : 'null'; } - exports.push(`export type Errors = ${errors};`); + exports.push(`export type FormData = ${type};`); } } else { server_data = 'null'; @@ -329,7 +323,7 @@ function process_node(node, outdir, is_page, all_pages_have_load = true) { if (node.shared) { const content = fs.readFileSync(node.shared, 'utf8'); - const proxy = tweak_types(content, shared_names); + const proxy = tweak_types(content, false); if (proxy?.modified) { fs.writeFileSync(`${outdir}/proxy${path.basename(node.shared)}`, proxy.code); written_proxies.push(`proxy${path.basename(node.shared)}`); @@ -432,10 +426,12 @@ function replace_ext_with_js(file_path) { /** * @param {string} content - * @param {Set} names + * @param {boolean} is_server * @returns {Proxy} */ -export function tweak_types(content, names) { +export function tweak_types(content, is_server) { + const names = new Set(is_server ? ['load', 'actions'] : ['load']); + try { let modified = false; @@ -486,6 +482,7 @@ export function tweak_types(content, names) { * @param {import('typescript').Node} value */ function replace_jsdoc_type_tags(node, value) { + let _modified = false; // @ts-ignore if (node.jsDoc) { // @ts-ignore @@ -498,22 +495,27 @@ export function tweak_types(content, names) { ts.isArrowFunction(value); if (is_fn && value.parameters?.length > 0) { + const name = ts.isIdentifier(value.parameters[0].name) + ? value.parameters[0].name.text + : 'event'; code.overwrite(tag.tagName.pos, tag.tagName.end, 'param'); code.prependRight(tag.typeExpression.pos + 1, 'Parameters<'); code.appendLeft(tag.typeExpression.end - 1, '>[0]'); - code.appendLeft(tag.typeExpression.end, ' event'); + code.appendLeft(tag.typeExpression.end, ` ${name}`); } else { code.overwrite(tag.pos, tag.end, ''); } - modified = true; + _modified = true; } } } } + modified ||= _modified; + return _modified; } ast.forEachChild((node) => { - if (ts.isFunctionDeclaration(node) && node.name?.text && names.has(node.name?.text)) { + if (ts.isFunctionDeclaration(node) && node.name?.text && node.name?.text === 'load') { // remove JSDoc comment above `export function load ...` replace_jsdoc_type_tags(node, node); } @@ -531,7 +533,7 @@ export function tweak_types(content, names) { for (const declaration of node.declarationList.declarations) { if ( ts.isIdentifier(declaration.name) && - names.has(declaration.name.text) && + declaration.name.text === 'load' && declaration.initializer ) { // edge case — remove JSDoc comment above individual export @@ -554,7 +556,6 @@ export function tweak_types(content, names) { rhs.parameters.length ) { const arg = rhs.parameters[0]; - const add_parens = content[arg.pos - 1] !== '('; if (add_parens) code.prependRight(arg.pos, '('); @@ -575,6 +576,80 @@ export function tweak_types(content, names) { modified = true; } + } else if ( + is_server && + ts.isIdentifier(declaration.name) && + declaration.name?.text === 'actions' && + declaration.initializer + ) { + // remove JSDoc comment from `export const actions = ..` + const removed = replace_jsdoc_type_tags(node, declaration.initializer); + // ... and move type to each individual action + if (removed) { + const rhs = declaration.initializer; + if (ts.isObjectLiteralExpression(rhs)) { + for (const prop of rhs.properties) { + if (ts.isPropertyAssignment(prop) && ts.isIdentifier(prop.name)) { + const rhs = prop.initializer; + const replaced = replace_jsdoc_type_tags(prop, rhs); + if ( + !replaced && + rhs && + (ts.isArrowFunction(rhs) || ts.isFunctionExpression(rhs)) && + rhs.parameters?.[0] + ) { + const name = ts.isIdentifier(rhs.parameters[0].name) + ? rhs.parameters[0].name.text + : 'event'; + code.prependRight( + rhs.pos, + `/** @param {import('./$types').ActionEvent} ${name} */ ` + ); + } + } + } + } + } + + // remove type from `export const actions: Actions ...` + if (declaration.type) { + let a = declaration.type.pos; + let b = declaration.type.end; + while (/\s/.test(content[a])) a += 1; + + const type = content.slice(a, b); + code.remove(declaration.name.end, declaration.type.end); + code.append(`;null as any as ${type};`); + modified = true; + + // ... and move type to each individual action + const rhs = declaration.initializer; + if (ts.isObjectLiteralExpression(rhs)) { + for (const prop of rhs.properties) { + if (ts.isPropertyAssignment(prop) && ts.isIdentifier(prop.name)) { + const rhs = prop.initializer; + + if ( + rhs && + (ts.isArrowFunction(rhs) || ts.isFunctionExpression(rhs)) && + rhs.parameters.length + ) { + const arg = rhs.parameters[0]; + const add_parens = content[arg.pos - 1] !== '('; + + if (add_parens) code.prependRight(arg.pos, '('); + + if (arg && !arg.type) { + code.appendLeft( + arg.name.end, + `: import('./$types').ActionEvent` + (add_parens ? ')' : '') + ); + } + } + } + } + } + } } } } diff --git a/packages/kit/src/core/sync/write_types/index.spec.js b/packages/kit/src/core/sync/write_types/index.spec.js index fd4c02e8760d..6939bc954076 100644 --- a/packages/kit/src/core/sync/write_types/index.spec.js +++ b/packages/kit/src/core/sync/write_types/index.spec.js @@ -90,21 +90,21 @@ test('Create $types with params and required return types for layout', async () test('Rewrites types for a TypeScript module', () => { const source = ` - export const GET: Get = ({ params }) => { + export const load: Get = ({ params }) => { return { a: 1 }; }; `; - const rewritten = tweak_types(source, new Set(['GET'])); + const rewritten = tweak_types(source, false); - assert.equal(rewritten?.exports, ['GET']); + assert.equal(rewritten?.exports, ['load']); assert.equal( rewritten?.code, `// @ts-nocheck - export const GET = ({ params }: Parameters[0]) => { + export const load = ({ params }: Parameters[0]) => { return { a: 1 }; @@ -115,21 +115,21 @@ test('Rewrites types for a TypeScript module', () => { test('Rewrites types for a TypeScript module without param', () => { const source = ` - export const GET: Get = () => { + export const load: Get = () => { return { a: 1 }; }; `; - const rewritten = tweak_types(source, new Set(['GET'])); + const rewritten = tweak_types(source, false); - assert.equal(rewritten?.exports, ['GET']); + assert.equal(rewritten?.exports, ['load']); assert.equal( rewritten?.code, `// @ts-nocheck - export const GET = () => { + export const load = () => { return { a: 1 }; @@ -141,22 +141,22 @@ test('Rewrites types for a TypeScript module without param', () => { test('Rewrites types for a JavaScript module with `function`', () => { const source = ` /** @type {import('./$types').Get} */ - export function GET({ params }) { + export function load({ params }) { return { a: 1 }; }; `; - const rewritten = tweak_types(source, new Set(['GET'])); + const rewritten = tweak_types(source, false); - assert.equal(rewritten?.exports, ['GET']); + assert.equal(rewritten?.exports, ['load']); assert.equal( rewritten?.code, `// @ts-nocheck /** @param {Parameters[0]} event */ - export function GET({ params }) { + export function load({ params }) { return { a: 1 }; @@ -168,22 +168,22 @@ test('Rewrites types for a JavaScript module with `function`', () => { test('Rewrites types for a JavaScript module with `const`', () => { const source = ` /** @type {import('./$types').Get} */ - export const GET = ({ params }) => { + export const load = ({ params }) => { return { a: 1 }; }; `; - const rewritten = tweak_types(source, new Set(['GET'])); + const rewritten = tweak_types(source, false); - assert.equal(rewritten?.exports, ['GET']); + assert.equal(rewritten?.exports, ['load']); assert.equal( rewritten?.code, `// @ts-nocheck /** @param {Parameters[0]} event */ - export const GET = ({ params }) => { + export const load = ({ params }) => { return { a: 1 }; @@ -195,23 +195,23 @@ test('Rewrites types for a JavaScript module with `const`', () => { test('Appends @ts-nocheck after @ts-check', () => { const source = `// @ts-check /** @type {import('./$types').Get} */ - export const GET = ({ params }) => { + export const load = ({ params }) => { return { a: 1 }; }; `; - const rewritten = tweak_types(source, new Set(['GET'])); + const rewritten = tweak_types(source, false); - assert.equal(rewritten?.exports, ['GET']); + assert.equal(rewritten?.exports, ['load']); assert.equal( rewritten?.code, `// @ts-check // @ts-nocheck /** @param {Parameters[0]} event */ - export const GET = ({ params }) => { + export const load = ({ params }) => { return { a: 1 }; @@ -220,4 +220,62 @@ test('Appends @ts-nocheck after @ts-check', () => { ); }); +test('Rewrites action types for a JavaScript module', () => { + const source = ` + /** @type {import('./$types').Actions} */ + export const actions = { + a: () => {}, + b: (param) => {}, + /** @type {import('./$types').Action} */ + c: (param) => {}, + } + `; + + const rewritten = tweak_types(source, true); + + assert.equal(rewritten?.exports, ['actions']); + assert.equal( + rewritten?.code, + `// @ts-nocheck + + /** */ + export const actions = { + a: () => {}, + b:/** @param {import('./$types').ActionEvent} param */ (param) => {}, + /** @param {Parameters[0]} param */ + c: (param) => {}, + } + ` + ); +}); + +test('Rewrites action types for a TypeScript module', () => { + const source = ` + import type { Actions, ActionEvent } from './$types'; + + export const actions: Actions = { + a: () => {}, + b: (param: ActionEvent) => {}, + c: (param) => {}, + } + `; + + const rewritten = tweak_types(source, true); + + assert.equal(rewritten?.exports, ['actions']); + assert.equal( + rewritten?.code, + `// @ts-nocheck + + import type { Actions, ActionEvent } from './$types'; + + export const actions = { + a: () => {}, + b: (param: ActionEvent) => {}, + c: (param: import('./$types').ActionEvent) => {}, + } + ;null as any as Actions;` + ); +}); + test.run(); diff --git a/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts index 7553cf5b6ac4..7c6e6889e6d1 100644 --- a/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/layout/_expected/$types.d.ts @@ -14,7 +14,7 @@ type EnsureParentData = T extends null | undefined ? {} : T; type PageServerParentData = EnsureParentData; type PageParentData = EnsureParentData; type FileType = typeof import('src/hooks.js') extends { handleFile: (...args: any) => infer T } - ? Awaited> + ? Awaited : File; type LayoutParams = RouteParams & {}; type LayoutServerParentData = EnsureParentData<{}>; @@ -26,7 +26,7 @@ export type PageServerLoad< | void > = Kit.ServerLoad; export type PageServerLoadEvent = Parameters[0]; -export type Errors = null; +export type FormData = unknown; export type PageServerData = Kit.AwaitedProperties< Awaited> >; @@ -45,6 +45,7 @@ export type PageData = Omit< >; export type Action = Kit.Action; export type Actions = Kit.Actions; +export type ActionEvent = Kit.ActionEvent; export type LayoutServerLoad< OutputData extends (Partial & Record) | void = | (Partial & Record) diff --git a/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts index a65fb29c8c36..e7d733675ca8 100644 --- a/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/simple-page-server-and-shared/_expected/$types.d.ts @@ -14,7 +14,7 @@ type EnsureParentData = T extends null | undefined ? {} : T; type PageServerParentData = EnsureParentData; type PageParentData = EnsureParentData; type FileType = typeof import('src/hooks.js') extends { handleFile: (...args: any) => infer T } - ? Awaited> + ? Awaited : File; type LayoutParams = RouteParams & {}; type LayoutParentData = EnsureParentData<{}>; @@ -25,7 +25,7 @@ export type PageServerLoad< | void > = Kit.ServerLoad; export type PageServerLoadEvent = Parameters[0]; -export type Errors = null; +export type FormData = unknown; export type PageServerData = Kit.AwaitedProperties< Awaited> >; @@ -44,5 +44,6 @@ export type PageData = Omit< >; export type Action = Kit.Action; export type Actions = Kit.Actions; +export type ActionEvent = Kit.ActionEvent; export type LayoutServerData = null; export type LayoutData = LayoutParentData; diff --git a/packages/kit/src/core/sync/write_types/test/simple-page-server-only/+page.server.js b/packages/kit/src/core/sync/write_types/test/simple-page-server-only/+page.server.js index d8d7765c6e25..918efcb130e0 100644 --- a/packages/kit/src/core/sync/write_types/test/simple-page-server-only/+page.server.js +++ b/packages/kit/src/core/sync/write_types/test/simple-page-server-only/+page.server.js @@ -3,3 +3,7 @@ export function load() { foo: 'bar' }; } + +export const actions = { + default: () => ({ foo: 'bar' }) +}; diff --git a/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts b/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts index f2b2fa0f28d0..81ac027bd84f 100644 --- a/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts +++ b/packages/kit/src/core/sync/write_types/test/simple-page-server-only/_expected/$types.d.ts @@ -14,7 +14,7 @@ type EnsureParentData = T extends null | undefined ? {} : T; type PageServerParentData = EnsureParentData; type PageParentData = EnsureParentData; type FileType = typeof import('src/hooks.js') extends { handleFile: (...args: any) => infer T } - ? Awaited> + ? Awaited : File; type LayoutParams = RouteParams & {}; type LayoutParentData = EnsureParentData<{}>; @@ -23,12 +23,15 @@ export type PageServerLoad< OutputData extends OutputDataShape = OutputDataShape > = Kit.ServerLoad; export type PageServerLoadEvent = Parameters[0]; -export type Errors = null; +export type FormData = Kit.AwaitedActions< + typeof import('../../../../../../../../+page.server.js').actions +>; export type PageServerData = Kit.AwaitedProperties< Awaited> >; export type PageData = Omit & PageServerData; export type Action = Kit.Action; export type Actions = Kit.Actions; +export type ActionEvent = Kit.ActionEvent; export type LayoutServerData = null; export type LayoutData = LayoutParentData; diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte index f1370e59596e..f4fe87f0b440 100644 --- a/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte +++ b/packages/kit/test/apps/basics/src/routes/shadowed/error-post/+page.svelte @@ -1,4 +1,6 @@ + +
    {JSON.stringify(form)}
    + diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index b1cc328eedf9..ba04f38551e2 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1786,4 +1786,17 @@ test.describe('Actions', () => { await expect(page.locator('pre')).toHaveText(JSON.stringify({ result: 'foo' })); }); + + test('updateForm works', async ({ page, javaScriptEnabled }) => { + await page.goto('/actions/update-form'); + expect(await page.textContent('pre')).toBe(JSON.stringify(null)); + + if (javaScriptEnabled) { + await page.click('button'); + await expect(page.locator('pre')).toHaveText(JSON.stringify({ count: 0 })); + + await page.click('button'); + await expect(page.locator('pre')).toHaveText(JSON.stringify({ count: 1 })); + } + }); }); diff --git a/packages/kit/types/ambient.d.ts b/packages/kit/types/ambient.d.ts index ff6bf1e98292..1b7bfe112b5a 100644 --- a/packages/kit/types/ambient.d.ts +++ b/packages/kit/types/ambient.d.ts @@ -159,6 +159,11 @@ declare module '$app/navigation' { */ export function prefetchRoutes(routes?: string[]): Promise; + /** + * Programmatically update the `$page.form` store with the given `data`. Also updates the `form` prop, if not set otherwise through the second argument. + */ + export function updateForm(data: Record | null): Promise; + /** * A navigation interceptor that triggers before we navigate to a new URL, whether by clicking a link, calling `goto(...)`, or using the browser back/forward controls. * Calling `cancel()` will prevent the navigation from completing. From 792b65987a9f1d69af172e765f30cf6860d178e9 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Sat, 3 Sep 2022 22:45:25 +0200 Subject: [PATCH 27/85] making a start at enhance --- documentation/docs/06-form-actions.md | 26 +++++--- packages/kit/src/core/sync/write_root.js | 4 +- packages/kit/src/exports/index.js | 59 +++++++++++++++++++ packages/kit/src/runtime/control.js | 53 +++++++++++++++++ .../kit/src/runtime/server/page/actions.js | 2 + .../routes/actions/enhance/+page.server.js | 15 +++++ .../src/routes/actions/enhance/+page.svelte | 13 ++++ packages/kit/types/index.d.ts | 21 +++++++ 8 files changed, 182 insertions(+), 11 deletions(-) create mode 100644 packages/kit/test/apps/basics/src/routes/actions/enhance/+page.server.js create mode 100644 packages/kit/test/apps/basics/src/routes/actions/enhance/+page.svelte diff --git a/documentation/docs/06-form-actions.md b/documentation/docs/06-form-actions.md index 25862dea860b..decea9ebcbdb 100644 --- a/documentation/docs/06-form-actions.md +++ b/documentation/docs/06-form-actions.md @@ -209,31 +209,39 @@ First we need to ensure that the page is _not_ reloaded on submission. For this, ``` -## `
    ` component +## `use:enhance` -As you can see, progressive enhancement is doable, but it may become a little cumbersome over time. That's why we will soon provide an opinionated wrapper component which does all the heavy lifting for you. Here's how the same login page would look like using the `` component: +As you can see, progressive enhancement is doable, but it may become a little cumbersome over time. That's why we provide a small `enhance` action which does most of the heavy lifting for you. Here's how the same login page would look like using the `enhance` action: ```svelte /// file: src/routes/login/+page.svelte - - - {#if errors?.username} - {errors.username} + + + {#if forms?.errors?.username} + {forms.errors.username} {/if} -
    + ``` +The `enhance` action is still pretty low-level, but it allows you to create your own wrapper components from it - or you can come up with your own action. + ## Alternatives In case you don't need your forms to work without JavaScript, you want to use HTTP verbs other than `POST`, or you want to send arbitrary JSON instead of being restricted to `FormData`, then you can resort to interacting with your API through `+server.js` endpoints (which will be possible to place next to `+page` files, soon). diff --git a/packages/kit/src/core/sync/write_root.js b/packages/kit/src/core/sync/write_root.js index 32a0e6a635ed..7111171a661e 100644 --- a/packages/kit/src/core/sync/write_root.js +++ b/packages/kit/src/core/sync/write_root.js @@ -21,12 +21,12 @@ export function write_root(manifest_data, output) { let l = max_depth; - let pyramid = ``; + let pyramid = ``; while (l--) { pyramid = ` {#if components[${l + 1}]} - + ${pyramid.replace(/\n/g, '\n\t\t\t\t\t')} {:else} diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index ab5344732c22..19ccf585c005 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -52,3 +52,62 @@ export function json(data, init) { export function invalid(status, data) { return new ValidationError(status, data); } + +/** @type {import('types').enhance} */ +export function enhance(form, { pending, error, invalid, redirect, result } = {}) { + /** @type {unknown} */ + let current_token; + + /** @param {SubmitEvent} event */ + async function handle_submit(event) { + const token = (current_token = {}); + + event.preventDefault(); + + const data = new FormData(form); + + if (pending) pending({ data, form }); + + try { + const response = await fetch(form.action, { + method: 'POST', + headers: { + accept: 'application/json' + }, + body: data + }); + + if (token !== current_token) return; + + if (response.ok) { + /** @type {import('types').FormFetchResponse} */ + const json = await response.json(); + if (json.type === 'success' && result) { + result({ data, form, response: /** @type {any} */ (json.data) }); + } else if (json.type === 'invalid' && invalid) { + invalid({ data, form, response: /** @type {any} */ (json.data) }); + } else if (json.type === 'redirect' && redirect) { + redirect({ data, form, location: json.location }); + } + } else if (error) { + error({ data, form, error: null, response }); + } else { + console.error(await response.text()); + } + } catch (err) { + if (error && err instanceof Error) { + error({ data, form, error: err, response: null }); + } else { + throw err; + } + } + } + + form.addEventListener('submit', handle_submit); + + return { + destroy() { + form.removeEventListener('submit', handle_submit); + } + }; +} diff --git a/packages/kit/src/runtime/control.js b/packages/kit/src/runtime/control.js index 7ef4be6781d1..680ed5d5cb82 100644 --- a/packages/kit/src/runtime/control.js +++ b/packages/kit/src/runtime/control.js @@ -45,3 +45,56 @@ export class ValidationError { this.data = data; } } + +/** + * Creates an `HttpError` object with an HTTP status code and an optional message. + * This object, if thrown during request handling, will cause SvelteKit to + * return an error response without invoking `handleError` + * @param {number} status + * @param {string | undefined} [message] + */ +export function error(status, message) { + return new HttpError(status, message); +} + +/** + * Creates a `Redirect` object. If thrown during request handling, SvelteKit will + * return a redirect response. + * @param {number} status + * @param {string} location + */ +export function redirect(status, location) { + if (isNaN(status) || status < 300 || status > 399) { + throw new Error('Invalid status code'); + } + + return new Redirect(status, location); +} + +/** + * Generates a JSON `Response` object from the supplied data. + * @param {any} data + * @param {ResponseInit} [init] + */ +export function json(data, init) { + // TODO deprecate this in favour of `Response.json` when it's + // more widely supported + const headers = new Headers(init?.headers); + if (!headers.has('content-type')) { + headers.set('content-type', 'application/json'); + } + + return new Response(JSON.stringify(data), { + ...init, + headers + }); +} + +/** + * Generates a `ValidationError` object. + * @param {number} status + * @param {Record | undefined} [data] + */ +export function invalid(status, data) { + return new ValidationError(status, data); +} diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 5cddba91c81a..65f55cc59f2c 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -291,3 +291,5 @@ class Iterator { return this; } } + +// enhance action lives in exports due to otherwise cyclic references diff --git a/packages/kit/test/apps/basics/src/routes/actions/enhance/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/enhance/+page.server.js new file mode 100644 index 000000000000..6927e8d43fbc --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/enhance/+page.server.js @@ -0,0 +1,15 @@ +/** @type {import('./$types').PageServerLoad} */ +export function load() { + return { + initial: 'initial' + }; +} + +/** @type {import('./$types').Actions} */ +export const actions = { + default: ({ fields }) => { + return { + result: fields.get('username') + }; + } +}; diff --git a/packages/kit/test/apps/basics/src/routes/actions/enhance/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/enhance/+page.svelte new file mode 100644 index 000000000000..521f51288b93 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/enhance/+page.svelte @@ -0,0 +1,13 @@ + + +
    {JSON.stringify(form)}
    + +
    form = response }}> + + +
    diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index d91c69c0a0ed..de7bd60691c0 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -412,3 +412,24 @@ export function invalid | undefined>( status: number, data?: T ): ValidationError; + +/** + * This action enhances a `
    ` element that otherwise would work without JavaScript. + * @param form The form element + * @param options Callbacks for different states of the form lifecycle, and an option to control whether or not `updateForm` and `invalidateAll` should be called on your behalf + */ +export function enhance, Invalid = Record>( + form: HTMLFormElement, + options: { + pending?: ({ data, form }: { data: FormData; form: HTMLFormElement }) => void; + invalid?: (input: { data: FormData; form: HTMLFormElement; response: Invalid }) => void; + error?: (input: { + data: FormData; + form: HTMLFormElement; + response: Response | null; + error: Error | null; + }) => void; + redirect?: (input: { data: FormData; form: HTMLFormElement; location: string }) => void; + result?: (input: { data: FormData; response: Success; form: HTMLFormElement }) => void; + } +): { destroy: () => void }; From ca3cae93ce391fd06471d165c1d09a9333b2ec0c Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Sat, 3 Sep 2022 23:29:50 +0200 Subject: [PATCH 28/85] bye bye method overrides --- documentation/docs/15-configuration.md | 11 ----- .../shared/+default+checkjs/svelte.config.js | 7 +-- .../+default+typescript/svelte.config.js | 7 +-- .../+default-typescript/svelte.config.js | 7 +-- packages/kit/src/core/config/index.spec.js | 5 +- packages/kit/src/core/config/options.js | 18 ++------ .../src/exports/vite/build/build_server.js | 1 - packages/kit/src/exports/vite/dev/index.js | 1 - packages/kit/src/runtime/client/client.js | 13 ++++++ packages/kit/src/runtime/server/index.js | 25 ---------- .../src/routes/method-override/+page.js | 6 --- .../src/routes/method-override/+page.svelte | 31 ------------- .../method-override/fetch.json/+server.js | 27 ----------- .../kit/test/apps/basics/svelte.config.js | 3 -- packages/kit/test/apps/basics/test/test.js | 46 ------------------- .../kit/test/apps/writes/svelte.config.js | 3 -- packages/kit/types/index.d.ts | 4 -- packages/kit/types/internal.d.ts | 6 --- 18 files changed, 21 insertions(+), 200 deletions(-) delete mode 100644 packages/kit/test/apps/basics/src/routes/method-override/+page.js delete mode 100644 packages/kit/test/apps/basics/src/routes/method-override/+page.svelte delete mode 100644 packages/kit/test/apps/basics/src/routes/method-override/fetch.json/+server.js diff --git a/documentation/docs/15-configuration.md b/documentation/docs/15-configuration.md index 6fe48cd98585..dbeda65a9cd9 100644 --- a/documentation/docs/15-configuration.md +++ b/documentation/docs/15-configuration.md @@ -43,10 +43,6 @@ const config = { errorTemplate: 'src/error.html' }, inlineStyleThreshold: 0, - methodOverride: { - parameter: '_method', - allowed: [] - }, moduleExtensions: ['.js', '.ts'], outDir: '.svelte-kit', paths: { @@ -197,13 +193,6 @@ Inline CSS inside a `