From 3cb9dcf186cadbff248d8fc0ecb10f324b92ef01 Mon Sep 17 00:00:00 2001 From: tanhauhau Date: Mon, 11 Apr 2022 18:56:49 +0800 Subject: [PATCH 1/8] ActionReturn should return destroy only if Parameter is void Co-authored-by: Ivan Hofer --- src/runtime/action/index.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/runtime/action/index.ts b/src/runtime/action/index.ts index d7cbf04b1294..3469ecbb7178 100644 --- a/src/runtime/action/index.ts +++ b/src/runtime/action/index.ts @@ -17,10 +17,14 @@ * * Docs: https://svelte.dev/docs#template-syntax-element-directives-use-action */ -export interface ActionReturn { - update?: (parameter: Parameter) => void; - destroy?: () => void; -} +export type ActionReturn = void extends Parameter + ? { + destroy?: () => void; + } + : { + update?: (parameter: Parameter) => void; + destroy?: () => void; + } /** * Actions are functions that are called when an element is created. @@ -37,6 +41,7 @@ export interface ActionReturn { * * Docs: https://svelte.dev/docs#template-syntax-element-directives-use-action */ -export interface Action { - (node: Node, parameter?: Parameter): void | ActionReturn; -} +export type Action = + void extends Parameter + ? (node: Node) => void | ActionReturn + : (node: Node, parameter: Parameter) => void | ActionReturn; From 1a2119715d1a5022f8efef7830e734600a852d8b Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 11 Apr 2022 14:21:04 +0200 Subject: [PATCH 2/8] add case for optional parameter --- src/runtime/action/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/runtime/action/index.ts b/src/runtime/action/index.ts index 3469ecbb7178..b1c344b6e542 100644 --- a/src/runtime/action/index.ts +++ b/src/runtime/action/index.ts @@ -32,7 +32,7 @@ export type ActionReturn = void extends Parameter * The following example defines an action that only works on `
` elements * and optionally accepts a parameter which it has a default value for: * ```ts - * export const myAction: Action = (node, param = { someProperty: true }) => { + * export const myAction: Action = (node, param = { someProperty: true }) => { * // ... * } * ``` @@ -44,4 +44,6 @@ export type ActionReturn = void extends Parameter export type Action = void extends Parameter ? (node: Node) => void | ActionReturn + : undefined extends Parameter + ? (node: Node, parameter?: Parameter) => void | ActionReturn : (node: Node, parameter: Parameter) => void | ActionReturn; From 34f7b95f5ddba6be594bc9299888af0dd4f234be Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 11 Apr 2022 14:24:46 +0200 Subject: [PATCH 3/8] docs --- src/runtime/action/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/runtime/action/index.ts b/src/runtime/action/index.ts index b1c344b6e542..cc0047603a5d 100644 --- a/src/runtime/action/index.ts +++ b/src/runtime/action/index.ts @@ -36,7 +36,9 @@ export type ActionReturn = void extends Parameter * // ... * } * ``` - * You can return an object with methods `update` and `destroy` from the function. + * If your action doesn't accept a parameter, type if as `Action`. + * + * You can return an object with methods `update` (if the action accepts a parameter) and `destroy` from the function. * See interface `ActionReturn` for more details. * * Docs: https://svelte.dev/docs#template-syntax-element-directives-use-action From 99764b2cf0f50cd38d0ee677229f2f1239db97dc Mon Sep 17 00:00:00 2001 From: Tan Li Hau Date: Wed, 13 Apr 2022 18:07:22 +0800 Subject: [PATCH 4/8] Update src/runtime/action/index.ts Co-authored-by: Ignatius Bagus --- src/runtime/action/index.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/runtime/action/index.ts b/src/runtime/action/index.ts index cc0047603a5d..679ebc7e414e 100644 --- a/src/runtime/action/index.ts +++ b/src/runtime/action/index.ts @@ -17,14 +17,10 @@ * * Docs: https://svelte.dev/docs#template-syntax-element-directives-use-action */ -export type ActionReturn = void extends Parameter - ? { - destroy?: () => void; - } - : { - update?: (parameter: Parameter) => void; - destroy?: () => void; - } +interface ActionReturn { + update?: void extends Parameter ? undefined : (parameter: Parameter) => void; + destroy?: () => void; +} /** * Actions are functions that are called when an element is created. From 5320874ea32e3182f529978c51dc4f87b69b3725 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 28 Feb 2023 15:11:51 +0100 Subject: [PATCH 5/8] docs --- src/runtime/action/index.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/runtime/action/index.ts b/src/runtime/action/index.ts index 1521941bf5ff..0c440e30a1cb 100644 --- a/src/runtime/action/index.ts +++ b/src/runtime/action/index.ts @@ -1,7 +1,8 @@ /** * Actions can return an object containing the two properties defined in this interface. Both are optional. * - update: An action can have a parameter. This method will be called whenever that parameter changes, - * immediately after Svelte has applied updates to the markup. + * immediately after Svelte has applied updates to the markup. `ActionReturn` and `ActionReturn` both + * mean that the action accepts no parameters, which makes it illegal to set the `update` method. * - destroy: Method that is called after the element is unmounted * * Additionally, you can specify which additional attributes and events the action enables on the applied element. @@ -25,7 +26,7 @@ * * Docs: https://svelte.dev/docs#template-syntax-element-directives-use-action */ -export interface ActionReturn = Record> { +export interface ActionReturn = Record> { update?: [Parameter] extends [never] ? never : (parameter: Parameter) => void; destroy?: () => void; /** @@ -42,15 +43,21 @@ export interface ActionReturn` elements * and optionally accepts a parameter which it has a default value for: * ```ts - * export const myAction: Action = (node, param = { someProperty: true }) => { + * export const myAction: Action = (node, param = { someProperty: true }) => { * // ... * } * ``` + * `Action` and `Action` both signal that the action accepts no parameters. + * * You can return an object with methods `update` and `destroy` from the function and type which additional attributes and events it has. * See interface `ActionReturn` for more details. * * Docs: https://svelte.dev/docs#template-syntax-element-directives-use-action */ export interface Action = Record> { - (...args: [Parameter] extends [never ]? [node: Node] : undefined extends Parameter? [node: Node, parameter?: Parameter] : [node: Node, parameter: Parameter]): void | ActionReturn; + (...args: [Parameter] extends [never] ? [node: Node] : undefined extends Parameter? [node: Node, parameter?: Parameter] : [node: Node, parameter: Parameter]): void | ActionReturn; } + +// Implementation notes: +// - undefined extends X instead of X extends undefined makes this work better with both strict and nonstrict mode +// - [X] extends [never] is needed, X extends never would reduce the whole resulting type to never and not to one of the condition outcomes From 0058204bfed49880cbfa5911285d02422db23249 Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Wed, 1 Mar 2023 00:52:48 +0700 Subject: [PATCH 6/8] add space --- src/runtime/action/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/action/index.ts b/src/runtime/action/index.ts index 0c440e30a1cb..a672bb7ae5f3 100644 --- a/src/runtime/action/index.ts +++ b/src/runtime/action/index.ts @@ -55,7 +55,7 @@ export interface ActionReturn = Record> { - (...args: [Parameter] extends [never] ? [node: Node] : undefined extends Parameter? [node: Node, parameter?: Parameter] : [node: Node, parameter: Parameter]): void | ActionReturn; + (...args: [Parameter] extends [never] ? [node: Node] : undefined extends Parameter ? [node: Node, parameter?: Parameter] : [node: Node, parameter: Parameter]): void | ActionReturn; } // Implementation notes: From 608cc0e3f3af55bee4153327afa05c29fee1d517 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 14 Apr 2023 12:53:46 +0200 Subject: [PATCH 7/8] tests --- CHANGELOG.md | 1 + src/runtime/internal/lifecycle.ts | 3 + test/types/actions.ts | 148 ++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+) create mode 100644 test/types/actions.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a6345d99b317..e28643f1f02c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * **breaking** Minimum supported Node version is now Node 14 * **breaking** Minimum supported TypeScript version is now 5 (it will likely work with lower versions, but we make no guarantess about that) * **breaking** Stricter types for `createEventDispatcher` (see PR for migration instructions) ([#7224](https://github.com/sveltejs/svelte/pull/7224)) +* **breaking** Stricter types for `Action` and `ActionReturn` (see PR for migration instructions) ([#7224](https://github.com/sveltejs/svelte/pull/7224)) ## Unreleased (3.0) diff --git a/src/runtime/internal/lifecycle.ts b/src/runtime/internal/lifecycle.ts index 7592e72a3860..29888e9de3da 100644 --- a/src/runtime/internal/lifecycle.ts +++ b/src/runtime/internal/lifecycle.ts @@ -57,6 +57,9 @@ export function onDestroy(fn: () => any) { } export interface EventDispatcher> { + // Implementation notes: + // - undefined extends X instead of X extends undefined makes this work better with both strict and nonstrict mode + // - [X] extends [never] is needed, X extends never would reduce the whole resulting type to never and not to one of the condition outcomes ( ...args: [EventMap[Type]] extends [never] ? [type: Type, parameter?: null | undefined, options?: DispatchOptions] : null extends EventMap[Type] ? [type: Type, parameter?: EventMap[Type], options?: DispatchOptions] : diff --git a/test/types/actions.ts b/test/types/actions.ts new file mode 100644 index 000000000000..d139143f12f4 --- /dev/null +++ b/test/types/actions.ts @@ -0,0 +1,148 @@ +import type { Action, ActionReturn } from '$runtime/action'; + +// ---------------- Action + +const href: Action = (node) => { + node.href = ''; + // @ts-expect-error + node.href = 1; +}; +href; + +const required: Action = (node, param) => { + node; + param; +}; +required(null as any, true); +// @ts-expect-error (only in strict mode) boolean missing +required(null as any); +// @ts-expect-error no boolean +required(null as any, 'string'); + +const required1: Action = (node, param) => { + node; + param; + return { + update: (p) => p === true, + destroy: () => {} + } +}; +required1; + +const required2: Action = (node) => { + node; +}; +required2; + +const required3: Action = (node, param) => { + node; + param; + return { + // @ts-expect-error comparison always resolves to false + update: (p) => p === 'd', + destroy: () => {} + } +}; +required3; + +const optional: Action = (node, param?) => { + node; + param; +}; +optional(null as any, true); +optional(null as any); +// @ts-expect-error no boolean +optional(null as any, 'string'); + +const optional1: Action = (node, param?) => { + node; + param; + return { + update: (p) => p === true, + destroy: () => {} + } +}; +optional1; + +const optional2: Action = (node) => { + node; +}; +optional2; + +const optional3: Action = (node, param) => { + node; + param; +}; +optional3; + +const optional4: Action = (node, param?) => { + node; + param; + return { + // @ts-expect-error comparison always resolves to false + update: (p) => p === 'd', + destroy: () => {} + } +}; +optional4; + +const no: Action = (node) => { + node; +}; +// @ts-expect-error second param +no(null as any, true); +no(null as any); +// @ts-expect-error second param +no(null as any, 'string'); + +const no1: Action = (node) => { + node; + return { + destroy: () => {} + } +}; +no1; + +// @ts-expect-error param given +const no2: Action = (node, param?) => {} + +// @ts-expect-error param given +const no3: Action = (node, param) => {} + +// @ts-expect-error update method given +const no4: Action = (node) => { + return { + update: () => {}, + destroy: () => {} + } +} + +// ---------------- ActionReturn + +const requiredReturn: ActionReturn = { + update: (p) => p.toString(), +}; +requiredReturn; + +const optionalReturn: ActionReturn = { + update: (p) => { + p === true; + // @ts-expect-error could be undefined + p.toString(); + }, +}; +optionalReturn; + +const invalidProperty: ActionReturn = { + // @ts-expect-error invalid property + invalid: () => {}, +}; +invalidProperty; + +type Attributes = ActionReturn['$$_attributes']; +const attributes: Attributes = { a: 'a' }; +attributes; +// @ts-expect-error wrong type +const invalidAttributes1: Attributes = { a: 1 }; +// @ts-expect-error missing prop +const invalidAttributes2: Attributes = {}; From 45a3f39d4eda51c0d3edfe4c8a19d554199d288d Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 14 Apr 2023 13:20:13 +0200 Subject: [PATCH 8/8] lint --- test/types/actions.ts | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/test/types/actions.ts b/test/types/actions.ts index d139143f12f4..2a604151a85b 100644 --- a/test/types/actions.ts +++ b/test/types/actions.ts @@ -25,7 +25,7 @@ const required1: Action = (node, param) => { return { update: (p) => p === true, destroy: () => {} - } + }; }; required1; @@ -41,7 +41,7 @@ const required3: Action = (node, param) => { // @ts-expect-error comparison always resolves to false update: (p) => p === 'd', destroy: () => {} - } + }; }; required3; @@ -60,7 +60,7 @@ const optional1: Action = (node, param?) => { return { update: (p) => p === true, destroy: () => {} - } + }; }; optional1; @@ -82,7 +82,7 @@ const optional4: Action = (node, param?) => { // @ts-expect-error comparison always resolves to false update: (p) => p === 'd', destroy: () => {} - } + }; }; optional4; @@ -99,28 +99,31 @@ const no1: Action = (node) => { node; return { destroy: () => {} - } + }; }; no1; // @ts-expect-error param given -const no2: Action = (node, param?) => {} +const no2: Action = (node, param?) => {}; +no2; // @ts-expect-error param given -const no3: Action = (node, param) => {} +const no3: Action = (node, param) => {}; +no3; // @ts-expect-error update method given const no4: Action = (node) => { return { update: () => {}, destroy: () => {} - } -} + }; +}; +no4; // ---------------- ActionReturn const requiredReturn: ActionReturn = { - update: (p) => p.toString(), + update: (p) => p.toString() }; requiredReturn; @@ -129,13 +132,13 @@ const optionalReturn: ActionReturn = { p === true; // @ts-expect-error could be undefined p.toString(); - }, + } }; optionalReturn; const invalidProperty: ActionReturn = { // @ts-expect-error invalid property - invalid: () => {}, + invalid: () => {} }; invalidProperty; @@ -144,5 +147,7 @@ const attributes: Attributes = { a: 'a' }; attributes; // @ts-expect-error wrong type const invalidAttributes1: Attributes = { a: 1 }; +invalidAttributes1; // @ts-expect-error missing prop const invalidAttributes2: Attributes = {}; +invalidAttributes2;