breaking: conditional ActionReturn type if Parameter is void

[tanhauhau]

This PR strictens the types for Action and ActionReturn.

Breaking changes / how to migrate

The Parameter generic of Action and ActionReturn is now never by default (it was any by default previously). This means that you'll get type errors if you're relying on the previous default type and were using a parameter:

const action: Action = (node, params) => { .. } // this is now an error, as params is expected to not exist

You'll also get an error when returning an update method from the action in that case because it's expected to not be set, since there's no parameter.

To migrate, type the Parameter generic explicitly:

const action: Action<HTMLElement, string> = (node, params) => { .. } // params is of type string

The same is true for ActionReturn:

- function action(node, params): ActionReturn {
+ function action(node, params): ActionReturn<string> {
  return {
    update: (newParams) => .. // this is an error if ActionReturn's Parameter generic is not set
  }
}

PR description

Based on #7347

Before submitting the PR, please make sure you do the following

• Prefix your PR title with [feat], [fix], [chore], or [docs].
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with npm test and lint the project with npm run lint

[dummdidumm]

Not marking parameters as optional has the same problems as discussed in #7121, where we decided to type it as optional. The great thing is that we can use the same conditional check to type that, too. I'll update the PR accordingly. cc @ignatiusmb

[ignatiusmb]

This gives a new perspective on how we can approach things, but I feel we're overcomplicating ourselves here

[ivanhofer]

The default value is not needed because everywhere a vaule gets passed

Suggested change

	interface ActionReturn<Parameter = void> {
	interface ActionReturn<Parameter> {

[ivanhofer]

You don't export this interface anymore. Not sure if this line could be confusing to people reading it and then trying to import that type from svelte/action

[ivanhofer]

is <Node extends Element> really needed?
I would just type node as Element

[ivanhofer]

I would use never instead of undefined

[dummdidumm]

Suggested change

	interface ActionReturn<Parameter = void> {
	export interface ActionReturn<Parameter = any>

not sure why this change slipped in (might have been an oversight by me), I think it's better to revert it to the previous version.

[dummdidumm]

Revisiting this I finally found a way to type this so that it

• works with and without strict mode
• avoids (ha) the void problem
• passes all other relevant type tests (see code snippet)

/**
 * 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.
 * - 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.
 * This applies to TypeScript typings only and has no effect at runtime.
 *
 * Example usage:
 * ```ts
 * interface Attributes {
 * 	newprop?: string;
 *	'on:event': (e: CustomEvent<boolean>) => void;
 * }
 *
 * export function myAction(node: HTMLElement, parameter: Parameter): ActionReturn<Parameter, Attributes> {
 *   // ...
 *   return {
 *     update: (updatedParameter) => {...},
 *     destroy: () => {...}
 *   };
 * }
 * ```
 *
 * 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] extends [never] ? never : (parameter: Parameter) => void;
	destroy?: () => void;
	/**
	 * ### DO NOT USE THIS
	 * This exists solely for type-checking and has no effect at runtime.
	 * Set this through the `Attributes` generic instead.
	 */
	$$_attributes?: Attributes;
}

/**
 * Actions are functions that are called when an element is created.
 * You can use this interface to type such actions.
 * 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 }) => {
 *   // ...
 * }
 * ```
 * 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 = 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>;
}

type A = (undefined  | boolean) extends undefined ? boolean : string;
type B = undefined extends (undefined | boolean) ? boolean : string;
type C = undefined extends (boolean) ? boolean : string;

const required: Action<HTMLElement, boolean> = (node, param) => {}
const required1: Action<HTMLElement, boolean> = (node, param) => {
    return {
        update: (p) => p === true,
        destroy: () => {}
    }
}
const required2: Action<HTMLElement, boolean> = (node) => {}
const required3: Action<HTMLElement, boolean> = (node, param) => {
    return {
        // @ts-expect-error comparison always resolves to false
        update: (p) => p === 'd',
        destroy: () => {}
    }
}
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 optional: Action<HTMLElement, boolean | undefined> = (node, param?) => {}
const optional1: Action<HTMLElement, boolean | undefined> = (node, param?) => {
    return {
        update: (p) => p === true,
        destroy: () => {}
    }
}
const optional2: Action<HTMLElement, boolean | undefined> = (node) => {}
const optional3: Action<HTMLElement, boolean | undefined> = (node, param) => {}
const optional4: Action<HTMLElement, boolean | undefined> = (node, param?) => {
    return {
        // @ts-expect-error comparison always resolves to false
        update: (p) => p === 'd',
        destroy: () => {}
    }
}
optional(null as any, true);
optional(null as any);
// @ts-expect-error no boolean
optional(null as any, 'string');

const no: Action<HTMLElement, never> = (node) => {}
const no1: Action<HTMLElement, never> = (node) => {
    return {
        destroy: () => {}
    }
}
// @ts-expect-error param given
const no2: Action<HTMLElement, never> = (node, param?) => {}
// @ts-expect-error param given
const no3: Action<HTMLElement, never> = (node, param) => {}
// @ts-expect-error update method given
const no4: Action<HTMLElement, never> = (node) => {
    return {
        update: () => {},
        destroy: () => {}
    }
}
// @ts-expect-error second param
no(null as any, true);
no(null as any);
// @ts-expect-error second param
no(null as any, 'string');

Given that this might result in some TS errors appearing for people who previously typed this differently, this should go into 4.x. We then should also find a proper place for the type tests I have in the code snippet.

[vercel]

@dummdidumm is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

[tanhauhau]

@dummdidumm not quite sure if which typescript version you run for your change, it seemed to have some syntax issues with linters and tsc, i've just fixed them

[dummdidumm]

Can you revert those? For Svelte version 4 we'll also bump the TS version so this will all be valid then.

[tanhauhau]

Can you revert those? For Svelte version 4 we'll also bump the TS version so this will all be valid then.

ok i've reverted it.

[ignatiusmb]

Brilliant, and clean!