Merge branch 'version-4' into custom-elements-rework

CHANGELOG.md

@@ -3,9 +3,11 @@
 ## Unreleased (4.0)
 
 * **breaking** Minimum supported Node version is now Node 14
-* **breaking** Minimum supported TypeScript version is now 5 (it will likely work with lower versions, but we make no guarantess about that)
+* **breaking** Minimum supported webpack version is now webpack 5
+* **breaking** Minimum supported TypeScript version is now TypeScript 5 (it will likely work with lower versions, but we make no guarantees 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))
 * **breaking** Overhaul and drastically improve creating custom elements with Svelte (see PR for list of changes and migration instructions) ([#8457](https://github.com/sveltejs/svelte/pull/8457))
 * 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))

rollup.config.mjs

@@ -14,136 +14,100 @@ 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
-// reference each other correctly instead of bundling their references to each other
+fs.writeFileSync(
+	`./compiler.d.ts`,
+	`export { compile, parse, preprocess, walk, VERSION } from './types/compiler/index.js';`
+);
 
-/**
- * Ensures that relative imports inside `src/runtime` like `./internal` and `../store` are externalized correctly
- */
-const external = (id, parent_id) => {
-	const parent_segments = parent_id.replace(/\\/g, '/').split('/');
-	// 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}`;
-}
+const runtime_entrypoints = Object.fromEntries(
+	fs
+		.readdirSync('src/runtime', { withFileTypes: true })
+		.filter((dirent) => dirent.isDirectory())
+		.map((dirent) => [dirent.name, `src/runtime/${dirent.name}/index.ts`])
+);
 
 /**
- * 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`,
-		output: [
-			{
-				file: `index.mjs`,
-				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]
-	},
+		input: {
+			...runtime_entrypoints,
+			index: 'src/runtime/index.ts',
+			ssr: 'src/runtime/ssr.ts'
+		},
+		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}`;
 
-	{
-		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')
+							return `${entry.name}/index.${ext}`;
+						}
+					},
+					chunkFileNames: `internal/[name]-[hash].${ext}`,
+					format,
+					minifyInternalExports: false,
+					dir: '.',
+				};
 			}
-		],
-		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')
+		),
+		plugins: [
+			replace({
+				preventAssignment: true,
+				values: {
+					__VERSION__: pkg.version,
 				},
-				{
-					file: `${dir}/index.js`,
-					format: 'cjs',
-					paths: id => replace_relative_svelte_imports(id, 'index.js')
-				}
-			],
-			external,
-			plugins: [
-				replace({
-					__VERSION__: pkg.version
-				}),
-				ts_plugin,
-				{
-					writeBundle(_options, bundle) {
+			}),
+			ts_plugin,
+			{
+				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({
-							main: './index',
-							module: './index.mjs',
-							types: './index.d.ts'
-						}, 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({
-				__VERSION__: pkg.version,
-				'process.env.NODE_DEBUG': false // appears inside the util package
+				preventAssignment: true,
+				values: {
+					__VERSION__: pkg.version,
+					'process.env.NODE_DEBUG': false // appears inside the util package
+				},
 			}),
 			{
 				resolveId(id) {
@@ -152,7 +116,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 +141,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/keyed_each.ts

@@ -108,12 +108,18 @@ export function update_keyed_each(old_blocks, dirty, get_key, dynamic, ctx, list
 }
 
 export function validate_each_keys(ctx, list, get_context, get_key) {
-	const keys = new Set();
+	const keys = new Map();
 	for (let i = 0; i < list.length; i++) {
 		const key = get_key(get_context(ctx, list, i));
 		if (keys.has(key)) {
-			throw new Error('Cannot have duplicate keys in a keyed each');
+			let value = '';
+			try {
+				value = `with value '${String(key)}' `;
+			} catch (e) {
+				// can't stringify
+			}
+			throw new Error(`Cannot have duplicate keys in a keyed each: Keys at index ${keys.get(key)} and ${i} ${value}are duplicates`);
 		}
-		keys.add(key);
+		keys.set(key, i);
 	}
 }

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/runtime/samples/keyed-each-dev-unique/_config.js

@@ -3,5 +3,5 @@ export default {
 		dev: true
 	},
 
-	error: 'Cannot have duplicate keys in a keyed each'
+	error: 'Cannot have duplicate keys in a keyed each: Keys at index 0 and 3 with value \'1\' are duplicates'
 };

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