From 9886329fd8cc679737b925159a0c9f50d1beb46d Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 25 Apr 2024 18:00:20 +0200 Subject: [PATCH 01/22] feat: provide migration function Provides the start of a migration function, exported as `migrate` from `svelte/compiler`, which tries its best to automatically migrate towards runes, render tags (instead of slots) and event attributes (instead of event handlers) The preview REPL was updated with a migrate button so people can try it out in the playground. closes #9239 --- packages/svelte/src/compiler/index.js | 2 +- packages/svelte/src/compiler/migrate/index.js | 369 ++++++++++++++++++ packages/svelte/types/index.d.ts | 4 + playgrounds/sandbox/run.js | 9 +- .../src/lib/Input/ComponentSelector.svelte | 10 + .../src/lib/Input/Migrate.svelte | 22 ++ .../src/lib/Output/Compiler.js | 18 + sites/svelte-5-preview/src/lib/Repl.svelte | 22 ++ sites/svelte-5-preview/src/lib/types.d.ts | 1 + .../src/lib/workers/compiler/index.js | 26 ++ .../src/lib/workers/workers.d.ts | 6 + 11 files changed, 487 insertions(+), 2 deletions(-) create mode 100644 packages/svelte/src/compiler/migrate/index.js create mode 100644 sites/svelte-5-preview/src/lib/Input/Migrate.svelte diff --git a/packages/svelte/src/compiler/index.js b/packages/svelte/src/compiler/index.js index 97dcc364f0b8..ba312a1a639e 100644 --- a/packages/svelte/src/compiler/index.js +++ b/packages/svelte/src/compiler/index.js @@ -190,5 +190,5 @@ export function walk() { } export { CompileError } from './errors.js'; - export { VERSION } from '../version.js'; +export { migrate } from './migrate/index.js'; diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js new file mode 100644 index 000000000000..d5e17153e59e --- /dev/null +++ b/packages/svelte/src/compiler/migrate/index.js @@ -0,0 +1,369 @@ +import MagicString from 'magic-string'; +import { walk } from 'zimmerframe'; +import { parse } from '../phases/1-parse/index.js'; +import { analyze_component } from '../phases/2-analyze/index.js'; +import { validate_component_options } from '../validate-options.js'; +import { get_rune } from '../phases/scope.js'; +import { reset_warnings } from '../warnings.js'; + +/** + * Does a best-effort migration of Svelte code towards using runes, event attributes and render tags. + * @param {string} source + * @returns {string} + */ +export function migrate(source) { + try { + reset_warnings({ source, filename: 'migrate.svelte' }); + + let parsed = parse(source); + + const { customElement: customElementOptions, ...parsed_options } = parsed.options || {}; + + /** @type {import('#compiler').ValidatedCompileOptions} */ + const combined_options = { + ...validate_component_options({}, ''), + ...parsed_options, + customElementOptions + }; + + const str = new MagicString(source); + const analysis = analyze_component(parsed, source, combined_options); + + /** @type {State} */ + let state = { + scope: analysis.instance.scope, + analysis, + str, + props: [], + props_insertion_point: 0, + has_props_rune: false, + props_name: analysis.root.unique('props').name, + rest_props_name: analysis.root.unique('rest').name + }; + + if (parsed.instance) { + walk(parsed.instance.content, state, instance_script); + } + + state = { ...state, scope: analysis.template.scope }; + walk(parsed.fragment, state, template); + + if (state.props.length > 0 || analysis.uses_rest_props || analysis.uses_props) { + let props = ''; + if (analysis.uses_props) { + props = `...${state.props_name}`; + } else { + props = state.props + .map((prop) => { + let prop_str = + prop.local === prop.exported ? prop.local : `${prop.exported}: ${prop.local}`; + if (prop.bindable) { + prop_str += ` = $bindable(${prop.init})`; + } else if (prop.init) { + prop_str += ` = ${prop.init}`; + } + return prop_str; + }) + .join(', '); + + if (analysis.uses_rest_props) { + props += `, ...${state.rest_props_name}`; + } + } + + if (state.has_props_rune) { + // some render tags or forwarded event attributes to add + str.appendRight(state.props_insertion_point, ` ${props},`); + } else { + const props_declaration = `let { ${props} } = $props();`; + if (parsed.instance) { + if (state.props_insertion_point === 0) { + // no regular props found, but render tags or events to forward found, $props() will be first in the script tag + str.appendRight( + /** @type {number} */ (parsed.instance.content.start), + `\n\t${props_declaration}` + ); + } else { + str.appendRight(state.props_insertion_point, props_declaration); + } + } else { + str.prepend(``); + } + } + } + + return str.toString(); + } catch (e) { + console.error('Error while migrating Svelte code'); + throw e; + } +} + +/** + * @typedef {{ + * scope: import('../phases/scope.js').Scope; + * str: MagicString; + * analysis: import('../phases/types.js').ComponentAnalysis; + * props: Array<{ local: string; exported: string; init: string; bindable: boolean }>; + * props_insertion_point: number; + * has_props_rune: boolean; + * props_name: string; + * rest_props_name: string; + * }} State + */ + +/** @type {import('zimmerframe').Visitors} */ +const instance_script = { + Identifier(node, { state }) { + handle_identifier(node, state); + }, + VariableDeclaration(node, { state, path }) { + if (state.scope !== state.analysis.instance.scope) { + return; + } + + let nr_of_props = 0; + + for (const declarator of node.declarations) { + if (state.analysis.runes) { + if (get_rune(declarator.init, state.scope) === '$props') { + state.props_insertion_point = /** @type {number} */ (declarator.id.start) + 1; + state.has_props_rune = true; + } + continue; + } + + const bindings = state.scope.get_bindings(declarator); + const has_state = bindings.some((binding) => binding.kind === 'state'); + const has_props = bindings.some((binding) => binding.kind === 'bindable_prop'); + + if (!has_state && !has_props) { + continue; + } + + if (has_props) { + nr_of_props++; + + if (declarator.id.type !== 'Identifier') { + // TODO + // Turn export let into props. It's really really weird because export let { x: foo, z: [bar]} = .. + // means that foo and bar are the props (i.e. the leafs are the prop names), not x and z. + // const tmp = state.scope.generate('tmp'); + // const paths = extract_paths(declarator.id); + // state.props_pre.push( + // b.declaration('const', b.id(tmp), visit(declarator.init!) as Expression) + // ); + // for (const path of paths) { + // const name = (path.node as Identifier).name; + // const binding = state.scope.get(name)!; + // const value = path.expression!(b.id(tmp)); + // if (binding.kind === 'bindable_prop' || binding.kind === 'rest_prop') { + // state.props.push({ + // local: name, + // exported: binding.prop_alias ? binding.prop_alias : name, + // init: value + // }); + // state.props_insertion_point = /** @type {number} */(declarator.end); + // } else { + // declarations.push(b.declarator(path.node, value)); + // } + // } + continue; + } + + const binding = /** @type {import('#compiler').Binding} */ ( + state.scope.get(declarator.id.name) + ); + + if ( + state.analysis.uses_props && + (declarator.init || binding.mutated || binding.reassigned) + ) { + throw new Error( + '$$props is used together with named props in a way that cannot be automatically migrated.' + ); + } + + state.props.push({ + local: declarator.id.name, + exported: binding.prop_alias ? binding.prop_alias : declarator.id.name, + init: declarator.init + ? state.str.original.substring( + /** @type {number} */ (declarator.init.start), + /** @type {number} */ (declarator.init.end) + ) + : '', + bindable: binding.mutated || binding.reassigned + }); + state.props_insertion_point = /** @type {number} */ (declarator.end); + state.str.update( + /** @type {number} */ (declarator.start), + /** @type {number} */ (declarator.end), + '' + ); + + continue; + } + + // state + if (declarator.init) { + state.str.prependLeft(/** @type {number} */ (declarator.init.start), '$state('); + state.str.appendRight(/** @type {number} */ (declarator.init.end), ')'); + } else { + state.str.prependLeft(/** @type {number} */ (declarator.id.end), ' = $state()'); + } + } + + if (nr_of_props === node.declarations.length) { + let start = /** @type {number} */ (node.start); + let end = /** @type {number} */ (node.end); + + const parent = path.at(-1); + if (parent?.type === 'ExportNamedDeclaration') { + start = /** @type {number} */ (parent.start); + end = /** @type {number} */ (parent.end); + } + state.str.update(start, end, ''); + } + }, + LabeledStatement(node, { path, state }) { + if (state.analysis.runes) return; + if (path.length > 1) return; + if (node.label.name !== '$') return; + + if ( + node.body.type === 'ExpressionStatement' && + node.body.expression.type === 'AssignmentExpression' + ) { + // $derived + // TODO $: ({ x } = ...) + state.str.update( + /** @type {number} */ (node.start), + /** @type {number} */ (node.body.expression.start), + 'let ' + ); + state.str.prependLeft(/** @type {number} */ (node.body.expression.right.start), '$derived('); + state.str.appendRight(/** @type {number} */ (node.body.expression.right.end), ')'); + } else { + // $effect.pre, to be precise, but we gloss over that + // TODO try to find out if we can use $derived.by instead? + // TODO SSR mode variant needed + state.str.update( + /** @type {number} */ (node.start), + /** @type {number} */ (node.body.start), + '$effect(() => {' + ); + state.str.appendRight(/** @type {number} */ (node.end), '})'); + } + } +}; + +/** @type {import('zimmerframe').Visitors} */ +const template = { + Identifier(node, { state }) { + handle_identifier(node, state); + }, + OnDirective(node, { state, path }) { + const parent = path.at(-1); + if ( + parent?.type === 'SvelteSelf' || + parent?.type === 'SvelteComponent' || + parent?.type === 'Component' + ) { + return; + } + + if (node.expression) { + // remove : from on:click + state.str.update(node.start, node.start + 3, 'on'); + } else { + // turn on:click into a prop + // Check if prop already set, could happen when on:click on different elements + // TODO what to do when this results in a variable name clash? + if (!state.props.some((prop) => prop.local === node.name)) { + state.props.push({ + local: node.name, + exported: node.name, + init: '', + bindable: false + }); + } + + state.str.update(node.start, node.end, `{${node.name}}`); + } + }, + SlotElement(node, { state }) { + let name = 'children'; + let slot_props = '{ '; + + for (const attr of node.attributes) { + if (attr.type === 'SpreadAttribute') { + slot_props += `...${state.str.original.substring(/** @type {number} */ (attr.expression.start), attr.expression.end)}, `; + } else if (attr.type === 'Attribute') { + if (attr.name === 'name') { + name = /** @type {any} */ (attr.value)[0].data; + } else { + const value = + attr.value !== true + ? state.str.original.substring( + attr.value[0].start, + attr.value[attr.value.length - 1].end + ) + : 'true'; + slot_props += value === attr.name ? `${value}, ` : `${attr.name}: ${value}, `; + } + } + } + + slot_props += '}'; + if (slot_props === '{ }') { + slot_props = ''; + } + + state.props.push({ + local: name, + exported: name, + init: '', + bindable: false + }); + + if (node.fragment.nodes.length > 0) { + state.str.update( + node.start, + node.fragment.nodes[0].start, + `{#if ${name}}{@render ${name}(${slot_props})}{:else}` + ); + state.str.update(node.fragment.nodes[node.fragment.nodes.length - 1].end, node.end, '{/if}'); + } else { + state.str.update(node.start, node.end, `{@render ${name}?.(${slot_props})}`); + } + } +}; + +/** + * @param {import('estree').Identifier} node + * @param {State} state + */ +function handle_identifier(node, state) { + if (state.analysis.uses_props) { + if (node.name === '$$props' || node.name === '$$restProps') { + // not 100% correct for $$restProps but it'll do + state.str.update( + /** @type {number} */ (node.start), + /** @type {number} */ (node.end), + state.props_name + ); + } else { + const binding = state.scope.get(node.name); + if (binding?.kind === 'bindable_prop') { + state.str.prependLeft(/** @type {number} */ (node.start), `${state.props_name}.`); + } + } + } else if (node.name === '$$restProps' && state.analysis.uses_rest_props) { + state.str.update( + /** @type {number} */ (node.start), + /** @type {number} */ (node.end), + state.rest_props_name + ); + } +} diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 9540b52e575f..dc5d4009f952 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -1086,6 +1086,10 @@ declare module 'svelte/compiler' { * https://svelte.dev/docs/svelte-compiler#svelte-version * */ export const VERSION: string; + /** + * Does a best-effort migration of Svelte code towards using runes, event attributes and render tags. + * */ + export function migrate(source: string): string; class Scope { constructor(root: ScopeRoot, parent: Scope | null, porous: boolean); diff --git a/playgrounds/sandbox/run.js b/playgrounds/sandbox/run.js index d292d640ba37..a6589e993d0e 100644 --- a/playgrounds/sandbox/run.js +++ b/playgrounds/sandbox/run.js @@ -3,7 +3,7 @@ import * as path from 'node:path'; import { fileURLToPath } from 'node:url'; import glob from 'tiny-glob/sync.js'; import minimist from 'minimist'; -import { compile, compileModule, parse } from 'svelte/compiler'; +import { compile, compileModule, parse, migrate } from 'svelte/compiler'; const argv = minimist(process.argv.slice(2)); @@ -47,6 +47,13 @@ for (const generate of ['client', 'server']) { }); fs.writeFileSync(`${cwd}/output/${file}.json`, JSON.stringify(ast, null, '\t')); + + try { + const migrated = migrate(source); + fs.writeFileSync(`${cwd}/output/${file}.migrated.svelte`, migrated); + } catch (e) { + console.warn(`Error migrating ${file}`, e); + } } const compiled = compile(source, { diff --git a/sites/svelte-5-preview/src/lib/Input/ComponentSelector.svelte b/sites/svelte-5-preview/src/lib/Input/ComponentSelector.svelte index 71cd9241b8f8..c63ca416cf8a 100644 --- a/sites/svelte-5-preview/src/lib/Input/ComponentSelector.svelte +++ b/sites/svelte-5-preview/src/lib/Input/ComponentSelector.svelte @@ -3,6 +3,7 @@ import { get_full_filename } from '$lib/utils.js'; import { createEventDispatcher, tick } from 'svelte'; import RunesInfo from './RunesInfo.svelte'; + import Migrate from './Migrate.svelte'; /** @type {boolean} */ export let show_modified; @@ -296,6 +297,8 @@
+ +
diff --git a/sites/svelte-5-preview/src/lib/Output/Compiler.js b/sites/svelte-5-preview/src/lib/Output/Compiler.js index da11186a2f15..ae363682411a 100644 --- a/sites/svelte-5-preview/src/lib/Output/Compiler.js +++ b/sites/svelte-5-preview/src/lib/Output/Compiler.js @@ -67,6 +67,24 @@ export default class Compiler { }); } + /** + * @param {import('$lib/types').File} file + * @returns {Promise} + */ + migrate(file) { + return new Promise((fulfil) => { + const id = uid++; + + this.handlers.set(id, fulfil); + + this.worker.postMessage({ + id, + type: 'migrate', + source: file.source + }); + }); + } + destroy() { this.worker.terminate(); } diff --git a/sites/svelte-5-preview/src/lib/Repl.svelte b/sites/svelte-5-preview/src/lib/Repl.svelte index fd30aca10ee1..40f03d1730e1 100644 --- a/sites/svelte-5-preview/src/lib/Repl.svelte +++ b/sites/svelte-5-preview/src/lib/Repl.svelte @@ -137,6 +137,7 @@ EDITOR_STATE_MAP, rebundle, + migrate, clear_state, go_to_warning_pos, handle_change, @@ -156,6 +157,27 @@ resolver(); } + async function migrate() { + if (!compiler || $selected?.type !== 'svelte') return; + + const result = await compiler.migrate($selected); + if (result.error) { + // TODO show somehow + return; + } + + const new_files = $files.map((file) => { + if (file.name === $selected?.name) { + return { + ...file, + source: result.source + }; + } + return file; + }); + set({ files: new_files }); + } + let is_select_changing = false; /** diff --git a/sites/svelte-5-preview/src/lib/types.d.ts b/sites/svelte-5-preview/src/lib/types.d.ts index 7f978aec699c..a758846d293b 100644 --- a/sites/svelte-5-preview/src/lib/types.d.ts +++ b/sites/svelte-5-preview/src/lib/types.d.ts @@ -65,6 +65,7 @@ export type ReplContext = { // Methods rebundle(): Promise; + migrate(): Promise; handle_select(filename: string): Promise; handle_change( event: CustomEvent<{ diff --git a/sites/svelte-5-preview/src/lib/workers/compiler/index.js b/sites/svelte-5-preview/src/lib/workers/compiler/index.js index d264e702a05e..8f691f0cecad 100644 --- a/sites/svelte-5-preview/src/lib/workers/compiler/index.js +++ b/sites/svelte-5-preview/src/lib/workers/compiler/index.js @@ -41,6 +41,11 @@ self.addEventListener( await ready; postMessage(compile(event.data)); break; + + case 'migrate': + await ready; + postMessage(migrate(event.data)); + break; } } ); @@ -127,3 +132,24 @@ function compile({ id, source, options, return_ast }) { }; } } + +/** @param {import("../workers").MigrateMessageData} param0 */ +function migrate({ id, source }) { + try { + source = svelte.migrate(source); + + return { + id, + source + }; + } catch (err) { + // @ts-ignore + let message = `/*\nError migrating ${err.filename ?? 'component'}:\n${err.message}\n*/`; + + return { + id, + source, + error: message + }; + } +} diff --git a/sites/svelte-5-preview/src/lib/workers/workers.d.ts b/sites/svelte-5-preview/src/lib/workers/workers.d.ts index 47238b48125a..0903194839bc 100644 --- a/sites/svelte-5-preview/src/lib/workers/workers.d.ts +++ b/sites/svelte-5-preview/src/lib/workers/workers.d.ts @@ -26,3 +26,9 @@ export type BundleMessageData = { svelte_url: string; files: File[]; }; + +export type MigrateMessageData = { + id: number; + source: string; + error?: string; +}; From f47ffa2ad0ac35859a3034e9d740ecb91f632fa0 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 25 Apr 2024 20:42:55 +0200 Subject: [PATCH 02/22] indentation --- packages/svelte/src/compiler/migrate/index.js | 63 ++++++++++++++++--- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index d5e17153e59e..b29d1e69c550 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -28,12 +28,14 @@ export function migrate(source) { const str = new MagicString(source); const analysis = analyze_component(parsed, source, combined_options); + const indent = guess_indent(source); /** @type {State} */ let state = { scope: analysis.instance.scope, analysis, str, + indent, props: [], props_insertion_point: 0, has_props_rune: false, @@ -81,13 +83,13 @@ export function migrate(source) { // no regular props found, but render tags or events to forward found, $props() will be first in the script tag str.appendRight( /** @type {number} */ (parsed.instance.content.start), - `\n\t${props_declaration}` + `\n${indent}${props_declaration}` ); } else { str.appendRight(state.props_insertion_point, props_declaration); } } else { - str.prepend(``); + str.prepend(``); } } } @@ -104,6 +106,7 @@ export function migrate(source) { * scope: import('../phases/scope.js').Scope; * str: MagicString; * analysis: import('../phases/types.js').ComponentAnalysis; + * indent: string; * props: Array<{ local: string; exported: string; init: string; bindable: boolean }>; * props_insertion_point: number; * has_props_rune: boolean; @@ -245,15 +248,32 @@ const instance_script = { state.str.prependLeft(/** @type {number} */ (node.body.expression.right.start), '$derived('); state.str.appendRight(/** @type {number} */ (node.body.expression.right.end), ')'); } else { + const is_block_stmt = node.body.type === 'BlockStatement'; + const start_end = /** @type {number} */ (node.body.start); // $effect.pre, to be precise, but we gloss over that // TODO try to find out if we can use $derived.by instead? // TODO SSR mode variant needed - state.str.update( - /** @type {number} */ (node.start), - /** @type {number} */ (node.body.start), - '$effect(() => {' - ); - state.str.appendRight(/** @type {number} */ (node.end), '})'); + if (is_block_stmt) { + state.str.update(/** @type {number} */ (node.start), start_end + 1, '$effect(() => {'); + const end = /** @type {number} */ (node.body.end); + state.str.update(end - 1, end, '});'); + } else { + state.str.update( + /** @type {number} */ (node.start), + start_end, + `$effect(() => {\n${state.indent}` + ); + state.str.indent(state.indent, { + exclude: [ + [0, /** @type {number} */ (node.body.start)], + [ + /** @type {number} */ (node.body.end), + /** @type {number} */ (/** @type {import('estree').Program} */ (path.at(-1)).end) + ] + ] + }); + state.str.appendRight(/** @type {number} */ (node.end), `\n${state.indent}});`); + } } } }; @@ -367,3 +387,30 @@ function handle_identifier(node, state) { ); } } + +/** @param {string} content */ +function guess_indent(content) { + const lines = content.split('\n'); + + const tabbed = lines.filter((line) => /^\t+/.test(line)); + const spaced = lines.filter((line) => /^ {2,}/.test(line)); + + if (tabbed.length === 0 && spaced.length === 0) { + return '\t'; + } + + // More lines tabbed than spaced? Assume tabs, and + // default to tabs in the case of a tie (or nothing + // to go on) + if (tabbed.length >= spaced.length) { + return '\t'; + } + + // Otherwise, we need to guess the multiple + const min = spaced.reduce((previous, current) => { + const count = /^ +/.exec(current)?.[0].length ?? 0; + return Math.min(count, previous); + }, Infinity); + + return ' '.repeat(min); +} From cf5d09d5594d159d72a32cbabdfc1d74399df05a Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 25 Apr 2024 20:46:33 +0200 Subject: [PATCH 03/22] more robust $derived --- packages/svelte/src/compiler/migrate/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index b29d1e69c550..87014fa314da 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -239,14 +239,17 @@ const instance_script = { node.body.expression.type === 'AssignmentExpression' ) { // $derived - // TODO $: ({ x } = ...) state.str.update( /** @type {number} */ (node.start), /** @type {number} */ (node.body.expression.start), 'let ' ); state.str.prependLeft(/** @type {number} */ (node.body.expression.right.start), '$derived('); - state.str.appendRight(/** @type {number} */ (node.body.expression.right.end), ')'); + state.str.update( + /** @type {number} */ (node.body.expression.right.end), + /** @type {number} */ (node.end), + ');' + ); } else { const is_block_stmt = node.body.type === 'BlockStatement'; const start_end = /** @type {number} */ (node.body.start); From fcd9dab94cc385c30659ae28bd3d46f05e2821bf Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 25 Apr 2024 21:03:00 +0200 Subject: [PATCH 04/22] handle implicitly-declared and reassigned $: vars --- packages/svelte/src/compiler/migrate/index.js | 90 +++++++++++-------- 1 file changed, 55 insertions(+), 35 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 87014fa314da..fe738d1a4556 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -5,6 +5,7 @@ import { analyze_component } from '../phases/2-analyze/index.js'; import { validate_component_options } from '../validate-options.js'; import { get_rune } from '../phases/scope.js'; import { reset_warnings } from '../warnings.js'; +import { extract_identifiers } from '../utils/ast.js'; /** * Does a best-effort migration of Svelte code towards using runes, event attributes and render tags. @@ -238,46 +239,65 @@ const instance_script = { node.body.type === 'ExpressionStatement' && node.body.expression.type === 'AssignmentExpression' ) { - // $derived - state.str.update( - /** @type {number} */ (node.start), - /** @type {number} */ (node.body.expression.start), - 'let ' - ); - state.str.prependLeft(/** @type {number} */ (node.body.expression.right.start), '$derived('); - state.str.update( - /** @type {number} */ (node.body.expression.right.end), - /** @type {number} */ (node.end), - ');' - ); - } else { - const is_block_stmt = node.body.type === 'BlockStatement'; - const start_end = /** @type {number} */ (node.body.start); - // $effect.pre, to be precise, but we gloss over that - // TODO try to find out if we can use $derived.by instead? - // TODO SSR mode variant needed - if (is_block_stmt) { - state.str.update(/** @type {number} */ (node.start), start_end + 1, '$effect(() => {'); - const end = /** @type {number} */ (node.body.end); - state.str.update(end - 1, end, '});'); - } else { + const ids = extract_identifiers(node.body.expression.left); + const reassigned_ids = ids.filter((id) => state.scope.get(id.name)?.reassigned); + if (reassigned_ids.length === 0) { + // $derived state.str.update( /** @type {number} */ (node.start), - start_end, - `$effect(() => {\n${state.indent}` + /** @type {number} */ (node.body.expression.start), + 'let ' ); - state.str.indent(state.indent, { - exclude: [ - [0, /** @type {number} */ (node.body.start)], - [ - /** @type {number} */ (node.body.end), - /** @type {number} */ (/** @type {import('estree').Program} */ (path.at(-1)).end) - ] - ] - }); - state.str.appendRight(/** @type {number} */ (node.end), `\n${state.indent}});`); + state.str.prependLeft( + /** @type {number} */ (node.body.expression.right.start), + '$derived(' + ); + state.str.update( + /** @type {number} */ (node.body.expression.right.end), + /** @type {number} */ (node.end), + ');' + ); + return; + } else { + for (const id of reassigned_ids) { + const binding = state.scope.get(id.name); + if (binding?.node === id) { + // implicitly-declared variable which we need to make explicit + state.str.prependLeft( + /** @type {number} */ (node.start), + `let ${id.name}${binding.kind === 'state' ? ' = $state()' : ''};\n${state.indent}` + ); + } + } } } + + const is_block_stmt = node.body.type === 'BlockStatement'; + const start_end = /** @type {number} */ (node.body.start); + // $effect.pre, to be precise, but we gloss over that + // TODO try to find out if we can use $derived.by instead? + // TODO SSR mode variant needed + if (is_block_stmt) { + state.str.update(/** @type {number} */ (node.start), start_end + 1, '$effect(() => {'); + const end = /** @type {number} */ (node.body.end); + state.str.update(end - 1, end, '});'); + } else { + state.str.update( + /** @type {number} */ (node.start), + start_end, + `$effect(() => {\n${state.indent}` + ); + state.str.indent(state.indent, { + exclude: [ + [0, /** @type {number} */ (node.body.start)], + [ + /** @type {number} */ (node.body.end), + /** @type {number} */ (/** @type {import('estree').Program} */ (path.at(-1)).end) + ] + ] + }); + state.str.appendRight(/** @type {number} */ (node.end), `\n${state.indent}});`); + } } }; From bae1a5638c3174f79fe8dfa3014636293def2fee Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 25 Apr 2024 21:10:38 +0200 Subject: [PATCH 05/22] fix indent --- packages/svelte/src/compiler/migrate/index.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index fe738d1a4556..ff15cd621aba 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -41,7 +41,8 @@ export function migrate(source) { props_insertion_point: 0, has_props_rune: false, props_name: analysis.root.unique('props').name, - rest_props_name: analysis.root.unique('rest').name + rest_props_name: analysis.root.unique('rest').name, + end: parsed.end }; if (parsed.instance) { @@ -113,6 +114,7 @@ export function migrate(source) { * has_props_rune: boolean; * props_name: string; * rest_props_name: string; + * end: number; * }} State */ @@ -290,10 +292,7 @@ const instance_script = { state.str.indent(state.indent, { exclude: [ [0, /** @type {number} */ (node.body.start)], - [ - /** @type {number} */ (node.body.end), - /** @type {number} */ (/** @type {import('estree').Program} */ (path.at(-1)).end) - ] + [/** @type {number} */ (node.body.end), state.end] ] }); state.str.appendRight(/** @type {number} */ (node.end), `\n${state.indent}});`); From 2620c468668e79b8a6ce9b118da686c5923b9198 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 25 Apr 2024 21:11:43 +0200 Subject: [PATCH 06/22] oops --- packages/svelte/src/compiler/phases/scope.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index d815ff88fd39..9778d1e04447 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -704,7 +704,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { } else { extract_identifiers(node).forEach((identifier) => { const binding = scope.get(identifier.name); - if (binding) { + if (binding && identifier !== binding.node) { binding.mutated = true; binding.reassigned = true; } From 05e8e2e177b5bbf81bf6c8f3aaa4536333f96923 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 10:54:04 +0200 Subject: [PATCH 07/22] add run migration helper method for replacement of `$:` --- packages/svelte/src/compiler/migrate/index.js | 30 +++++++++++++++---- packages/svelte/src/legacy/legacy-client.js | 12 ++++++++ packages/svelte/src/legacy/legacy-server.js | 11 +++++++ packages/svelte/types/index.d.ts | 6 ++++ 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index ff15cd621aba..175085d14c43 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -42,7 +42,9 @@ export function migrate(source) { has_props_rune: false, props_name: analysis.root.unique('props').name, rest_props_name: analysis.root.unique('rest').name, - end: parsed.end + end: parsed.end, + run_name: analysis.root.unique('run').name, + needs_run: false }; if (parsed.instance) { @@ -52,6 +54,9 @@ export function migrate(source) { state = { ...state, scope: analysis.template.scope }; walk(parsed.fragment, state, template); + const run_import = `import { run${state.run_name === 'run' ? '' : `as ${state.run_name}`} } from 'svelte/legacy';`; + let added_legacy_import = false; + if (state.props.length > 0 || analysis.uses_rest_props || analysis.uses_props) { let props = ''; if (analysis.uses_props) { @@ -91,13 +96,27 @@ export function migrate(source) { str.appendRight(state.props_insertion_point, props_declaration); } } else { - str.prepend(``); + const imports = state.needs_run ? `${indent}${run_import}\n` : ''; + str.prepend(``); + added_legacy_import = true; } } } + if (state.needs_run && !added_legacy_import) { + if (parsed.instance) { + str.appendRight( + /** @type {number} */ (parsed.instance.content.start), + `\n${indent}${run_import}\n` + ); + } else { + str.prepend(``); + } + } + return str.toString(); } catch (e) { + // eslint-disable-next-line no-console console.error('Error while migrating Svelte code'); throw e; } @@ -115,6 +134,8 @@ export function migrate(source) { * props_name: string; * rest_props_name: string; * end: number; + * run_name: string; + * needs_run: boolean; * }} State */ @@ -276,18 +297,17 @@ const instance_script = { const is_block_stmt = node.body.type === 'BlockStatement'; const start_end = /** @type {number} */ (node.body.start); - // $effect.pre, to be precise, but we gloss over that // TODO try to find out if we can use $derived.by instead? - // TODO SSR mode variant needed if (is_block_stmt) { state.str.update(/** @type {number} */ (node.start), start_end + 1, '$effect(() => {'); const end = /** @type {number} */ (node.body.end); state.str.update(end - 1, end, '});'); } else { + state.needs_run = true; state.str.update( /** @type {number} */ (node.start), start_end, - `$effect(() => {\n${state.indent}` + `${state.run_name}(() => {\n${state.indent}` ); state.str.indent(state.indent, { exclude: [ diff --git a/packages/svelte/src/legacy/legacy-client.js b/packages/svelte/src/legacy/legacy-client.js index 570a7c845655..8784489c4d48 100644 --- a/packages/svelte/src/legacy/legacy-client.js +++ b/packages/svelte/src/legacy/legacy-client.js @@ -1,4 +1,5 @@ import { proxy } from '../internal/client/proxy.js'; +import { user_pre_effect } from '../internal/client/reactivity/effects.js'; import { hydrate, mount, unmount } from '../internal/client/render.js'; import { define_property } from '../internal/client/utils.js'; @@ -128,3 +129,14 @@ class Svelte4Component { this.#instance.$destroy(); } } + +/** + * Runs the given function once immediately on the server, and works like `$effect.pre` on the client. + * + * @deprecated Use this only as a temporary solution to migrate your component code to Svelte 5. + * @param {() => void | (() => void)} fn + * @returns {void} + */ +export function run(fn) { + user_pre_effect(fn); +} diff --git a/packages/svelte/src/legacy/legacy-server.js b/packages/svelte/src/legacy/legacy-server.js index 6b8d5bfd2cc4..dad89d06038f 100644 --- a/packages/svelte/src/legacy/legacy-server.js +++ b/packages/svelte/src/legacy/legacy-server.js @@ -36,3 +36,14 @@ export function asClassComponent(component) { // @ts-ignore return component_constructor; } + +/** + * Runs the given function once immediately on the server, and works like `$effect.pre` on the client. + * + * @deprecated Use this only as a temporary solution to migrate your component code to Svelte 5. + * @param {() => void | (() => void)} fn + * @returns {void} + */ +export function run(fn) { + fn(); +} diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index dc5d4009f952..72e3a22485c1 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -1963,6 +1963,12 @@ declare module 'svelte/legacy' { * * */ export function asClassComponent, Exports extends Record, Events extends Record, Slots extends Record>(component: import("svelte").SvelteComponent): import("svelte").ComponentType & Exports>; + /** + * Runs the given function once immediately on the server, and works like `$effect.pre` on the client. + * + * @deprecated Use this only as a temporary solution to migrate your component code to Svelte 5. + * */ + export function run(fn: () => void | (() => void)): void; } declare module 'svelte/motion' { From d31a8b3574f69b0f52ba4c4a1af9ef2087eff11a Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 12:58:43 +0200 Subject: [PATCH 08/22] tests, loads of fixes --- packages/svelte/src/compiler/migrate/index.js | 206 +++++++++++++++--- .../src/compiler/phases/1-parse/index.js | 4 +- .../migrate/samples/derivations/input.svelte | 7 + .../migrate/samples/derivations/output.svelte | 7 + .../migrate/samples/effects/input.svelte | 10 + .../migrate/samples/effects/output.svelte | 16 ++ .../samples/event-handlers/input.svelte | 11 + .../samples/event-handlers/output.svelte | 28 +++ .../samples/props-rest-props/input.svelte | 5 + .../samples/props-rest-props/output.svelte | 5 + .../tests/migrate/samples/props/input.svelte | 11 + .../tests/migrate/samples/props/output.svelte | 11 + .../tests/migrate/samples/slots/input.svelte | 5 + .../tests/migrate/samples/slots/output.svelte | 9 + packages/svelte/tests/migrate/test.ts | 30 +++ 15 files changed, 330 insertions(+), 35 deletions(-) create mode 100644 packages/svelte/tests/migrate/samples/derivations/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/derivations/output.svelte create mode 100644 packages/svelte/tests/migrate/samples/effects/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/effects/output.svelte create mode 100644 packages/svelte/tests/migrate/samples/event-handlers/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/event-handlers/output.svelte create mode 100644 packages/svelte/tests/migrate/samples/props-rest-props/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/props-rest-props/output.svelte create mode 100644 packages/svelte/tests/migrate/samples/props/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/props/output.svelte create mode 100644 packages/svelte/tests/migrate/samples/slots/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/slots/output.svelte create mode 100644 packages/svelte/tests/migrate/test.ts diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 175085d14c43..8e1a6901a1a4 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -6,6 +6,7 @@ import { validate_component_options } from '../validate-options.js'; import { get_rune } from '../phases/scope.js'; import { reset_warnings } from '../warnings.js'; import { extract_identifiers } from '../utils/ast.js'; +import { regex_is_valid_identifier } from '../phases/patterns.js'; /** * Does a best-effort migration of Svelte code towards using runes, event attributes and render tags. @@ -42,7 +43,7 @@ export function migrate(source) { has_props_rune: false, props_name: analysis.root.unique('props').name, rest_props_name: analysis.root.unique('rest').name, - end: parsed.end, + end: source.length, run_name: analysis.root.unique('run').name, needs_run: false }; @@ -97,7 +98,7 @@ export function migrate(source) { } } else { const imports = state.needs_run ? `${indent}${run_import}\n` : ''; - str.prepend(``); + str.prepend(`\n\n`); added_legacy_import = true; } } @@ -110,7 +111,7 @@ export function migrate(source) { `\n${indent}${run_import}\n` ); } else { - str.prepend(``); + str.prepend(`\n\n`); } } @@ -295,15 +296,19 @@ const instance_script = { } } + state.needs_run = true; const is_block_stmt = node.body.type === 'BlockStatement'; const start_end = /** @type {number} */ (node.body.start); // TODO try to find out if we can use $derived.by instead? if (is_block_stmt) { - state.str.update(/** @type {number} */ (node.start), start_end + 1, '$effect(() => {'); + state.str.update( + /** @type {number} */ (node.start), + start_end + 1, + `${state.run_name}(() => {` + ); const end = /** @type {number} */ (node.body.end); state.str.update(end - 1, end, '});'); } else { - state.needs_run = true; state.str.update( /** @type {number} */ (node.start), start_end, @@ -325,36 +330,27 @@ const template = { Identifier(node, { state }) { handle_identifier(node, state); }, - OnDirective(node, { state, path }) { - const parent = path.at(-1); - if ( - parent?.type === 'SvelteSelf' || - parent?.type === 'SvelteComponent' || - parent?.type === 'Component' - ) { - return; - } - - if (node.expression) { - // remove : from on:click - state.str.update(node.start, node.start + 3, 'on'); - } else { - // turn on:click into a prop - // Check if prop already set, could happen when on:click on different elements - // TODO what to do when this results in a variable name clash? - if (!state.props.some((prop) => prop.local === node.name)) { - state.props.push({ - local: node.name, - exported: node.name, - init: '', - bindable: false - }); - } - - state.str.update(node.start, node.end, `{${node.name}}`); - } + RegularElement(node, { state, next }) { + handle_events(node, state); + next(); + }, + SvelteElement(node, { state, next }) { + handle_events(node, state); + next(); + }, + SvelteWindow(node, { state, next }) { + handle_events(node, state); + next(); }, - SlotElement(node, { state }) { + SvelteBody(node, { state, next }) { + handle_events(node, state); + next(); + }, + SvelteDocument(node, { state, next }) { + handle_events(node, state); + next(); + }, + SlotElement(node, { state, next }) { let name = 'children'; let slot_props = '{ '; @@ -390,6 +386,7 @@ const template = { }); if (node.fragment.nodes.length > 0) { + next(); state.str.update( node.start, node.fragment.nodes[0].start, @@ -402,6 +399,147 @@ const template = { } }; +/** + * @param {import('#compiler').RegularElement | import('#compiler').SvelteElement | import('#compiler').SvelteWindow | import('#compiler').SvelteDocument | import('#compiler').SvelteBody} node + * @param {State} state + */ +function handle_events(node, state) { + /** @type {Map} */ + const handlers = new Map(); + for (const attribute of node.attributes) { + if (attribute.type !== 'OnDirective') continue; + + const nodes = handlers.get(attribute.name) || []; + nodes.push(attribute); + handlers.set(attribute.name, nodes); + } + + for (const [name, nodes] of handlers) { + // turn on:click into a prop + let exported = `on${name}`; + if (!regex_is_valid_identifier.test(name)) { + exported = `'${name}'`; + } + // Check if prop already set, could happen when on:click on different elements + let local = state.props.find((prop) => prop.exported === exported)?.local; + + const last = nodes[nodes.length - 1]; + const payload_name = + last.expression?.type === 'ArrowFunctionExpression' && + last.expression.params[0]?.type === 'Identifier' + ? last.expression.params[0].name + : state.scope.generate('payload'); + let prepend = ''; + + for (let i = 0; i < nodes.length - 1; i += 1) { + const node = nodes[i]; + if (node.expression) { + let body = ''; + if (node.expression.type === 'ArrowFunctionExpression') { + body = state.str.original.substring( + /** @type {number} */ (node.expression.body.start), + /** @type {number} */ (node.expression.body.end) + ); + } else { + body = `${state.str.original.substring( + /** @type {number} */ (node.expression.start), + /** @type {number} */ (node.expression.end) + )}();`; + } + // TODO check how many indents needed + prepend += `\n${state.indent}${body}\n`; + } else { + if (!local) { + local = state.scope.generate(`on${node.name}`); + state.props.push({ + local, + exported, + init: '', + bindable: false + }); + } + prepend += `\n${state.indent}${local}?.(${payload_name});\n`; + } + + state.str.remove(node.start, node.end); + } + + if (last.expression) { + // remove : from on:click + state.str.update(last.start, last.start + 3, 'on'); + if (prepend) { + let pos = last.expression.start; + if (last.expression.type === 'ArrowFunctionExpression') { + pos = last.expression.body.start; + if ( + last.expression.params.length > 0 && + last.expression.params[0].type !== 'Identifier' + ) { + const start = /** @type {number} */ (last.expression.params[0].start); + const end = /** @type {number} */ (last.expression.params[0].end); + // replace event payload with generated one that others use, + // then destructure generated payload param into what the user wrote + state.str.overwrite(start, end, payload_name); + prepend = `let ${state.str.original.substring( + start, + end + )} = ${payload_name};\n${prepend}`; + } else if (last.expression.params.length === 0) { + // add generated payload param to arrow function + const pos = state.str.original.lastIndexOf(')', last.expression.body.start); + state.str.prependLeft(pos, payload_name); + } + + const needs_curlies = last.expression.body.type !== 'BlockStatement'; + state.str.prependRight( + /** @type {number} */ (pos), + `${needs_curlies ? '{' : ''}${prepend}${state.indent}` + ); + state.str.appendRight( + /** @type {number} */ (last.expression.body.end), + `\n${needs_curlies ? '}' : ''}` + ); + } else { + state.str.update( + /** @type {number} */ (last.expression.start), + /** @type {number} */ (last.expression.end), + `(${payload_name}) => {${prepend}\n${state.indent}${state.str.original.substring( + /** @type {number} */ (last.expression.start), + /** @type {number} */ (last.expression.end) + )}?.(${payload_name});\n}` + ); + } + } + } else { + // turn on:click into a prop + // Check if prop already set, could happen when on:click on different elements + if (!local) { + local = state.scope.generate(`on${last.name}`); + state.props.push({ + local, + exported, + init: '', + bindable: false + }); + } + + const event_name = `on${name}`; + let replacement = ''; + if (!prepend) { + if (exported === local) { + replacement = `{${event_name}}`; + } else { + replacement = `${event_name}={${local}}`; + } + } else { + replacement = `${event_name}={(${payload_name}) => {${prepend}\n${state.indent}${local}?.(${payload_name});\n}}`; + } + + state.str.update(last.start, last.end, replacement); + } + } +} + /** * @param {import('estree').Identifier} node * @param {State} state diff --git a/packages/svelte/src/compiler/phases/1-parse/index.js b/packages/svelte/src/compiler/phases/1-parse/index.js index d5f5cda052db..830cf497cb04 100644 --- a/packages/svelte/src/compiler/phases/1-parse/index.js +++ b/packages/svelte/src/compiler/phases/1-parse/index.js @@ -64,7 +64,9 @@ export class Parser { this.root = { css: null, - js: [], + instance: null, + module: null, + parent: null, // @ts-ignore start: null, // @ts-ignore diff --git a/packages/svelte/tests/migrate/samples/derivations/input.svelte b/packages/svelte/tests/migrate/samples/derivations/input.svelte new file mode 100644 index 000000000000..9fde4a4359f2 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/derivations/input.svelte @@ -0,0 +1,7 @@ + + +{count} / {doubled} / {quadrupled} diff --git a/packages/svelte/tests/migrate/samples/derivations/output.svelte b/packages/svelte/tests/migrate/samples/derivations/output.svelte new file mode 100644 index 000000000000..f6af6dd45069 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/derivations/output.svelte @@ -0,0 +1,7 @@ + + +{count} / {doubled} / {quadrupled} \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/effects/input.svelte b/packages/svelte/tests/migrate/samples/effects/input.svelte new file mode 100644 index 000000000000..98002a0d7feb --- /dev/null +++ b/packages/svelte/tests/migrate/samples/effects/input.svelte @@ -0,0 +1,10 @@ + diff --git a/packages/svelte/tests/migrate/samples/effects/output.svelte b/packages/svelte/tests/migrate/samples/effects/output.svelte new file mode 100644 index 000000000000..28ba67a0ad47 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/effects/output.svelte @@ -0,0 +1,16 @@ + \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/event-handlers/input.svelte b/packages/svelte/tests/migrate/samples/event-handlers/input.svelte new file mode 100644 index 000000000000..e84bee3ed009 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/event-handlers/input.svelte @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/packages/svelte/tests/migrate/samples/event-handlers/output.svelte b/packages/svelte/tests/migrate/samples/event-handlers/output.svelte new file mode 100644 index 000000000000..474c48dc34df --- /dev/null +++ b/packages/svelte/tests/migrate/samples/event-handlers/output.svelte @@ -0,0 +1,28 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/props-rest-props/input.svelte b/packages/svelte/tests/migrate/samples/props-rest-props/input.svelte new file mode 100644 index 000000000000..2086c32c526a --- /dev/null +++ b/packages/svelte/tests/migrate/samples/props-rest-props/input.svelte @@ -0,0 +1,5 @@ + + + diff --git a/packages/svelte/tests/migrate/samples/props-rest-props/output.svelte b/packages/svelte/tests/migrate/samples/props-rest-props/output.svelte new file mode 100644 index 000000000000..3a872b15850b --- /dev/null +++ b/packages/svelte/tests/migrate/samples/props-rest-props/output.svelte @@ -0,0 +1,5 @@ + + + \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/props/input.svelte b/packages/svelte/tests/migrate/samples/props/input.svelte new file mode 100644 index 000000000000..b29e9be48022 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/props/input.svelte @@ -0,0 +1,11 @@ + + +{readonly} +{optional} + + diff --git a/packages/svelte/tests/migrate/samples/props/output.svelte b/packages/svelte/tests/migrate/samples/props/output.svelte new file mode 100644 index 000000000000..d1c327d3ebd5 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/props/output.svelte @@ -0,0 +1,11 @@ + + +{readonly} +{optional} + + \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/slots/input.svelte b/packages/svelte/tests/migrate/samples/slots/input.svelte new file mode 100644 index 000000000000..c383095f75c3 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/slots/input.svelte @@ -0,0 +1,5 @@ + + +{#if foo} + +{/if} diff --git a/packages/svelte/tests/migrate/samples/slots/output.svelte b/packages/svelte/tests/migrate/samples/slots/output.svelte new file mode 100644 index 000000000000..79ba8f3a9724 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/slots/output.svelte @@ -0,0 +1,9 @@ + + + + +{#if foo} + {@render foo?.({ foo, })} +{/if} \ No newline at end of file diff --git a/packages/svelte/tests/migrate/test.ts b/packages/svelte/tests/migrate/test.ts new file mode 100644 index 000000000000..71d774328f0f --- /dev/null +++ b/packages/svelte/tests/migrate/test.ts @@ -0,0 +1,30 @@ +import * as fs from 'node:fs'; +import { assert } from 'vitest'; +import { migrate } from 'svelte/compiler'; +import { try_read_file } from '../helpers.js'; +import { suite, type BaseTest } from '../suite.js'; + +interface ParserTest extends BaseTest {} + +const { test, run } = suite(async (config, cwd) => { + const input = fs + .readFileSync(`${cwd}/input.svelte`, 'utf-8') + .replace(/\s+$/, '') + .replace(/\r/g, ''); + + const actual = migrate(input); + + // run `UPDATE_SNAPSHOTS=true pnpm test migrate` to update parser tests + if (process.env.UPDATE_SNAPSHOTS || !fs.existsSync(`${cwd}/output.svelte`)) { + fs.writeFileSync(`${cwd}/output.svelte`, actual); + } else { + fs.writeFileSync(`${cwd}/_actual.svelte`, actual); + + const expected = try_read_file(`${cwd}/output.svelte`); + assert.deepEqual(actual, expected); + } +}); + +export { test }; + +await run(__dirname); From ebd25239ad3171dace220bef2055aa67f8d47252 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 13:03:36 +0200 Subject: [PATCH 09/22] changeset --- .changeset/orange-zoos-heal.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/orange-zoos-heal.md diff --git a/.changeset/orange-zoos-heal.md b/.changeset/orange-zoos-heal.md new file mode 100644 index 000000000000..f134a4b8a292 --- /dev/null +++ b/.changeset/orange-zoos-heal.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +feat: provide migration helper From 27a5c62e01759981e28b0a141c1824d87f65c393 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 14:02:30 +0200 Subject: [PATCH 10/22] revert this unrelated change --- packages/svelte/src/compiler/phases/1-parse/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/svelte/src/compiler/phases/1-parse/index.js b/packages/svelte/src/compiler/phases/1-parse/index.js index 830cf497cb04..d5f5cda052db 100644 --- a/packages/svelte/src/compiler/phases/1-parse/index.js +++ b/packages/svelte/src/compiler/phases/1-parse/index.js @@ -64,9 +64,7 @@ export class Parser { this.root = { css: null, - instance: null, - module: null, - parent: null, + js: [], // @ts-ignore start: null, // @ts-ignore From 394d61c7d2a321648dc9aad2e52758220561e9b4 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 14:26:34 +0200 Subject: [PATCH 11/22] props type --- packages/svelte/src/compiler/migrate/index.js | 59 +++++++++++++++++-- .../migrate/samples/props-ts/input.svelte | 11 ++++ .../migrate/samples/props-ts/output.svelte | 11 ++++ 3 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 packages/svelte/tests/migrate/samples/props-ts/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/props-ts/output.svelte diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 8e1a6901a1a4..69f5d73f1aac 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -85,7 +85,24 @@ export function migrate(source) { // some render tags or forwarded event attributes to add str.appendRight(state.props_insertion_point, ` ${props},`); } else { - const props_declaration = `let { ${props} } = $props();`; + let type = ''; + if ( + parsed.instance?.attributes.some( + (attr) => attr.name === 'lang' && /** @type {any} */ (attr).value[0].data === 'ts' + ) + ) { + if (analysis.uses_props || analysis.uses_rest_props) { + type = ' : Record'; + } else { + type = ` : { ${state.props + .map((prop) => { + return `${prop.exported}${prop.optional ? '?' : ''}: ${prop.type}`; + }) + .join(', ')} }`; + } + } + + const props_declaration = `let { ${props} }${type} = $props();`; if (parsed.instance) { if (state.props_insertion_point === 0) { // no regular props found, but render tags or events to forward found, $props() will be first in the script tag @@ -129,7 +146,7 @@ export function migrate(source) { * str: MagicString; * analysis: import('../phases/types.js').ComponentAnalysis; * indent: string; - * props: Array<{ local: string; exported: string; init: string; bindable: boolean }>; + * props: Array<{ local: string; exported: string; init: string; bindable: boolean; optional: boolean; type: string }>; * props_insertion_point: number; * has_props_rune: boolean; * props_name: string; @@ -221,6 +238,8 @@ const instance_script = { /** @type {number} */ (declarator.init.end) ) : '', + optional: !!declarator.init, + type: extract_type(declarator, state.str), bindable: binding.mutated || binding.reassigned }); state.props_insertion_point = /** @type {number} */ (declarator.end); @@ -382,7 +401,9 @@ const template = { local: name, exported: name, init: '', - bindable: false + bindable: false, + optional: true, + type: slot_props ? 'Snippet' : 'Snippet<[any]>' }); if (node.fragment.nodes.length > 0) { @@ -399,6 +420,30 @@ const template = { } }; +/** + * @param {import('estree').VariableDeclarator} declarator + * @param {MagicString} str + */ +function extract_type(declarator, str) { + if (declarator.id.typeAnnotation) { + let start = declarator.id.typeAnnotation.start + 1; // skip the colon + while (str.original[start] === ' ') { + start++; + } + return str.original.substring(start, declarator.id.typeAnnotation.end); + } + + // try to infer it from the init + if (declarator.init?.type === 'Literal') { + const type = typeof declarator.init.value; + if (type === 'string' || type === 'number' || type === 'boolean') { + return type; + } + } + + return 'any'; +} + /** * @param {import('#compiler').RegularElement | import('#compiler').SvelteElement | import('#compiler').SvelteWindow | import('#compiler').SvelteDocument | import('#compiler').SvelteBody} node * @param {State} state @@ -455,7 +500,9 @@ function handle_events(node, state) { local, exported, init: '', - bindable: false + bindable: false, + optional: true, + type: '(payload: any) => void' }); } prepend += `\n${state.indent}${local}?.(${payload_name});\n`; @@ -519,7 +566,9 @@ function handle_events(node, state) { local, exported, init: '', - bindable: false + bindable: false, + optional: true, + type: '(payload: any) => void' }); } diff --git a/packages/svelte/tests/migrate/samples/props-ts/input.svelte b/packages/svelte/tests/migrate/samples/props-ts/input.svelte new file mode 100644 index 000000000000..373863064913 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/props-ts/input.svelte @@ -0,0 +1,11 @@ + + +{readonly} +{optional} + + diff --git a/packages/svelte/tests/migrate/samples/props-ts/output.svelte b/packages/svelte/tests/migrate/samples/props-ts/output.svelte new file mode 100644 index 000000000000..12d1ced2a079 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/props-ts/output.svelte @@ -0,0 +1,11 @@ + + +{readonly} +{optional} + + \ No newline at end of file From 58cb0a9b14de28fb81212e57006eb853f03d39f6 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 15:11:14 +0200 Subject: [PATCH 12/22] nicer types, delete empty lines --- packages/svelte/src/compiler/migrate/index.js | 35 ++++++++++--------- .../migrate/samples/props-ts/output.svelte | 17 ++++++--- .../tests/migrate/samples/props/output.svelte | 10 +++--- 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 69f5d73f1aac..5de2bebc6552 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -39,7 +39,7 @@ export function migrate(source) { str, indent, props: [], - props_insertion_point: 0, + props_insertion_point: parsed.instance?.content.start ?? 0, has_props_rune: false, props_name: analysis.root.unique('props').name, rest_props_name: analysis.root.unique('rest').name, @@ -59,6 +59,8 @@ export function migrate(source) { let added_legacy_import = false; if (state.props.length > 0 || analysis.uses_rest_props || analysis.uses_props) { + const has_many_props = state.props.length > 3; + const props_separator = has_many_props ? `\n${indent}${indent}` : ' '; let props = ''; if (analysis.uses_props) { props = `...${state.props_name}`; @@ -74,10 +76,10 @@ export function migrate(source) { } return prop_str; }) - .join(', '); + .join(`,${props_separator}`); if (analysis.uses_rest_props) { - props += `, ...${state.rest_props_name}`; + props += `,${props_separator}...${state.rest_props_name}`; } } @@ -85,6 +87,7 @@ export function migrate(source) { // some render tags or forwarded event attributes to add str.appendRight(state.props_insertion_point, ` ${props},`); } else { + const type_name = state.scope.root.unique('Props').name; let type = ''; if ( parsed.instance?.attributes.some( @@ -92,27 +95,25 @@ export function migrate(source) { ) ) { if (analysis.uses_props || analysis.uses_rest_props) { - type = ' : Record'; + type = `interface ${type_name} { [key: string]: any }`; } else { - type = ` : { ${state.props + type = `interface ${type_name} {${props_separator}${state.props .map((prop) => { return `${prop.exported}${prop.optional ? '?' : ''}: ${prop.type}`; }) - .join(', ')} }`; + .join(`,${props_separator}`)}${has_many_props ? `\n${indent}` : ' '}}`; } } - const props_declaration = `let { ${props} }${type} = $props();`; + let props_declaration = `let {${props_separator}${props}${has_many_props ? `\n${indent}` : ' '}}`; + if (type) { + props_declaration = `${type}\n\n${indent}${props_declaration}`; + } + props_declaration = `${props_declaration}${type ? `: ${type_name}` : ''} = $props();`; + if (parsed.instance) { - if (state.props_insertion_point === 0) { - // no regular props found, but render tags or events to forward found, $props() will be first in the script tag - str.appendRight( - /** @type {number} */ (parsed.instance.content.start), - `\n${indent}${props_declaration}` - ); - } else { - str.appendRight(state.props_insertion_point, props_declaration); - } + props_declaration = `\n${indent}${props_declaration}`; + str.appendRight(state.props_insertion_point, props_declaration); } else { const imports = state.needs_run ? `${indent}${run_import}\n` : ''; str.prepend(`\n\n`); @@ -270,6 +271,8 @@ const instance_script = { start = /** @type {number} */ (parent.start); end = /** @type {number} */ (parent.end); } + while (state.str.original[start] !== '\n') start--; + while (state.str.original[end] !== '\n') end++; state.str.update(start, end, ''); } }, diff --git a/packages/svelte/tests/migrate/samples/props-ts/output.svelte b/packages/svelte/tests/migrate/samples/props-ts/output.svelte index 12d1ced2a079..441d48b82359 100644 --- a/packages/svelte/tests/migrate/samples/props-ts/output.svelte +++ b/packages/svelte/tests/migrate/samples/props-ts/output.svelte @@ -1,8 +1,17 @@ {readonly} diff --git a/packages/svelte/tests/migrate/samples/props/output.svelte b/packages/svelte/tests/migrate/samples/props/output.svelte index d1c327d3ebd5..4c2d0363ae47 100644 --- a/packages/svelte/tests/migrate/samples/props/output.svelte +++ b/packages/svelte/tests/migrate/samples/props/output.svelte @@ -1,8 +1,10 @@ {readonly} From 9a5088b705d3084ecbdee4bcc858610380c3ac53 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 15:49:23 +0200 Subject: [PATCH 13/22] props alias --- packages/svelte/src/compiler/migrate/index.js | 24 +++++++++++++++++++ .../samples/props-export-alias/input.svelte | 6 +++++ .../samples/props-export-alias/output.svelte | 8 +++++++ 3 files changed, 38 insertions(+) create mode 100644 packages/svelte/tests/migrate/samples/props-export-alias/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/props-export-alias/output.svelte diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 5de2bebc6552..5c94738f7f13 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -163,6 +163,27 @@ const instance_script = { Identifier(node, { state }) { handle_identifier(node, state); }, + ExportNamedDeclaration(node, { state, next }) { + if (node.declaration) { + next(); + return; + } + + let count_removed = 0; + for (const specifier of node.specifiers) { + const binding = state.scope.get(specifier.local.name); + if (binding?.kind === 'bindable_prop') { + state.str.remove( + /** @type {number} */ (specifier.start), + /** @type {number} */ (specifier.end) + ); + count_removed++; + } + } + if (count_removed === node.specifiers.length) { + state.str.remove(/** @type {number} */ (node.start), /** @type {number} */ (node.end)); + } + }, VariableDeclaration(node, { state, path }) { if (state.scope !== state.analysis.instance.scope) { return; @@ -192,6 +213,9 @@ const instance_script = { if (declarator.id.type !== 'Identifier') { // TODO + throw new Error( + 'Encountered an export declaration pattern that is not supported for automigration.' + ); // Turn export let into props. It's really really weird because export let { x: foo, z: [bar]} = .. // means that foo and bar are the props (i.e. the leafs are the prop names), not x and z. // const tmp = state.scope.generate('tmp'); diff --git a/packages/svelte/tests/migrate/samples/props-export-alias/input.svelte b/packages/svelte/tests/migrate/samples/props-export-alias/input.svelte new file mode 100644 index 000000000000..1cb57ef5fd26 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/props-export-alias/input.svelte @@ -0,0 +1,6 @@ + + +{klass} diff --git a/packages/svelte/tests/migrate/samples/props-export-alias/output.svelte b/packages/svelte/tests/migrate/samples/props-export-alias/output.svelte new file mode 100644 index 000000000000..76d25afb08bc --- /dev/null +++ b/packages/svelte/tests/migrate/samples/props-export-alias/output.svelte @@ -0,0 +1,8 @@ + + +{klass} \ No newline at end of file From 1d17ad0eeca1ff6366db47017be3bf7104588b97 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 16:33:47 +0200 Subject: [PATCH 14/22] treat store reassignment as a side effect --- packages/svelte/src/compiler/migrate/index.js | 23 +++++++++++-------- .../migrate/samples/effects/input.svelte | 1 + .../migrate/samples/effects/output.svelte | 3 +++ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 5c94738f7f13..ab6ef83fbd22 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -200,7 +200,13 @@ const instance_script = { continue; } - const bindings = state.scope.get_bindings(declarator); + let bindings; + try { + bindings = state.scope.get_bindings(declarator); + } catch (e) { + // no bindings, so we can skip this + continue; + } const has_state = bindings.some((binding) => binding.kind === 'state'); const has_props = bindings.some((binding) => binding.kind === 'bindable_prop'); @@ -212,7 +218,7 @@ const instance_script = { nr_of_props++; if (declarator.id.type !== 'Identifier') { - // TODO + // TODO invest time in this? throw new Error( 'Encountered an export declaration pattern that is not supported for automigration.' ); @@ -238,7 +244,6 @@ const instance_script = { // declarations.push(b.declarator(path.node, value)); // } // } - continue; } const binding = /** @type {import('#compiler').Binding} */ ( @@ -310,8 +315,9 @@ const instance_script = { node.body.expression.type === 'AssignmentExpression' ) { const ids = extract_identifiers(node.body.expression.left); - const reassigned_ids = ids.filter((id) => state.scope.get(id.name)?.reassigned); - if (reassigned_ids.length === 0) { + const bindings = ids.map((id) => state.scope.get(id.name)); + const reassigned_bindings = bindings.filter((b) => b?.reassigned); + if (reassigned_bindings.length === 0 && !bindings.some((b) => b?.kind === 'store_sub')) { // $derived state.str.update( /** @type {number} */ (node.start), @@ -329,13 +335,12 @@ const instance_script = { ); return; } else { - for (const id of reassigned_ids) { - const binding = state.scope.get(id.name); - if (binding?.node === id) { + for (const binding of reassigned_bindings) { + if (binding && ids.includes(binding.node)) { // implicitly-declared variable which we need to make explicit state.str.prependLeft( /** @type {number} */ (node.start), - `let ${id.name}${binding.kind === 'state' ? ' = $state()' : ''};\n${state.indent}` + `let ${binding.node.name}${binding.kind === 'state' ? ' = $state()' : ''};\n${state.indent}` ); } } diff --git a/packages/svelte/tests/migrate/samples/effects/input.svelte b/packages/svelte/tests/migrate/samples/effects/input.svelte index 98002a0d7feb..80704507f041 100644 --- a/packages/svelte/tests/migrate/samples/effects/input.svelte +++ b/packages/svelte/tests/migrate/samples/effects/input.svelte @@ -7,4 +7,5 @@ $: { console.log('foo'); } + $: $count = 1; diff --git a/packages/svelte/tests/migrate/samples/effects/output.svelte b/packages/svelte/tests/migrate/samples/effects/output.svelte index 28ba67a0ad47..cb2c745756ff 100644 --- a/packages/svelte/tests/migrate/samples/effects/output.svelte +++ b/packages/svelte/tests/migrate/samples/effects/output.svelte @@ -13,4 +13,7 @@ run(() => { console.log('foo'); }); + run(() => { + $count = 1; + }); \ No newline at end of file From 3e2cd3609aaf5fd47db460099d530497fe34ab89 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 16:41:33 +0200 Subject: [PATCH 15/22] convert `break $;` to `return;` --- packages/svelte/src/compiler/migrate/index.js | 13 ++++++++++++- .../tests/migrate/samples/effects/input.svelte | 2 ++ .../tests/migrate/samples/effects/output.svelte | 2 ++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index ab6ef83fbd22..6949be129ba7 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -305,11 +305,22 @@ const instance_script = { state.str.update(start, end, ''); } }, - LabeledStatement(node, { path, state }) { + BreakStatement(node, { state, path }) { + if (path[1].type !== 'LabeledStatement') return; + if (node.label?.name !== '$') return; + state.str.update( + /** @type {number} */ (node.start), + /** @type {number} */ (node.end), + 'return;' + ); + }, + LabeledStatement(node, { path, state, next }) { if (state.analysis.runes) return; if (path.length > 1) return; if (node.label.name !== '$') return; + next(); + if ( node.body.type === 'ExpressionStatement' && node.body.expression.type === 'AssignmentExpression' diff --git a/packages/svelte/tests/migrate/samples/effects/input.svelte b/packages/svelte/tests/migrate/samples/effects/input.svelte index 80704507f041..f7f1edfb97b3 100644 --- a/packages/svelte/tests/migrate/samples/effects/input.svelte +++ b/packages/svelte/tests/migrate/samples/effects/input.svelte @@ -6,6 +6,8 @@ } $: { console.log('foo'); + if (x) break $; + console.log('bar'); } $: $count = 1; diff --git a/packages/svelte/tests/migrate/samples/effects/output.svelte b/packages/svelte/tests/migrate/samples/effects/output.svelte index cb2c745756ff..006ce4b23db3 100644 --- a/packages/svelte/tests/migrate/samples/effects/output.svelte +++ b/packages/svelte/tests/migrate/samples/effects/output.svelte @@ -12,6 +12,8 @@ }); run(() => { console.log('foo'); + if (x) return; + console.log('bar'); }); run(() => { $count = 1; From d996519105ef1c9eeb4ab595ea13259b331411c2 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 16:51:01 +0200 Subject: [PATCH 16/22] handle slot name clashes and invalid names --- packages/svelte/src/compiler/migrate/index.js | 2 +- packages/svelte/tests/migrate/samples/slots/input.svelte | 2 ++ packages/svelte/tests/migrate/samples/slots/output.svelte | 8 +++++--- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 6949be129ba7..8337431dcfe7 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -421,7 +421,7 @@ const template = { slot_props += `...${state.str.original.substring(/** @type {number} */ (attr.expression.start), attr.expression.end)}, `; } else if (attr.type === 'Attribute') { if (attr.name === 'name') { - name = /** @type {any} */ (attr.value)[0].data; + name = state.scope.generate(/** @type {any} */ (attr.value)[0].data); } else { const value = attr.value !== true diff --git a/packages/svelte/tests/migrate/samples/slots/input.svelte b/packages/svelte/tests/migrate/samples/slots/input.svelte index c383095f75c3..e71004fcfed6 100644 --- a/packages/svelte/tests/migrate/samples/slots/input.svelte +++ b/packages/svelte/tests/migrate/samples/slots/input.svelte @@ -3,3 +3,5 @@ {#if foo} {/if} + + \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/slots/output.svelte b/packages/svelte/tests/migrate/samples/slots/output.svelte index 79ba8f3a9724..8bdf9a06c4f6 100644 --- a/packages/svelte/tests/migrate/samples/slots/output.svelte +++ b/packages/svelte/tests/migrate/samples/slots/output.svelte @@ -1,9 +1,11 @@ {#if foo} - {@render foo?.({ foo, })} -{/if} \ No newline at end of file + {@render foo_1?.({ foo, })} +{/if} + +{@render dashed_name?.()} \ No newline at end of file From 294a944aa10fd48648c5a0a4dd2a7f48a406c16c Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 17:34:01 +0200 Subject: [PATCH 17/22] handle jsdoc --- packages/svelte/src/compiler/migrate/index.js | 48 ++++++++++++++----- .../samples/event-handlers/output.svelte | 3 +- .../samples/props-rest-props/output.svelte | 1 + .../tests/migrate/samples/props/output.svelte | 1 + .../tests/migrate/samples/slots/output.svelte | 1 + 5 files changed, 42 insertions(+), 12 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 8337431dcfe7..0172dd224b54 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -87,13 +87,12 @@ export function migrate(source) { // some render tags or forwarded event attributes to add str.appendRight(state.props_insertion_point, ` ${props},`); } else { + const uses_ts = parsed.instance?.attributes.some( + (attr) => attr.name === 'lang' && /** @type {any} */ (attr).value[0].data === 'ts' + ); const type_name = state.scope.root.unique('Props').name; let type = ''; - if ( - parsed.instance?.attributes.some( - (attr) => attr.name === 'lang' && /** @type {any} */ (attr).value[0].data === 'ts' - ) - ) { + if (uses_ts) { if (analysis.uses_props || analysis.uses_rest_props) { type = `interface ${type_name} { [key: string]: any }`; } else { @@ -103,13 +102,26 @@ export function migrate(source) { }) .join(`,${props_separator}`)}${has_many_props ? `\n${indent}` : ' '}}`; } + } else { + if (analysis.uses_props || analysis.uses_rest_props) { + type = `{Record}`; + } else { + type = `{${state.props + .map((prop) => { + return `${prop.exported}${prop.optional ? '?' : ''}: ${prop.type}`; + }) + .join(`, `)}}`; + } } let props_declaration = `let {${props_separator}${props}${has_many_props ? `\n${indent}` : ' '}}`; - if (type) { + if (uses_ts) { props_declaration = `${type}\n\n${indent}${props_declaration}`; + props_declaration = `${props_declaration}${type ? `: ${type_name}` : ''} = $props();`; + } else { + props_declaration = `/** @type {${type}} */\n${indent}${props_declaration}`; + props_declaration = `${props_declaration} = $props();`; } - props_declaration = `${props_declaration}${type ? `: ${type_name}` : ''} = $props();`; if (parsed.instance) { props_declaration = `\n${indent}${props_declaration}`; @@ -269,7 +281,7 @@ const instance_script = { ) : '', optional: !!declarator.init, - type: extract_type(declarator, state.str), + type: extract_type(declarator, state.str, path), bindable: binding.mutated || binding.reassigned }); state.props_insertion_point = /** @type {number} */ (declarator.end); @@ -446,7 +458,7 @@ const template = { init: '', bindable: false, optional: true, - type: slot_props ? 'Snippet' : 'Snippet<[any]>' + type: `import('svelte').${slot_props ? 'Snippet<[any]>' : 'Snippet'}` }); if (node.fragment.nodes.length > 0) { @@ -466,8 +478,9 @@ const template = { /** * @param {import('estree').VariableDeclarator} declarator * @param {MagicString} str + * @param {import('#compiler').SvelteNode[]} path */ -function extract_type(declarator, str) { +function extract_type(declarator, str, path) { if (declarator.id.typeAnnotation) { let start = declarator.id.typeAnnotation.start + 1; // skip the colon while (str.original[start] === ' ') { @@ -476,6 +489,19 @@ function extract_type(declarator, str) { return str.original.substring(start, declarator.id.typeAnnotation.end); } + // try to find a comment with a type annotation, hinting at jsdoc + const parent = path.at(-1); + if (parent?.type === 'ExportNamedDeclaration' && parent.leadingComments) { + const last = parent.leadingComments[parent.leadingComments.length - 1]; + if (last.type === 'Block') { + const match = /@type {([^}]+)}/.exec(last.value); + if (match) { + str.update(/** @type {any} */ (last).start, /** @type {any} */ (last).end, ''); + return match[1]; + } + } + } + // try to infer it from the init if (declarator.init?.type === 'Literal') { const type = typeof declarator.init.value; @@ -506,7 +532,7 @@ function handle_events(node, state) { // turn on:click into a prop let exported = `on${name}`; if (!regex_is_valid_identifier.test(name)) { - exported = `'${name}'`; + exported = `'${exported}'`; } // Check if prop already set, could happen when on:click on different elements let local = state.props.find((prop) => prop.exported === exported)?.local; diff --git a/packages/svelte/tests/migrate/samples/event-handlers/output.svelte b/packages/svelte/tests/migrate/samples/event-handlers/output.svelte index 474c48dc34df..d2d9783d0037 100644 --- a/packages/svelte/tests/migrate/samples/event-handlers/output.svelte +++ b/packages/svelte/tests/migrate/samples/event-handlers/output.svelte @@ -1,5 +1,6 @@ + + + + + + diff --git a/packages/svelte/tests/migrate/samples/event-handlers/output.svelte b/packages/svelte/tests/migrate/samples/event-handlers/output.svelte index d2d9783d0037..2333ee92def0 100644 --- a/packages/svelte/tests/migrate/samples/event-handlers/output.svelte +++ b/packages/svelte/tests/migrate/samples/event-handlers/output.svelte @@ -26,4 +26,22 @@ + + + + + + \ No newline at end of file From 3266531967fde34d71311849c5b280003637438a Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 17:59:04 +0200 Subject: [PATCH 19/22] handle empty function --- packages/svelte/src/compiler/migrate/index.js | 4 ++-- .../svelte/tests/migrate/samples/event-handlers/input.svelte | 2 +- .../svelte/tests/migrate/samples/event-handlers/output.svelte | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index d7244b4015f6..1085756b5660 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -647,11 +647,11 @@ function handle_events(node, state) { const needs_curlies = last.expression.body.type !== 'BlockStatement'; state.str.prependRight( - /** @type {number} */ (pos), + /** @type {number} */ (pos) + (needs_curlies ? 0 : 1), `${needs_curlies ? '{' : ''}${prepend}${state.indent}` ); state.str.appendRight( - /** @type {number} */ (last.expression.body.end), + /** @type {number} */ (last.expression.body.end) - (needs_curlies ? 0 : 1), `\n${needs_curlies ? '}' : ''}` ); } else { diff --git a/packages/svelte/tests/migrate/samples/event-handlers/input.svelte b/packages/svelte/tests/migrate/samples/event-handlers/input.svelte index c88538718ca0..a3cbe7288977 100644 --- a/packages/svelte/tests/migrate/samples/event-handlers/input.svelte +++ b/packages/svelte/tests/migrate/samples/event-handlers/input.svelte @@ -9,7 +9,7 @@ - + diff --git a/packages/svelte/tests/migrate/samples/event-handlers/output.svelte b/packages/svelte/tests/migrate/samples/event-handlers/output.svelte index 2333ee92def0..9b4f217af755 100644 --- a/packages/svelte/tests/migrate/samples/event-handlers/output.svelte +++ b/packages/svelte/tests/migrate/samples/event-handlers/output.svelte @@ -32,7 +32,7 @@ }}>click me - - @@ -26,20 +26,20 @@ - - - - From f95c709ced35008640eb7d0516f9a15c9c94505a Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 20:46:49 +0200 Subject: [PATCH 21/22] future-proof --- packages/svelte/src/compiler/migrate/index.js | 6 ++++-- packages/svelte/types/index.d.ts | 6 +++++- sites/svelte-5-preview/src/lib/Repl.svelte | 2 +- sites/svelte-5-preview/src/lib/workers/compiler/index.js | 6 +++--- sites/svelte-5-preview/src/lib/workers/workers.d.ts | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index b4530edd642a..e6328c70baff 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -10,8 +10,10 @@ import { regex_is_valid_identifier } from '../phases/patterns.js'; /** * Does a best-effort migration of Svelte code towards using runes, event attributes and render tags. + * May throw an error if the code is too complex to migrate automatically. + * * @param {string} source - * @returns {string} + * @returns {{ code: string; }} */ export function migrate(source) { try { @@ -145,7 +147,7 @@ export function migrate(source) { } } - return str.toString(); + return { code: str.toString() }; } catch (e) { // eslint-disable-next-line no-console console.error('Error while migrating Svelte code'); diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 72e3a22485c1..e3d07e836a35 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -1088,8 +1088,12 @@ declare module 'svelte/compiler' { export const VERSION: string; /** * Does a best-effort migration of Svelte code towards using runes, event attributes and render tags. + * May throw an error if the code is too complex to migrate automatically. + * * */ - export function migrate(source: string): string; + export function migrate(source: string): { + code: string; + }; class Scope { constructor(root: ScopeRoot, parent: Scope | null, porous: boolean); diff --git a/sites/svelte-5-preview/src/lib/Repl.svelte b/sites/svelte-5-preview/src/lib/Repl.svelte index 40f03d1730e1..80a2b7ff6110 100644 --- a/sites/svelte-5-preview/src/lib/Repl.svelte +++ b/sites/svelte-5-preview/src/lib/Repl.svelte @@ -170,7 +170,7 @@ if (file.name === $selected?.name) { return { ...file, - source: result.source + source: result.result.code }; } return file; diff --git a/sites/svelte-5-preview/src/lib/workers/compiler/index.js b/sites/svelte-5-preview/src/lib/workers/compiler/index.js index 8f691f0cecad..3c8fc1909cca 100644 --- a/sites/svelte-5-preview/src/lib/workers/compiler/index.js +++ b/sites/svelte-5-preview/src/lib/workers/compiler/index.js @@ -136,11 +136,11 @@ function compile({ id, source, options, return_ast }) { /** @param {import("../workers").MigrateMessageData} param0 */ function migrate({ id, source }) { try { - source = svelte.migrate(source); + const result = svelte.migrate(source); return { id, - source + result }; } catch (err) { // @ts-ignore @@ -148,7 +148,7 @@ function migrate({ id, source }) { return { id, - source, + result: { code: source }, error: message }; } diff --git a/sites/svelte-5-preview/src/lib/workers/workers.d.ts b/sites/svelte-5-preview/src/lib/workers/workers.d.ts index 0903194839bc..2c0015cf3288 100644 --- a/sites/svelte-5-preview/src/lib/workers/workers.d.ts +++ b/sites/svelte-5-preview/src/lib/workers/workers.d.ts @@ -29,6 +29,6 @@ export type BundleMessageData = { export type MigrateMessageData = { id: number; - source: string; + result: { code: string }; error?: string; }; From 9dc124b5a1482717aba952b5c4a0e43305c3aab1 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Apr 2024 20:50:13 +0200 Subject: [PATCH 22/22] ug --- packages/svelte/tests/migrate/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/tests/migrate/test.ts b/packages/svelte/tests/migrate/test.ts index 71d774328f0f..a3c247245cf2 100644 --- a/packages/svelte/tests/migrate/test.ts +++ b/packages/svelte/tests/migrate/test.ts @@ -12,7 +12,7 @@ const { test, run } = suite(async (config, cwd) => { .replace(/\s+$/, '') .replace(/\r/g, ''); - const actual = migrate(input); + const actual = migrate(input).code; // run `UPDATE_SNAPSHOTS=true pnpm test migrate` to update parser tests if (process.env.UPDATE_SNAPSHOTS || !fs.existsSync(`${cwd}/output.svelte`)) {