Merge branch 'version-4' into feat/conditional-slots

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

rollup.config.mjs

@@ -14,136 +14,113 @@ const is_publish = !!process.env.PUBLISH;
 
 const ts_plugin = is_publish
 	? typescript({
-		typescript: require('typescript')
+			typescript: require('typescript'),
 	  })
 	: sucrase({
-		transforms: ['typescript']
+			transforms: ['typescript'],
 	  });
 
-// The following external and path logic is necessary so that the bundled runtime pieces and the index file
+fs.writeFileSync(
-// reference each other correctly instead of bundling their references to each other
+	`./compiler.d.ts`,
+	`export { compile, parse, preprocess, walk, VERSION } from './types/compiler/index.js';`
+);
 
-/**
+const runtime_entrypoints = Object.fromEntries(
- * Ensures that relative imports inside `src/runtime` like `./internal` and `../store` are externalized correctly
+	fs
- */
+		.readdirSync('src/runtime', { withFileTypes: true })
-const external = (id, parent_id) => {
+		.filter((dirent) => dirent.isDirectory())
-	const parent_segments = parent_id.replace(/\\/g, '/').split('/');
+		.map((dirent) => [dirent.name, `src/runtime/${dirent.name}/index.ts`])
-	// TODO needs to be adjusted when we move to JS modules
+);
-	if (parent_segments[parent_segments.length - 3] === 'runtime') {
-		return /\.\.\/\w+$/.test(id);
-	} else {
-		return id === './internal' && parent_segments[parent_segments.length - 2] === 'runtime';
-	}
-}
-
-/**
- * Transforms externalized import paths like `../store` into correct relative imports with correct index file extension import
- */
-const replace_relative_svelte_imports = (id, ending) => {
-	id = id.replace(/\\/g, '/');
-	// TODO needs to be adjusted when we move to JS modules
-	return /src\/runtime\/\w+$/.test(id) && `../${id.split('/').pop()}/${ending}`;
-}
 
 /**
- * Transforms externalized `./internal` import path into correct relative import with correct index file extension import
+ * @type {import("rollup").RollupOptions[]}
  */
