- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 2.1k
          breaking: invalid now must be imported from @sveltejs/kit
          #14768
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@sveltejs/kit': patch | ||
| --- | ||
|  | ||
| breaking: `invalid` now must be imported from `@sveltejs/kit` | 
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|  | @@ -452,11 +452,12 @@ Alternatively, you could use `select` and `select multiple`: | |||||||||||||
|  | ||||||||||||||
| ### Programmatic validation | ||||||||||||||
|  | ||||||||||||||
| In addition to declarative schema validation, you can programmatically mark fields as invalid inside the form handler using the `invalid` function. This is useful for cases where you can't know if something is valid until you try to perform some action: | ||||||||||||||
| In addition to declarative schema validation, you can programmatically mark fields as invalid inside the form handler using the `invalid` function. This is useful for cases where you can't know if something is valid until you try to perform some action. Just like `redirect` or `error`, `invalid` throws. It expects a list of standard-schema-compliant issues. Use the `issue` parameter for type-safe creation of such issues: | ||||||||||||||
|  | ||||||||||||||
| ```js | ||||||||||||||
| /// file: src/routes/shop/data.remote.js | ||||||||||||||
| import * as v from 'valibot'; | ||||||||||||||
| import { invalid } from '@sveltejs/kit'; | ||||||||||||||
| import { form } from '$app/server'; | ||||||||||||||
| import * as db from '$lib/server/database'; | ||||||||||||||
|  | ||||||||||||||
|  | @@ -467,13 +468,17 @@ export const buyHotcakes = form( | |||||||||||||
| v.minValue(1, 'you must buy at least one hotcake') | ||||||||||||||
| ) | ||||||||||||||
| }), | ||||||||||||||
| async (data, invalid) => { | ||||||||||||||
| async (data, issue) => { | ||||||||||||||
| try { | ||||||||||||||
| await db.buy(data.qty); | ||||||||||||||
| } catch (e) { | ||||||||||||||
| if (e.code === 'OUT_OF_STOCK') { | ||||||||||||||
| invalid( | ||||||||||||||
| invalid.qty(`we don't have enough hotcakes`) | ||||||||||||||
| // This will show up on the root issue list | ||||||||||||||
| 'Purchase failed', | ||||||||||||||
| // Creates a `{ message: ..., path: ['qty'] }` object, | ||||||||||||||
| // will show up on the issue list for the `qty` field | ||||||||||||||
| issue.qty(`we don't have enough hotcakes`) | ||||||||||||||
| 
      Comment on lines
    
      +477
     to 
      +481
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know that the previous example didn't illustrate root-level issues, but I think that showing both like this makes things more confusing rather than less — it suggests that you're supposed to add a root-level issue alongside a field-level issue which I would definitely consider unusual. I think we're better off reverting, and relying on the prose above 
        Suggested change
       
 | ||||||||||||||
| ); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|  | ||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,6 @@ | ||||||||||||||||||||||
| import { HttpError, Redirect, ActionFailure } from './internal/index.js'; | ||||||||||||||||||||||
| /** @import { StandardSchemaV1 } from '@standard-schema/spec' */ | ||||||||||||||||||||||
|  | ||||||||||||||||||||||
| import { HttpError, Redirect, ActionFailure, ValidationError } from './internal/index.js'; | ||||||||||||||||||||||
| import { BROWSER, DEV } from 'esm-env'; | ||||||||||||||||||||||
| import { | ||||||||||||||||||||||
| add_data_suffix, | ||||||||||||||||||||||
|  | @@ -215,6 +217,52 @@ export function isActionFailure(e) { | |||||||||||||||||||||
| return e instanceof ActionFailure; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|  | ||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * Use this to throw a validation error to imperatively fail form validation. | ||||||||||||||||||||||
| * Can be used in combination with `issue` passed to form actions to create field-specific issues. | ||||||||||||||||||||||
| * | ||||||||||||||||||||||
| * @example | ||||||||||||||||||||||
| * ```ts | ||||||||||||||||||||||
| * import { invalid } from '@sveltejs/kit'; | ||||||||||||||||||||||
| * import { form } from '$app/server'; | ||||||||||||||||||||||
| * import * as v from 'valibot'; | ||||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can save ourselves some space by making this an import, and I think we should do 'login' rather than 'register' (reasons below) 
        Suggested change
       
 | ||||||||||||||||||||||
| * | ||||||||||||||||||||||
| * function tryRegisterUser(name: string, password: string) { | ||||||||||||||||||||||
| * // ... | ||||||||||||||||||||||
| * } | ||||||||||||||||||||||
| 
      Comment on lines
    
      +229
     to 
      +232
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
        Suggested change
       
 | ||||||||||||||||||||||
| * | ||||||||||||||||||||||
| * export const register = form( | ||||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
        Suggested change
       
 | ||||||||||||||||||||||
| * v.object({ name: v.string(), _password: v.string() }), | ||||||||||||||||||||||
| * async ({ name, _password }, issue) => { | ||||||||||||||||||||||
| * const success = tryRegisterUser(name, _password); | ||||||||||||||||||||||
| * if (!success) { | ||||||||||||||||||||||
| * invalid('Registration failed', issue.name('This username is already taken')); | ||||||||||||||||||||||
| * } | ||||||||||||||||||||||
| 
      Comment on lines
    
      +236
     to 
      +240
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the same reason as https://github.com/sveltejs/kit/pull/14768/files#r2444740445 I think we should avoid mixing and matching here — this example reads very much as though 'Registration failed' is the title of the issue and 'This username is already taken' is the detail. In reality you would only create the  Given that the suggestion shows the use of  
        Suggested change
       
 | ||||||||||||||||||||||
| * | ||||||||||||||||||||||
| * // ... | ||||||||||||||||||||||
| * } | ||||||||||||||||||||||
| * ); | ||||||||||||||||||||||
| * ``` | ||||||||||||||||||||||
| * @param {...(StandardSchemaV1.Issue | string)} issues | ||||||||||||||||||||||
| * @returns {never} | ||||||||||||||||||||||
| * @since 2.47.3 | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| export function invalid(...issues) { | ||||||||||||||||||||||
| throw new ValidationError( | ||||||||||||||||||||||
| issues.map((issue) => (typeof issue === 'string' ? { message: issue } : issue)) | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|  | ||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * Checks whether this is an validation error thrown by {@link invalid}. | ||||||||||||||||||||||
| * @param {unknown} e The object to check. | ||||||||||||||||||||||
| * @return {e is import('./public.js').ActionFailure} | ||||||||||||||||||||||
| * @since 2.47.3 | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| export function isValidationError(e) { | ||||||||||||||||||||||
| return e instanceof ValidationError; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|  | ||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * Strips possible SvelteKit-internal suffixes and trailing slashes from the URL pathname. | ||||||||||||||||||||||
| * Returns the normalized URL as well as a method for adding the potential suffix back | ||||||||||||||||||||||
|  | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||||
| /** @import { RemoteFormInput, RemoteForm } from '@sveltejs/kit' */ | ||||||||||
| /** @import { RemoteFormInput, RemoteForm, InvalidField } from '@sveltejs/kit' */ | ||||||||||
| /** @import { InternalRemoteFormIssue, MaybePromise, RemoteInfo } from 'types' */ | ||||||||||
| /** @import { StandardSchemaV1 } from '@standard-schema/spec' */ | ||||||||||
| import { get_request_store } from '@sveltejs/kit/internal/server'; | ||||||||||
|  | @@ -13,6 +13,7 @@ import { | |||||||||
| flatten_issues | ||||||||||
| } from '../../../form-utils.svelte.js'; | ||||||||||
| import { get_cache, run_remote_function } from './shared.js'; | ||||||||||
| import { ValidationError } from '@sveltejs/kit/internal'; | ||||||||||
|  | ||||||||||
| /** | ||||||||||
| * Creates a form object that can be spread onto a `<form>` element. | ||||||||||
|  | @@ -21,7 +22,7 @@ import { get_cache, run_remote_function } from './shared.js'; | |||||||||
| * | ||||||||||
| * @template Output | ||||||||||
| * @overload | ||||||||||
| * @param {(invalid: import('@sveltejs/kit').Invalid<void>) => MaybePromise<Output>} fn | ||||||||||
| * @param {() => MaybePromise<Output>} fn | ||||||||||
| * @returns {RemoteForm<void, Output>} | ||||||||||
| * @since 2.27 | ||||||||||
| */ | ||||||||||
|  | @@ -34,7 +35,7 @@ import { get_cache, run_remote_function } from './shared.js'; | |||||||||
| * @template Output | ||||||||||
| * @overload | ||||||||||
| * @param {'unchecked'} validate | ||||||||||
| * @param {(data: Input, invalid: import('@sveltejs/kit').Invalid<Input>) => MaybePromise<Output>} fn | ||||||||||
| * @param {(data: Input, issue: InvalidField<Input>) => MaybePromise<Output>} fn | ||||||||||
| * @returns {RemoteForm<Input, Output>} | ||||||||||
| * @since 2.27 | ||||||||||
| */ | ||||||||||
|  | @@ -47,15 +48,15 @@ import { get_cache, run_remote_function } from './shared.js'; | |||||||||
| * @template Output | ||||||||||
| * @overload | ||||||||||
| * @param {Schema} validate | ||||||||||
| * @param {(data: StandardSchemaV1.InferOutput<Schema>, invalid: import('@sveltejs/kit').Invalid<StandardSchemaV1.InferInput<Schema>>) => MaybePromise<Output>} fn | ||||||||||
| * @param {(data: StandardSchemaV1.InferOutput<Schema>, issue: InvalidField<StandardSchemaV1.InferInput<Schema>>) => MaybePromise<Output>} fn | ||||||||||
| * @returns {RemoteForm<StandardSchemaV1.InferInput<Schema>, Output>} | ||||||||||
| * @since 2.27 | ||||||||||
| */ | ||||||||||
| /** | ||||||||||
| * @template {RemoteFormInput} Input | ||||||||||
| * @template Output | ||||||||||
| * @param {any} validate_or_fn | ||||||||||
| * @param {(data_or_invalid: any, invalid?: any) => MaybePromise<Output>} [maybe_fn] | ||||||||||
| * @param {(data_or_issue: any, issue?: any) => MaybePromise<Output>} [maybe_fn] | ||||||||||
| * @returns {RemoteForm<Input, Output>} | ||||||||||
| * @since 2.27 | ||||||||||
| */ | ||||||||||
|  | @@ -165,7 +166,7 @@ export function form(validate_or_fn, maybe_fn) { | |||||||||
|  | ||||||||||
| state.refreshes ??= {}; | ||||||||||
|  | ||||||||||
| const invalid = create_invalid(); | ||||||||||
| const issue = create_issues(); | ||||||||||
|  | ||||||||||
| try { | ||||||||||
| output.result = await run_remote_function( | ||||||||||
|  | @@ -174,7 +175,7 @@ export function form(validate_or_fn, maybe_fn) { | |||||||||
| true, | ||||||||||
| data, | ||||||||||
| (d) => d, | ||||||||||
| (data) => (!maybe_fn ? fn(invalid) : fn(data, invalid)) | ||||||||||
| (data) => (!maybe_fn ? fn() : fn(data, issue)) | ||||||||||
| ); | ||||||||||
| } catch (e) { | ||||||||||
| if (e instanceof ValidationError) { | ||||||||||
|  | @@ -329,89 +330,72 @@ function handle_issues(output, issues, is_remote_request, form_data) { | |||||||||
|  | ||||||||||
| /** | ||||||||||
| * Creates an invalid function that can be used to imperatively mark form fields as invalid | ||||||||||
| * @returns {import('@sveltejs/kit').Invalid} | ||||||||||
| * @returns {InvalidField<any>} | ||||||||||
| */ | ||||||||||
| function create_invalid() { | ||||||||||
| /** | ||||||||||
| * @param {...(string | StandardSchemaV1.Issue)} issues | ||||||||||
| * @returns {never} | ||||||||||
| */ | ||||||||||
| function invalid(...issues) { | ||||||||||
| throw new ValidationError( | ||||||||||
| issues.map((issue) => { | ||||||||||
| if (typeof issue === 'string') { | ||||||||||
| return { | ||||||||||
| path: [], | ||||||||||
| message: issue | ||||||||||
| }; | ||||||||||
| function create_issues() { | ||||||||||
| return /** @type {InvalidField<any>} */ ( | ||||||||||
| new Proxy( | ||||||||||
| /** @param {string} message */ | ||||||||||
| (message) => { | ||||||||||
| // TODO 3.0 remove | ||||||||||
| if (typeof message !== 'string') { | ||||||||||
| throw new Error( | ||||||||||
| 'invalid() should now be imported from @sveltejs/kit to throw validaition issues. ' + | ||||||||||
| 'Keep using the parameter (now named issue) provided to the form function only to construct the issues, e.g. invalid(issue.field("message")). ' + | ||||||||||
| 
      Comment on lines
    
      +343
     to 
      +344
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
        Suggested change
       
 | ||||||||||
| 'For more info see https://github.com/sveltejs/kit/pulls/14768' | ||||||||||
| ); | ||||||||||
| } | ||||||||||
|  | ||||||||||
| return issue; | ||||||||||
| }) | ||||||||||
| ); | ||||||||||
| } | ||||||||||
|  | ||||||||||
| return /** @type {import('@sveltejs/kit').Invalid} */ ( | ||||||||||
| new Proxy(invalid, { | ||||||||||
| get(target, prop) { | ||||||||||
| if (typeof prop === 'symbol') return /** @type {any} */ (target)[prop]; | ||||||||||
| return create_issue(message); | ||||||||||
| }, | ||||||||||
| { | ||||||||||
| get(target, prop) { | ||||||||||
| if (typeof prop === 'symbol') return /** @type {any} */ (target)[prop]; | ||||||||||
|  | ||||||||||
| /** | ||||||||||
| * @param {string} message | ||||||||||
| * @param {(string | number)[]} path | ||||||||||
| * @returns {StandardSchemaV1.Issue} | ||||||||||
| */ | ||||||||||
| const create_issue = (message, path = []) => ({ | ||||||||||
| message, | ||||||||||
| path | ||||||||||
| }); | ||||||||||
|  | ||||||||||
| return create_issue_proxy(prop, create_issue, []); | ||||||||||
| return create_issue_proxy(prop, []); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| }) | ||||||||||
| ) | ||||||||||
| ); | ||||||||||
| } | ||||||||||
|  | ||||||||||
| /** | ||||||||||
| * Error thrown when form validation fails imperatively | ||||||||||
| */ | ||||||||||
| class ValidationError extends Error { | ||||||||||
| /** | ||||||||||
| * @param {StandardSchemaV1.Issue[]} issues | ||||||||||
| * @param {string} message | ||||||||||
| * @param {(string | number)[]} path | ||||||||||
| * @returns {StandardSchemaV1.Issue} | ||||||||||
| */ | ||||||||||
| constructor(issues) { | ||||||||||
| super('Validation failed'); | ||||||||||
| this.name = 'ValidationError'; | ||||||||||
| this.issues = issues; | ||||||||||
| function create_issue(message, path = []) { | ||||||||||
| return { | ||||||||||
| message, | ||||||||||
| path | ||||||||||
| }; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|  | ||||||||||
| /** | ||||||||||
| * Creates a proxy that builds up a path and returns a function to create an issue | ||||||||||
| * @param {string | number} key | ||||||||||
| * @param {(message: string, path: (string | number)[]) => StandardSchemaV1.Issue} create_issue | ||||||||||
| * @param {(string | number)[]} path | ||||||||||
| */ | ||||||||||
| function create_issue_proxy(key, create_issue, path) { | ||||||||||
| const new_path = [...path, key]; | ||||||||||
|  | ||||||||||
| /** | ||||||||||
| * @param {string} message | ||||||||||
| * @returns {StandardSchemaV1.Issue} | ||||||||||
| * Creates a proxy that builds up a path and returns a function to create an issue | ||||||||||
| * @param {string | number} key | ||||||||||
| * @param {(string | number)[]} path | ||||||||||
| */ | ||||||||||
| const issue_func = (message) => create_issue(message, new_path); | ||||||||||
| function create_issue_proxy(key, path) { | ||||||||||
| const new_path = [...path, key]; | ||||||||||
|  | ||||||||||
| return new Proxy(issue_func, { | ||||||||||
| get(target, prop) { | ||||||||||
| if (typeof prop === 'symbol') return /** @type {any} */ (target)[prop]; | ||||||||||
| /** | ||||||||||
| * @param {string} message | ||||||||||
| * @returns {StandardSchemaV1.Issue} | ||||||||||
| */ | ||||||||||
| const issue_func = (message) => create_issue(message, new_path); | ||||||||||
|  | ||||||||||
| // Handle array access like invalid.items[0] | ||||||||||
| if (/^\d+$/.test(prop)) { | ||||||||||
| return create_issue_proxy(parseInt(prop, 10), create_issue, new_path); | ||||||||||
| } | ||||||||||
| return new Proxy(issue_func, { | ||||||||||
| get(target, prop) { | ||||||||||
| if (typeof prop === 'symbol') return /** @type {any} */ (target)[prop]; | ||||||||||
|  | ||||||||||
| // Handle property access like invalid.field.nested | ||||||||||
| return create_issue_proxy(prop, create_issue, new_path); | ||||||||||
| } | ||||||||||
| }); | ||||||||||
| // Handle array access like invalid.items[0] | ||||||||||
| if (/^\d+$/.test(prop)) { | ||||||||||
| return create_issue_proxy(parseInt(prop, 10), new_path); | ||||||||||
| } | ||||||||||
|  | ||||||||||
| // Handle property access like invalid.field.nested | ||||||||||
| return create_issue_proxy(prop, new_path); | ||||||||||
| } | ||||||||||
| }); | ||||||||||
| } | ||||||||||
| } | ||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.