fix: robustify migration script (#12019)

* handle multiple slots of same kind correctly

* put props below imports

* migrate $$slots, better error position for slot-children-conflict error

* handle event braces+modifier case

* migrate jsdoc type with curly braces correctly

* handle comments above types, fixes #11508

* changeset
pull/12024/head
Simon H 6 months ago committed by GitHub
parent 4cdc3710d0
commit bf23f692fe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
"svelte": patch
---
fix: robustify migration script

@ -63,7 +63,8 @@ export function migrate(source) {
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}` : ' ';
const newline_separator = `\n${indent}${indent}`;
const props_separator = has_many_props ? newline_separator : ' ';
let props = '';
if (analysis.uses_props) {
props = `...${state.props_name}`;
@ -99,11 +100,12 @@ export function migrate(source) {
if (analysis.uses_props || analysis.uses_rest_props) {
type = `interface ${type_name} { [key: string]: any }`;
} else {
type = `interface ${type_name} {${props_separator}${state.props
type = `interface ${type_name} {${newline_separator}${state.props
.map((prop) => {
return `${prop.exported}${prop.optional ? '?' : ''}: ${prop.type}`;
const comment = prop.comment ? `${prop.comment}${newline_separator}` : '';
return `${comment}${prop.exported}${prop.optional ? '?' : ''}: ${prop.type};`;
})
.join(`,${props_separator}`)}${has_many_props ? `\n${indent}` : ' '}}`;
.join(newline_separator)}\n${indent}}`;
}
} else {
if (analysis.uses_props || analysis.uses_rest_props) {
@ -162,7 +164,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; optional: boolean; type: string }>;
* props: Array<{ local: string; exported: string; init: string; bindable: boolean; slot_name?: string; optional: boolean; type: string; comment?: string }>;
* props_insertion_point: number;
* has_props_rune: boolean;
* props_name: string;
@ -190,8 +192,11 @@ const instance_script = {
}
next();
},
Identifier(node, { state }) {
handle_identifier(node, state);
Identifier(node, { state, path }) {
handle_identifier(node, state, path);
},
ImportDeclaration(node, { state }) {
state.props_insertion_point = node.end ?? state.props_insertion_point;
},
ExportNamedDeclaration(node, { state, next }) {
if (node.declaration) {
@ -299,8 +304,8 @@ const instance_script = {
)
: '',
optional: !!declarator.init,
type: extract_type(declarator, state.str, path),
bindable: binding.mutated || binding.reassigned
bindable: binding.mutated || binding.reassigned,
...extract_type_and_comment(declarator, state.str, path)
});
state.props_insertion_point = /** @type {number} */ (declarator.end);
state.str.update(
@ -423,8 +428,8 @@ const instance_script = {
/** @type {import('zimmerframe').Visitors<import('../types/template.js').SvelteNode, State>} */
const template = {
Identifier(node, { state }) {
handle_identifier(node, state);
Identifier(node, { state, path }) {
handle_identifier(node, state, path);
},
RegularElement(node, { state, next }) {
handle_events(node, state);
@ -468,6 +473,7 @@ const template = {
},
SlotElement(node, { state, next }) {
let name = 'children';
let slot_name = 'default';
let slot_props = '{ ';
for (const attr of node.attributes) {
@ -475,7 +481,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 = state.scope.generate(/** @type {any} */ (attr.value)[0].data);
slot_name = /** @type {any} */ (attr.value)[0].data;
} else {
const value =
attr.value !== true
@ -494,14 +500,24 @@ const template = {
slot_props = '';
}
state.props.push({
local: name,
exported: name,
init: '',
bindable: false,
optional: true,
type: `import('svelte').${slot_props ? 'Snippet<[any]>' : 'Snippet'}`
});
const existing_prop = state.props.find((prop) => prop.slot_name === slot_name);
if (existing_prop) {
name = existing_prop.local;
} else if (slot_name !== 'default') {
name = state.scope.generate(slot_name);
}
if (!existing_prop) {
state.props.push({
local: name,
exported: name,
init: '',
bindable: false,
optional: true,
slot_name,
type: `import('svelte').${slot_props ? 'Snippet<[any]>' : 'Snippet'}`
});
}
if (node.fragment.nodes.length > 0) {
next();
@ -528,25 +544,34 @@ const template = {
* @param {MagicString} str
* @param {import('#compiler').SvelteNode[]} path
*/
function extract_type(declarator, str, path) {
function extract_type_and_comment(declarator, str, path) {
const parent = path.at(-1);
// Try to find jsdoc above the declaration
let comment_node = /** @type {import('estree').Node} */ (parent)?.leadingComments?.at(-1);
if (comment_node?.type !== 'Block') comment_node = undefined;
const comment_start = /** @type {any} */ (comment_node)?.start;
const comment_end = /** @type {any} */ (comment_node)?.end;
const comment = comment_node && str.original.substring(comment_start, comment_end);
if (comment_node) {
str.update(comment_start, comment_end, '');
}
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);
return { type: str.original.substring(start, declarator.id.typeAnnotation.end), comment };
}
// 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];
}
if (parent?.type === 'ExportNamedDeclaration' && comment_node) {
const match = /@type {(.+)}/.exec(comment_node.value);
if (match) {
return { type: match[1] };
}
}
@ -554,11 +579,11 @@ function extract_type(declarator, str, path) {
if (declarator.init?.type === 'Literal') {
const type = typeof declarator.init.value;
if (type === 'string' || type === 'number' || type === 'boolean') {
return type;
return { type, comment };
}
}
return 'any';
return { type: 'any', comment };
}
/**
@ -694,14 +719,16 @@ function handle_events(node, state) {
}
const needs_curlies = last.expression.body.type !== 'BlockStatement';
state.str.prependRight(
/** @type {number} */ (pos) + (needs_curlies ? 0 : 1),
`${needs_curlies ? '{' : ''}${prepend}${state.indent}`
);
state.str.appendRight(
/** @type {number} */ (last.expression.body.end) - (needs_curlies ? 0 : 1),
`\n${needs_curlies ? '}' : ''}`
);
const end = /** @type {number} */ (last.expression.body.end) - (needs_curlies ? 0 : 1);
pos = /** @type {number} */ (pos) + (needs_curlies ? 0 : 1);
if (needs_curlies && state.str.original[pos - 1] === '(') {
// Prettier does something like on:click={() => (foo = true)}, we need to remove the braces in this case
state.str.update(pos - 1, pos, `{${prepend}${state.indent}`);
state.str.update(end, end + 1, '\n}');
} else {
state.str.prependRight(pos, `${needs_curlies ? '{' : ''}${prepend}${state.indent}`);
state.str.appendRight(end, `\n${needs_curlies ? '}' : ''}`);
}
} else {
state.str.update(
/** @type {number} */ (last.expression.start),
@ -763,8 +790,12 @@ function generate_event_name(last, state) {
/**
* @param {import('estree').Identifier} node
* @param {State} state
* @param {any[]} path
*/
function handle_identifier(node, state) {
function handle_identifier(node, state, path) {
const parent = path.at(-1);
if (parent?.type === 'MemberExpression' && parent.property === node) return;
if (state.analysis.uses_props) {
if (node.name === '$$props' || node.name === '$$restProps') {
// not 100% correct for $$restProps but it'll do
@ -785,6 +816,11 @@ function handle_identifier(node, state) {
/** @type {number} */ (node.end),
state.rest_props_name
);
} else if (node.name === '$$slots' && state.analysis.uses_slots) {
if (parent?.type === 'MemberExpression') {
state.str.update(/** @type {number} */ (node.start), parent.property.start, '');
}
// else passed as identifier, we don't know what to do here, so let it error
}
}

@ -554,7 +554,8 @@ export function analyze_component(root, source, options) {
}
if (analysis.uses_render_tags && (analysis.uses_slots || analysis.slot_names.size > 0)) {
e.slot_snippet_conflict(analysis.slot_names.values().next().value);
const pos = analysis.slot_names.values().next().value ?? analysis.source.indexOf('$$slot');
e.slot_snippet_conflict(pos);
}
if (analysis.css.ast) {

@ -1,5 +1,7 @@
<button on:click={() => console.log('hi')} on:click>click me</button>
<button on:click={() => console.log('before')} on:click on:click={() => console.log('after')}>click me</button>
<button on:click={() => console.log('before')} on:click on:click={() => console.log('after')}
>click me</button
>
<button on:click on:click={foo}>click me</button>
<button on:click>click me</button>
@ -9,6 +11,7 @@
<button on:custom-event-bubble>click me</button>
<button on:click|preventDefault={() => ''}>click me</button>
<button on:click|preventDefault={() => (searching = true)}>click me</button>
<button on:click|stopPropagation={() => {}}>click me</button>
<button on:click|stopImmediatePropagation={() => ''}>click me</button>
<button on:click|capture={() => ''}>click me</button>

@ -13,7 +13,9 @@
onclick?.(event);
console.log('after')
}}>click me</button>
}}
>click me</button
>
<button onclick={(event) => {
onclick?.(event);
@ -30,6 +32,10 @@
event.preventDefault();
''
}}>click me</button>
<button onclick={(event) => {
event.preventDefault();
searching = true
}}>click me</button>
<button onclick={(event) => {
event.stopPropagation();

@ -1,5 +1,7 @@
<script lang="ts">
interface Props { class?: string }
interface Props {
class?: string;
}
let { class: klass = '' }: Props = $props();

@ -1,8 +1,9 @@
<script lang="ts">
export let readonly: number;
export let optional = 'foo';
export let binding: string;
export let bindingOptional: string | undefined = 'bar';
/** some comment */
export let readonly: number;
export let optional = 'foo';
export let binding: string;
export let bindingOptional: string | undefined = 'bar';
</script>
{readonly}

@ -1,17 +1,19 @@
<script lang="ts">
interface Props {
readonly: number,
optional?: string,
binding: string,
bindingOptional?: string | undefined
}
interface Props {
/** some comment */
readonly: number;
optional?: string;
binding: string;
bindingOptional?: string | undefined;
}
let {
readonly,
optional = 'foo',
binding = $bindable(),
bindingOptional = $bindable('bar')
}: Props = $props();
let {
readonly,
optional = 'foo',
binding = $bindable(),
bindingOptional = $bindable('bar')
}: Props = $props();
</script>
{readonly}

@ -1,8 +1,9 @@
<script>
export let readonly;
export let optional = 'foo';
export let binding;
export let bindingOptional = 'bar';
/** @type {Record<string, { href: string; title: string; }[]>} */
export let readonly;
export let optional = 'foo';
export let binding;
export let bindingOptional = 'bar';
</script>
{readonly}

@ -1,11 +1,12 @@
<script>
/** @type {{readonly: any, optional?: string, binding: any, bindingOptional?: string}} */
let {
readonly,
optional = 'foo',
binding = $bindable(),
bindingOptional = $bindable('bar')
} = $props();
/** @type {{readonly: Record<string, { href: string; title: string; }[]>, optional?: string, binding: any, bindingOptional?: string}} */
let {
readonly,
optional = 'foo',
binding = $bindable(),
bindingOptional = $bindable('bar')
} = $props();
</script>
{readonly}

@ -0,0 +1,6 @@
<script>
import Foo from './Foo.svelte';
</script>
<slot />
<Foo />

@ -0,0 +1,8 @@
<script>
import Foo from './Foo.svelte';
/** @type {{children?: import('svelte').Snippet}} */
let { children } = $props();
</script>
{@render children?.()}
<Foo />

@ -0,0 +1,5 @@
<button><slot /></button>
<button><slot /></button>
<slot name="dashed-name" />
<slot name="dashed-name" />

@ -0,0 +1,10 @@
<script>
/** @type {{children?: import('svelte').Snippet, dashed_name?: import('svelte').Snippet}} */
let { children, dashed_name } = $props();
</script>
<button>{@render children?.()}</button>
<button>{@render children?.()}</button>
{@render dashed_name?.()}
{@render dashed_name?.()}

@ -4,4 +4,9 @@
<slot name="foo" {foo} />
{/if}
<slot name="dashed-name" />
{#if $$slots.bar}
{$$slots}
<slot name="bar" />
{/if}
<slot name="dashed-name" />

@ -1,6 +1,11 @@
<script>
/** @type {{children?: import('svelte').Snippet, foo_1?: import('svelte').Snippet<[any]>, dashed_name?: import('svelte').Snippet}} */
let { children, foo_1, dashed_name } = $props();
/** @type {{children?: import('svelte').Snippet, foo_1?: import('svelte').Snippet<[any]>, bar?: import('svelte').Snippet, dashed_name?: import('svelte').Snippet}} */
let {
children,
foo_1,
bar,
dashed_name
} = $props();
</script>
<button>{@render children?.()}</button>
@ -9,4 +14,9 @@
{@render foo_1?.({ foo, })}
{/if}
{#if bar}
{$$slots}
{@render bar?.()}
{/if}
{@render dashed_name?.()}
Loading…
Cancel
Save