breaking: conditional ActionReturn type if Parameter is void (#7442)

---------

Co-authored-by: Ivan Hofer <[email protected]>
Co-authored-by: Simon H <[email protected]>
Co-authored-by: Ignatius Bagus <[email protected]>
Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Simon Holthausen <[email protected]>

Author: 5 people

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)
 

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<never>` 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,8 +26,8 @@
  *
  * Docs: https://svelte.dev/docs#template-syntax-element-directives-use-action
  */
-export interface ActionReturn<Parameter = any, Attributes extends Record<string, any> = Record<never, any>> {
-	update?: (parameter: Parameter) => void;
+export interface ActionReturn<Parameter = never, Attributes extends Record<string, any> = Record<never, any>> {
+	update?: [Parameter] extends [never] ? never : (parameter: Parameter) => void;
 	destroy?: () => void;
 	/**
 	 * ### DO NOT USE THIS
@@ -42,15 +43,21 @@ export interface ActionReturn<Parameter = any, Attributes extends Record<string,
  * The following example defines an action that only works on `<div>` elements
  * and optionally accepts a parameter which it has a default value for:
  * ```ts
- * export const myAction: Action<HTMLDivElement, { someProperty: boolean }> = (node, param = { someProperty: true }) => {
+ * export const myAction: Action<HTMLDivElement, { someProperty: boolean } | undefined> = (node, param = { someProperty: true }) => {
  *   // ...
  * }
  * ```
+ * `Action<HTMLDivElement>` and `Action<HTMLDiveElement, never>` 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<Element = HTMLElement, Parameter = any, Attributes extends Record<string, any> = Record<never, any>> {
-	<Node extends Element>(node: Node, parameter?: Parameter): void | ActionReturn<Parameter, Attributes>;
+export interface Action<Element = HTMLElement, Parameter = never, Attributes extends Record<string, any> = Record<never, any>> {
+	<Node extends Element>(...args: [Parameter] extends [never] ? [node: Node] : undefined extends Parameter ? [node: Node, parameter?: Parameter] : [node: Node, parameter: Parameter]): void | ActionReturn<Parameter, Attributes>;
 }
+
+// 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

src/runtime/internal/lifecycle.ts

@@ -57,6 +57,9 @@ export function onDestroy(fn: () => any) {
 }
 
 export interface EventDispatcher<EventMap extends Record<string, any>> {
+	// 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
 	<Type extends keyof EventMap>(
 		...args: [EventMap[Type]] extends [never] ? [type: Type, parameter?: null | undefined, options?: DispatchOptions] :
 		null extends EventMap[Type] ? [type: Type, parameter?: EventMap[Type], options?: DispatchOptions] :

test/types/actions.ts

@@ -0,0 +1,153 @@
+import type { Action, ActionReturn } from '$runtime/action';
+
+// ---------------- Action
+
+const href: Action<HTMLAnchorElement> = (node) => {
+    node.href = '';
+    // @ts-expect-error
+    node.href = 1;
+};
+href;
+
+const required: Action<HTMLElement, boolean> = (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<HTMLElement, boolean> = (node, param) => {
+    node;
+    param;
+    return {
+        update: (p) => p === true,
+        destroy: () => {}
+    };
+};
+required1;
+
+const required2: Action<HTMLElement, boolean> = (node) => {
+    node;
+};
+required2;
+
+const required3: Action<HTMLElement, boolean> = (node, param) => {
+    node;
+    param;
+    return {
+        // @ts-expect-error comparison always resolves to false
+        update: (p) => p === 'd',
+        destroy: () => {}
+    };
+};
+required3;
+
+const optional: Action<HTMLElement, boolean | undefined> = (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<HTMLElement, boolean | undefined> = (node, param?) => {
+    node;
+    param;
+    return {
+        update: (p) => p === true,
+        destroy: () => {}
+    };
+};
+optional1;
+
+const optional2: Action<HTMLElement, boolean | undefined> = (node) => {
+    node;
+};
+optional2;
+
+const optional3: Action<HTMLElement, boolean | undefined> = (node, param) => {
+    node;
+    param;
+};
+optional3;
+
+const optional4: Action<HTMLElement, boolean | undefined> = (node, param?) => {
+    node;
+    param;
+    return {
+        // @ts-expect-error comparison always resolves to false
+        update: (p) => p === 'd',
+        destroy: () => {}
+    };
+};
+optional4;
+
+const no: Action<HTMLElement, never> = (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<HTMLElement, never> = (node) => {
+    node;
+    return {
+        destroy: () => {}
+    };
+};
+no1;
+
+// @ts-expect-error param given
+const no2: Action<HTMLElement, never> = (node, param?) => {};
+no2;
+
+// @ts-expect-error param given
+const no3: Action<HTMLElement, never> = (node, param) => {};
+no3;
+
+// @ts-expect-error update method given
+const no4: Action<HTMLElement, never> = (node) => {
+    return {
+        update: () => {},
+        destroy: () => {}
+    };
+};
+no4;
+
+// ---------------- ActionReturn
+
+const requiredReturn: ActionReturn<string> = {
+    update: (p) => p.toString()
+};
+requiredReturn;
+
+const optionalReturn: ActionReturn<boolean | undefined> = {
+    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<never, { a: string; }>['$$_attributes'];
+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;