Skip to content
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

fix: update onMount type definition to prevent async function return #8136

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* **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:/sveltejs/svelte/pull/7224))
* **breaking** Stricter types for `Action` and `ActionReturn` (see PR for migration instructions) ([#7224](https:/sveltejs/svelte/pull/7224))
* **breaking** Stricter types for `onMount` - now throws a type error when returning a function asynchronously to catch potential mistakes around callback functions (see PR for migration instructions) ([#8136](https:/sveltejs/svelte/pull/8136))
* Add `a11y no-noninteractive-element-interactions` rule ([#8391](https:/sveltejs/svelte/pull/8391))
* Add `a11y-no-static-element-interactions`rule ([#8251](https:/sveltejs/svelte/pull/8251))
* Bind `null` option and input values consistently ([#8312](https:/sveltejs/svelte/issues/8312))
Expand Down
4 changes: 3 additions & 1 deletion src/runtime/internal/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ export function beforeUpdate(fn: () => any) {
* It must be called during the component's initialisation (but doesn't need to live *inside* the component;
* it can be called from an external module).
*
* If a function is returned _synchronously_ from `onMount`, it will be called when the component is unmounted.
*
* `onMount` does not run inside a [server-side component](/docs#run-time-server-side-component-api).
*
* https://svelte.dev/docs#run-time-svelte-onmount
*/
export function onMount(fn: () => any) {
export function onMount<T>(fn: () => T extends Promise<() => any> ? never : T): void {
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
get_current_component().$$.on_mount.push(fn);
}

Expand Down
108 changes: 54 additions & 54 deletions test/types/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import type { Action, ActionReturn } from '$runtime/action';
// ---------------- Action

const href: Action<HTMLAnchorElement> = (node) => {
node.href = '';
// @ts-expect-error
node.href = 1;
node.href = '';
// @ts-expect-error
node.href = 1;
};
href;

const required: Action<HTMLElement, boolean> = (node, param) => {
node;
param;
node;
param;
};
required(null as any, true);
// @ts-expect-error (only in strict mode) boolean missing
Expand All @@ -20,74 +20,74 @@ required(null as any);
required(null as any, 'string');

const required1: Action<HTMLElement, boolean> = (node, param) => {
node;
param;
return {
update: (p) => p === true,
destroy: () => {}
};
node;
param;
return {
update: (p) => p === true,
destroy: () => {}
};
};
required1;

const required2: Action<HTMLElement, boolean> = (node) => {
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: () => {}
};
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;
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: () => {}
};
node;
param;
return {
update: (p) => p === true,
destroy: () => {}
};
};
optional1;

const optional2: Action<HTMLElement, boolean | undefined> = (node) => {
node;
node;
};
optional2;

const optional3: Action<HTMLElement, boolean | undefined> = (node, param) => {
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: () => {}
};
node;
param;
return {
// @ts-expect-error comparison always resolves to false
update: (p) => p === 'd',
destroy: () => {}
};
};
optional4;

const no: Action<HTMLElement, never> = (node) => {
node;
node;
};
// @ts-expect-error second param
no(null as any, true);
Expand All @@ -96,10 +96,10 @@ no(null as any);
no(null as any, 'string');

const no1: Action<HTMLElement, never> = (node) => {
node;
return {
destroy: () => {}
};
node;
return {
destroy: () => {}
};
};
no1;

Expand All @@ -113,32 +113,32 @@ no3;

// @ts-expect-error update method given
const no4: Action<HTMLElement, never> = (node) => {
return {
update: () => {},
destroy: () => {}
};
return {
update: () => {},
destroy: () => {}
};
};
no4;

// ---------------- ActionReturn

const requiredReturn: ActionReturn<string> = {
update: (p) => p.toString()
update: (p) => p.toString()
};
requiredReturn;

const optionalReturn: ActionReturn<boolean | undefined> = {
update: (p) => {
p === true;
// @ts-expect-error could be undefined
p.toString();
}
update: (p) => {
p === true;
// @ts-expect-error could be undefined
p.toString();
}
};
optionalReturn;

const invalidProperty: ActionReturn = {
// @ts-expect-error invalid property
invalid: () => {}
// @ts-expect-error invalid property
invalid: () => {}
};
invalidProperty;

Expand Down
8 changes: 4 additions & 4 deletions test/types/create-event-dispatcher.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { createEventDispatcher } from '$runtime/internal/lifecycle';

const dispatch = createEventDispatcher<{
loaded: never
change: string
valid: boolean
optional: number | null
loaded: never
change: string
valid: boolean
optional: number | null
}>();

// @ts-expect-error: dispatch invalid event
Expand Down
58 changes: 58 additions & 0 deletions test/types/on-mount.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { onMount } from '$runtime/index';

// sync and no return
onMount(() => {
console.log('mounted');
});

// sync and return value
onMount(() => {
return 'done';
});

// sync and return sync
onMount(() => {
return () => {
return 'done';
};
});

// sync and return async
onMount(() => {
return async () => {
const res = await fetch('');
return res;
};
});

// async and no return
onMount(async () => {
await fetch('');
});

// async and return value
onMount(async () => {
const res = await fetch('');
return res;
});

// @ts-expect-error async and return sync
onMount(async () => {
return () => {
return 'done';
};
});

// @ts-expect-error async and return async
onMount(async () => {
return async () => {
const res = await fetch('');
return res;
};
});

// @ts-expect-error async and return any
onMount(async () => {
const a: any = null as any;
return a;
});