Skip to content

fix: properly migrate ts with inferred type comments #13761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/chatty-feet-unite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: properly migrate ts with inferred type comments
27 changes: 14 additions & 13 deletions packages/svelte/src/compiler/migrate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ export function migrate(source, { filename } = {}) {
script_insertions: new Set(),
derived_components: new Map(),
derived_labeled_statements: new Set(),
has_svelte_self: false
has_svelte_self: false,
uses_ts: !!parsed.instance?.attributes.some(
(attr) => attr.name === 'lang' && /** @type {any} */ (attr).value[0].data === 'ts'
)
};

if (parsed.module) {
Expand Down Expand Up @@ -207,15 +210,12 @@ export function migrate(source, { filename } = {}) {
// some render tags or forwarded event attributes to add
str.appendRight(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 = '';

// Try to infer when we don't want to add types (e.g. user doesn't use types, or this is a zero-types +page.svelte)
if (state.has_type_or_fallback || state.props.every((prop) => prop.slot_name)) {
if (uses_ts) {
if (state.uses_ts) {
type = `interface ${type_name} {${newline_separator}${state.props
.map((prop) => {
const comment = prop.comment ? `${prop.comment}${newline_separator}` : '';
Expand All @@ -236,7 +236,7 @@ export function migrate(source, { filename } = {}) {
}

let props_declaration = `let {${props_separator}${props}${has_many_props ? `\n${indent}` : ' '}}`;
if (uses_ts) {
if (state.uses_ts) {
if (type) {
props_declaration = `${type}\n\n${indent}${props_declaration}`;
}
Expand Down Expand Up @@ -355,6 +355,7 @@ export function migrate(source, { filename } = {}) {
* derived_components: Map<string, string>;
* derived_labeled_statements: Set<LabeledStatement>;
* has_svelte_self: boolean;
* uses_ts: boolean;
* }} State
*/

Expand Down Expand Up @@ -1319,7 +1320,7 @@ function extract_type_and_comment(declarator, state, path) {
return { type: str.original.substring(start, declarator.id.typeAnnotation.end), comment };
}

let cleaned_comment = comment
let cleaned_comment_arr = comment
?.split('\n')
.map((line) =>
line
Expand All @@ -1334,17 +1335,17 @@ function extract_type_and_comment(declarator, state, path) {
.replace(/^\*\s*/g, '')
)
.filter(Boolean);
const first_at_comment = cleaned_comment?.findIndex((line) => line.startsWith('@'));
comment = cleaned_comment
?.slice(0, first_at_comment !== -1 ? first_at_comment : cleaned_comment.length)
const first_at_comment = cleaned_comment_arr?.findIndex((line) => line.startsWith('@'));
let cleaned_comment = cleaned_comment_arr
?.slice(0, first_at_comment !== -1 ? first_at_comment : cleaned_comment_arr.length)
.join('\n');

// try to find a comment with a type annotation, hinting at jsdoc
if (parent?.type === 'ExportNamedDeclaration' && comment_node) {
state.has_type_or_fallback = true;
const match = /@type {(.+)}/.exec(comment_node.value);
if (match) {
return { type: match[1], comment };
return { type: match[1], comment: cleaned_comment };
}
}

Expand All @@ -1353,11 +1354,11 @@ function extract_type_and_comment(declarator, state, path) {
state.has_type_or_fallback = true; // only assume type if it's trivial to infer - else someone would've added a type annotation
const type = typeof declarator.init.value;
if (type === 'string' || type === 'number' || type === 'boolean') {
return { type, comment };
return { type, comment: state.uses_ts ? comment : cleaned_comment };
}
}

return { type: 'any', comment };
return { type: 'any', comment: state.uses_ts ? comment : cleaned_comment };
}

// Ensure modifiers are applied in the same order as Svelte 4
Expand Down
2 changes: 2 additions & 0 deletions packages/svelte/tests/migrate/samples/props-ts/input.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
export let optional = 'foo';
export let binding: string;
export let bindingOptional: string | undefined = 'bar';
/** this should stay a comment */
export let no_type_but_comment = 0;
</script>

{readonly}
Expand Down
6 changes: 5 additions & 1 deletion packages/svelte/tests/migrate/samples/props-ts/output.svelte
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
<script lang="ts">


interface Props {
/** some comment */
readonly: number;
optional?: string;
binding: string;
bindingOptional?: string | undefined;
/** this should stay a comment */
no_type_but_comment?: number;
}

let {
readonly,
optional = 'foo',
binding = $bindable(),
bindingOptional = $bindable('bar')
bindingOptional = $bindable('bar'),
no_type_but_comment = 0
}: Props = $props();
</script>

Expand Down