Skip to content

Commit

Permalink
breaking: update onMount type definition to prevent async function re…
Browse files Browse the repository at this point in the history
…turn (#8136)


---------

Co-authored-by: Yuichiro Yamashita <[email protected]>
Co-authored-by: Simon H <[email protected]>
  • Loading branch information
3 people committed Apr 18, 2023
1 parent 2f42347 commit 236ffa8
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 59 deletions.
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> ? "Returning a function asynchronously from onMount won't call that function on destroy" : T): void {
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;
});

0 comments on commit 236ffa8

Please sign in to comment.