-const replace_relative_internal_import = (id, ending) => {
-	id = id.replace(/\\/g, '/');
-	// TODO needs to be adjusted when we move to JS modules
-	return id.endsWith('src/runtime/internal') && `./internal/${ending}`;
-}
-
-fs.writeFileSync(`./compiler.d.ts`, `export { compile, parse, preprocess, walk, VERSION } from './types/compiler/index';`);
-
 export default [
-	/* runtime */
 	{
-		input: `src/runtime/index.ts`,
+		input: {
-		output: [
+			...runtime_entrypoints,
-			{
+			index: 'src/runtime/index.ts',
-				file: `index.mjs`,
+			ssr: 'src/runtime/ssr.ts'
-				format: 'esm',
-				paths: id => replace_relative_internal_import(id, 'index.mjs')
-			},
-			{
-				file: `index.js`,
-				format: 'cjs',
-				paths: id => replace_relative_internal_import(id, 'index.js')
-			}
-		],
-		external,
-		plugins: [ts_plugin]
 		},
+		output: ['es', 'cjs'].map(
+			/** @returns {import('rollup').OutputOptions} */
+			(format) => {
+				const ext = format === 'es' ? 'mjs' : 'js';
+				return {
+					entryFileNames: (entry) => {
+						if (entry.isEntry) {
+							if (entry.name === 'index') return `index.${ext}`;
+							else if (entry.name === 'ssr') return `ssr.${ext}`;
 
-	{
+							return `${entry.name}/index.${ext}`;
-		input: `src/runtime/ssr.ts`,
-		output: [
-			{
-				file: `ssr.mjs`,
-				format: 'esm',
-				paths: id => replace_relative_internal_import(id, 'index.mjs')
-			},
-			{
-				file: `ssr.js`,
-				format: 'cjs',
-				paths: id => replace_relative_internal_import(id, 'index.js')
 						}
-		],
-		external,
-		plugins: [ts_plugin]
-	},
-
-	...fs.readdirSync('src/runtime')
-		.filter(dir => fs.statSync(`src/runtime/${dir}`).isDirectory())
-		.map(dir => ({
-			input: `src/runtime/${dir}/index.ts`,
-			output: [
-				{
-					file: `${dir}/index.mjs`,
-					format: 'esm',
-					paths: id => replace_relative_svelte_imports(id, 'index.mjs')
 					},
-				{
+					chunkFileNames: `internal/[name]-[hash].${ext}`,
-					file: `${dir}/index.js`,
+					format,
-					format: 'cjs',
+					minifyInternalExports: false,
-					paths: id => replace_relative_svelte_imports(id, 'index.js')
+					dir: '.',
+				};
 			}
-			],
+		),
-			external,
 		plugins: [
 			replace({
-					__VERSION__: pkg.version
+				preventAssignment: true,
+				values: {
+					__VERSION__: pkg.version,
+				},
 			}),
 			ts_plugin,
 			{
-					writeBundle(_options, bundle) {
+				writeBundle(options, bundle) {
+					if (options.format !== 'es') return;
+
+					for (const entry of Object.values(bundle)) {
+						const dir = entry.name;
+						if (!entry.isEntry || !runtime_entrypoints[dir]) continue;
+
 						if (dir === 'internal') {
-							const mod = bundle['index.mjs'];
+							const mod = bundle[`internal/index.mjs`];
 							if (mod) {
-								fs.writeFileSync('src/compiler/compile/internal_exports.ts', `// This file is automatically generated\nexport default new Set(${JSON.stringify(mod.exports)});`);
+								fs.writeFileSync(
+									'src/compiler/compile/internal_exports.ts',
+									`// This file is automatically generated\n` +
+										`export default new Set(${JSON.stringify(mod.exports)});`
+								);
 							}
 						}
 
-						fs.writeFileSync(`${dir}/package.json`, JSON.stringify({
+						fs.writeFileSync(
-							main: './index',
+							`${dir}/package.json`,
+							JSON.stringify(
+								{
+									main: './index.js',
 									module: './index.mjs',
-							types: './index.d.ts'
+									types: './index.d.ts',
-						}, null, '  '));
+								},
+								null,
+								'  '
+							)
+						);
 
-						fs.writeFileSync(`${dir}/index.d.ts`, `export * from '../types/runtime/${dir}/index';`);
+						fs.writeFileSync(
+							`${dir}/index.d.ts`,
+							`export * from '../types/runtime/${dir}/index.js';`
+						);
+					}
 				}
 			}
 		]
-		})),
+	},
-
 	/* compiler.js */
 	{
 		input: 'src/compiler/index.ts',
 		plugins: [
 			replace({
+				preventAssignment: true,
+				values: {
 					__VERSION__: pkg.version,
 					'process.env.NODE_DEBUG': false // appears inside the util package
+				},
 			}),
 			{
 				resolveId(id) {
@@ -152,7 +129,7 @@ export default [
 					if (id === 'util') {
 						return require.resolve('./node_modules/util'); // just 'utils' would resolve this to the built-in module
 					}
-				}
+				},
 			},
 			resolve(),
 			commonjs({
@@ -177,6 +154,7 @@ export default [
 		],
 		external: is_publish
 			? []
-			: id => id === 'acorn' || id === 'magic-string' || id.startsWith('css-tree')
+			: (id) =>
+					id === 'acorn' || id === 'magic-string' || id.startsWith('css-tree')
 	}
 ];

src/compiler/index.ts

@@ -4,4 +4,4 @@ export { default as preprocess } from './preprocess/index';
 export { walk } from 'estree-walker';
 export type { CompileOptions, ModuleFormat, EnableSourcemap, CssHashGetter } from './interfaces';
 
-export const VERSION = '__VERSION__';
+export const VERSION: string = '__VERSION__';

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

src/runtime/store/index.ts

@@ -12,8 +12,15 @@ export type Updater<T> = (value: T) => T;
 /** Cleanup logic callback. */
 type Invalidator<T> = (value?: T) => void;
 
-/** Start and stop notification callbacks. */
+/** 
-export type StartStopNotifier<T> = (set: Subscriber<T>) => Unsubscriber | void;
+ * Start and stop notification callbacks.
+ * This function is called when the first subscriber subscribes.
+ * 
+ * @param {(value: T) => void} set Function that sets the value of the store.
+ * @returns {void | (() => void)} Optionally, a cleanup function that is called when the last remaining
+ * subscriber unsubscribes.
+ */
+export type StartStopNotifier<T> = (set: (value: T) => void) => void | (() => void);
 
 /** Readable interface for subscribing. */
 export interface Readable<T> {
@@ -48,7 +55,7 @@ const subscriber_queue = [];
 /**
  * Creates a `Readable` store that allows reading by subscription.
  * @param value initial value
- * @param {StartStopNotifier}start start and stop notifications for subscriptions
+ * @param {StartStopNotifier} [start] 
  */
 export function readable<T>(value?: T, start?: StartStopNotifier<T>): Readable<T> {
 	return {
@@ -59,7 +66,7 @@ export function readable<T>(value?: T, start?: StartStopNotifier<T>): Readable<T
 /**
  * Create a `Writable` store that allows both updating and reading by subscription.
  * @param {*=}value initial value
- * @param {StartStopNotifier=}start start and stop notifications for subscriptions
+ * @param {StartStopNotifier=} start
  */
 export function writable<T>(value?: T, start: StartStopNotifier<T> = noop): Writable<T> {
 	let stop: Unsubscriber;

test/runtime-puppeteer/index.ts

@@ -75,9 +75,6 @@ describe('runtime (puppeteer)', () => {
 
 	function runTest(dir, hydrate, is_first_run) {
 		if (dir[0] === '.') return;
-		// MEMO: puppeteer can not execute Chromium properly with Node8,10 on Linux at GitHub actions.
-		const { version } = process;
-		if ((version.startsWith('v8.') || version.startsWith('v10.')) && process.platform === 'linux') return;
 
 		const config = loadConfig(`${__dirname}/samples/${dir}/_config.js`);
 		const solo = config.solo || /\.solo/.test(dir);

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