breaking: update onMount type definition to prevent async function return (#8136)

---------

Co-authored-by: Yuichiro Yamashita <xydybaseball@gmail.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

CHANGELOG.md

@@ -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://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))
+* **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://github.com/sveltejs/svelte/pull/8136))
 * Add `a11y no-noninteractive-element-interactions` rule ([#8391](https://github.com/sveltejs/svelte/pull/8391))
 * Add `a11y-no-static-element-interactions`rule ([#8251](https://github.com/sveltejs/svelte/pull/8251))
 * Bind `null` option and input values consistently ([#8312](https://github.com/sveltejs/svelte/issues/8312))

src/runtime/internal/lifecycle.ts

@@ -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);
 }
 

test/types/actions.ts

@@ -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
@@ -20,34 +20,34 @@ 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);
@@ -55,39 +55,39 @@ optional(null as any);
 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);
@@ -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;
 
@@ -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;
 

test/types/create-event-dispatcher.ts

@@ -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

test/types/on-mount.ts

@@ -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;
+});