From 39333b13345b688a1bfc554dba61e5de98ce1ca9 Mon Sep 17 00:00:00 2001 From: Yuichiro Yamashita Date: Sun, 16 Apr 2023 00:25:53 +0900 Subject: [PATCH 1/3] chore: remove Node 8 and 10 logic (#8503) --- test/runtime-puppeteer/index.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/runtime-puppeteer/index.ts b/test/runtime-puppeteer/index.ts index efb7ac02e3..e2bcf2add1 100644 --- a/test/runtime-puppeteer/index.ts +++ b/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); From 662804e80eae2e21c20ce364db05494362dd2d69 Mon Sep 17 00:00:00 2001 From: gtmnayan <50981692+gtm-nayan@users.noreply.github.com> Date: Tue, 18 Apr 2023 14:38:50 +0545 Subject: [PATCH 2/3] chore: produce single bundle for runtime with multiple entrypoints (#8504) * single runtime bundle * formatting * dedupe output options * fix tests apparently * skip writeBundle for cjs build * revert quotes * remove manualChunks * some node16 module resolution compliance * disable minifyInternalExports (doesn't really make sense for a library since users' build step will do it again anyway) --- rollup.config.mjs | 194 ++++++++++++++++++++-------------------------- 1 file changed, 86 insertions(+), 108 deletions(-) diff --git a/rollup.config.mjs b/rollup.config.mjs index 56ebdaa755..d6a22733a4 100644 --- a/rollup.config.mjs +++ b/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 -// 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}/package.json`, + JSON.stringify( + { + main: './index.js', + 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 +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') } ]; From 350c6c32151b2d15920c51c295deacd0ca4e56b2 Mon Sep 17 00:00:00 2001 From: Chris Kerr Date: Tue, 18 Apr 2023 18:59:33 +1000 Subject: [PATCH 3/3] breaking: update onMount type definition to prevent async function return (#8136) --------- Co-authored-by: Yuichiro Yamashita Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- CHANGELOG.md | 1 + src/runtime/internal/lifecycle.ts | 4 +- test/types/actions.ts | 108 +++++++++++++------------- test/types/create-event-dispatcher.ts | 8 +- test/types/on-mount.ts | 58 ++++++++++++++ 5 files changed, 120 insertions(+), 59 deletions(-) create mode 100644 test/types/on-mount.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index fb855b2996..381d02de72 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/src/runtime/internal/lifecycle.ts b/src/runtime/internal/lifecycle.ts index 29888e9de3..1c1726c29c 100644 --- a/src/runtime/internal/lifecycle.ts +++ b/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(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); } diff --git a/test/types/actions.ts b/test/types/actions.ts index 2a604151a8..7a0ca97714 100644 --- a/test/types/actions.ts +++ b/test/types/actions.ts @@ -3,15 +3,15 @@ import type { Action, ActionReturn } from '$runtime/action'; // ---------------- Action const href: Action = (node) => { - node.href = ''; - // @ts-expect-error - node.href = 1; + node.href = ''; + // @ts-expect-error + node.href = 1; }; href; const required: Action = (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 = (node, param) => { - node; - param; - return { - update: (p) => p === true, - destroy: () => {} - }; + node; + param; + return { + update: (p) => p === true, + destroy: () => {} + }; }; required1; const required2: Action = (node) => { - node; + node; }; required2; const required3: Action = (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 = (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 = (node, param?) => { - node; - param; - return { - update: (p) => p === true, - destroy: () => {} - }; + node; + param; + return { + update: (p) => p === true, + destroy: () => {} + }; }; optional1; const optional2: Action = (node) => { - node; + node; }; optional2; const optional3: Action = (node, param) => { - node; - param; + node; + param; }; optional3; const optional4: Action = (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 = (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 = (node) => { - node; - return { - destroy: () => {} - }; + node; + return { + destroy: () => {} + }; }; no1; @@ -113,32 +113,32 @@ no3; // @ts-expect-error update method given const no4: Action = (node) => { - return { - update: () => {}, - destroy: () => {} - }; + return { + update: () => {}, + destroy: () => {} + }; }; no4; // ---------------- ActionReturn const requiredReturn: ActionReturn = { - update: (p) => p.toString() + update: (p) => p.toString() }; requiredReturn; const optionalReturn: ActionReturn = { - 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; diff --git a/test/types/create-event-dispatcher.ts b/test/types/create-event-dispatcher.ts index d9fc6c65bd..37e31fd179 100644 --- a/test/types/create-event-dispatcher.ts +++ b/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 diff --git a/test/types/on-mount.ts b/test/types/on-mount.ts new file mode 100644 index 0000000000..47d272b8f5 --- /dev/null +++ b/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; +});