Skip to content

Commit

Permalink
fix: Adjust fail method and ActionFailure type
Browse files Browse the repository at this point in the history
- add ActionFailure interface and use that publicly instead of the class. Prevents the false impression that you could do "instanceof" on the return type, fixes #10361
- add uniqueSymbol to ActionFailure instance so we can distinguish it from a regular return with the same shape
- fix setup to actually run test/types
  • Loading branch information
dummdidumm committed Dec 11, 2023
1 parent 608e3ef commit bf0282b
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 20 deletions.
5 changes: 5 additions & 0 deletions .changeset/swift-clocks-kneel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: Adjust fail method and ActionFailure type
2 changes: 1 addition & 1 deletion packages/kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
],
"scripts": {
"lint": "prettier --config ../../.prettierrc --check .",
"check": "tsc",
"check": "tsc && cd ./test/types && tsc",
"check:all": "tsc && pnpm -r --filter=\"./**\" check",
"format": "prettier --config ../../.prettierrc --write .",
"test": "pnpm test:unit && pnpm test:integration",
Expand Down
21 changes: 19 additions & 2 deletions packages/kit/src/exports/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,31 @@ export function text(body, init) {
});
}

/**
* Create an `ActionFailure` object.
* @param {number} status The [HTTP status code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#client_error_responses). Must be in the range 400-599.
* @overload
* @param {number} status
* @returns {import('./public.js').ActionFailure<undefined>}
*/
/**
* Create an `ActionFailure` object.
* @template {Record<string, unknown> | undefined} [T=undefined]
* @param {number} status The [HTTP status code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#client_error_responses). Must be in the range 400-599.
* @param {T} [data] Data associated with the failure (e.g. validation errors)
* @returns {ActionFailure<T>}
* @param {T} data Data associated with the failure (e.g. validation errors)
* @overload
* @param {number} status
* @param {T} data
* @returns {import('./public.js').ActionFailure<T>}
*/
/**
* Create an `ActionFailure` object.
* @param {number} status
* @param {any} [data]
* @returns {import('./public.js').ActionFailure<any>}
*/
export function fail(status, data) {
// @ts-expect-error unique symbol missing
return new ActionFailure(status, data);
}

Expand Down
10 changes: 8 additions & 2 deletions packages/kit/src/exports/public.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ import {
RequestOptions,
RouteSegment
} from '../types/private.js';
import { ActionFailure } from '../runtime/control.js';
import { BuildData, SSRNodeLoader, SSRRoute, ValidatedConfig } from 'types';
import type { PluginOptions } from '@sveltejs/vite-plugin-svelte';

export { PrerenderOption } from '../types/private.js';
export { ActionFailure };

/**
* [Adapters](https://kit.svelte.dev/docs/adapters) are responsible for taking the production build and turning it into something that can be deployed to a platform of your choosing.
Expand Down Expand Up @@ -58,6 +56,14 @@ type OptionalUnion<
A extends keyof U = U extends U ? keyof U : never
> = U extends unknown ? { [P in Exclude<A, keyof U>]?: never } & U : never;

declare const uniqueSymbol: unique symbol;

export interface ActionFailure<T extends Record<string, unknown> | undefined = undefined> {
status: number;
data: T;
[uniqueSymbol]: true; // necessary or else UnpackValidationError could wrongly unpack objects with the same shape as ActionFailure
}

type UnpackValidationError<T> = T extends ActionFailure<infer X>
? X
: T extends void
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/runtime/control.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class Redirect {
export class ActionFailure {
/**
* @param {number} status
* @param {T} [data]
* @param {T} data
*/
constructor(status, data) {
this.status = status;
Expand Down
13 changes: 12 additions & 1 deletion packages/kit/test/types/actions.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import Kit from '@sveltejs/kit';
import * as Kit from '@sveltejs/kit';

// Test: Action types inferred correctly and transformed into a union
type Actions = {
foo: () => Promise<void>;
bar: () => Promise<{ success: boolean } | Kit.ActionFailure<{ message: string }>>;
baz: () => Kit.ActionFailure<{ foo: string }> | { status: number; data: string };
};

let form: Kit.AwaitedActions<Actions> = null as any;
Expand All @@ -23,3 +24,13 @@ form2.message = '';
form2.success = true;
// @ts-expect-error - cannot both be present at the same time
form2 = { message: '', success: true };

// Test: ActionFailure is correctly infered to be different from the normal return type even if they have the same shape
type Actions3 = {
bar: () => Kit.ActionFailure<{ foo: string }> | { status: number; data: { bar: string } };
};
let form3: Kit.AwaitedActions<Actions3> = null as any;
form3.foo = '';
form3.status = 200;
// @ts-expect-error - cannot both be present at the same time
form3 = { foo: '', status: 200 };
18 changes: 5 additions & 13 deletions packages/kit/test/types/load.test.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,12 @@
import Kit, { Deferred } from '@sveltejs/kit';
import * as Kit from '@sveltejs/kit';

// Test: Return types inferred correctly and transformed into a union
type LoadReturn1 = { success: true } | { message: Promise<string> };
type LoadReturn1 =
| { success: true; message?: undefined }
| { success?: undefined; message: string };

let result1: Kit.AwaitedProperties<LoadReturn1> = null as any;
let result1: Kit.LoadProperties<LoadReturn1> = null as any;
result1.message = '';
result1.success = true;
// @ts-expect-error - cannot both be present at the same time
result1 = { message: '', success: true };

// Test: Return types keep promise for Deferred
type LoadReturn2 = { success: true } | Deferred<{ message: Promise<string>; eager: true }>;

let result2: Kit.AwaitedProperties<LoadReturn2> = null as any;
result2.message = Promise.resolve('');
result2.eager = true;
result2.success = true;
// @ts-expect-error - cannot both be present at the same time
result2 = { message: '', success: true };

0 comments on commit bf0282b

Please sign in to comment.