fix: ensure migrate keeps inline/trailing comments in $props type def…

…inition and produces non-broken code (#14143) * feat[sv migrate]: keep inline/trailing comments when migrating export let types to type definition * test: add tests for inline comment migration * chore: add changeset * fix: migrate trailing multiline comment parsing no longer results in broken code, FIXES PR #14143#issuecomment-2455702689 * test: add migrate test with same-line trailing multiline comments and same-line leading multiline comments * chore: add changeset * fix: lint --------- Co-authored-by: Gonzalo Ruiz <[email protected]> Co-authored-by: paoloricciuti <[email protected]>
sveltejs · Nov 4, 2024 · e6dd871 · e6dd871
1 parent 7dbe812
commit e6dd871
Show file tree

Hide file tree

Showing 7 changed files with 134 additions and 11 deletions.
diff --git a/.changeset/mean-seas-give.md b/.changeset/mean-seas-give.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure trailing multiline comments on props produce correct code (#14143#issuecomment-2455702689)
diff --git a/.changeset/purple-owls-hug.md b/.changeset/purple-owls-hug.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure migrate keeps inline/trailing comments in $props type definition
diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js
@@ -279,7 +279,7 @@ export function migrate(source, { filename, use_ts } = {}) {
 						type = `interface ${type_name} {${newline_separator}${state.props
 							.map((prop) => {
 								const comment = prop.comment ? `${prop.comment}${newline_separator}` : '';
-								return `${comment}${prop.exported}${prop.optional ? '?' : ''}: ${prop.type};`;
+								return `${comment}${prop.exported}${prop.optional ? '?' : ''}: ${prop.type};${prop.trailing_comment ? ' ' + prop.trailing_comment : ''}`;
 							})
 							.join(newline_separator)}`;
 						if (analysis.uses_props || analysis.uses_rest_props) {
@@ -289,7 +289,7 @@ export function migrate(source, { filename, use_ts } = {}) {
 					} else {
 						type = `/**\n${indent} * @typedef {Object} ${type_name}${state.props
 							.map((prop) => {
-								return `\n${indent} * @property {${prop.type}} ${prop.optional ? `[${prop.exported}]` : prop.exported}${prop.comment ? ` - ${prop.comment}` : ''}`;
+								return `\n${indent} * @property {${prop.type}} ${prop.optional ? `[${prop.exported}]` : prop.exported}${prop.comment ? ` - ${prop.comment}` : ''}${prop.trailing_comment ? ` - ${prop.trailing_comment.trim()}` : ''}`;
 							})
 							.join(``)}\n${indent} */`;
 					}
@@ -414,7 +414,7 @@ export function migrate(source, { filename, use_ts } = {}) {
  *  analysis: ComponentAnalysis;
  *  filename?: string;
  *  indent: string;
- *  props: Array<{ local: string; exported: string; init: string; bindable: boolean; slot_name?: string; optional: boolean; type: string; comment?: string; type_only?: boolean; needs_refine_type?: boolean; }>;
+ *  props: Array<{ local: string; exported: string; init: string; bindable: boolean; slot_name?: string; optional: boolean; type: string; comment?: string; trailing_comment?: string; type_only?: boolean; needs_refine_type?: boolean; }>;
  *  props_insertion_point: number;
  *  has_props_rune: boolean;
  *  has_type_or_fallback: boolean;
@@ -1497,13 +1497,28 @@ function extract_type_and_comment(declarator, state, path) {
 		str.update(comment_start, comment_end, '');
 	}
 
+	// Find trailing comments
+	const trailing_comment_node = /** @type {Node} */ (parent)?.trailingComments?.at(0);
+	const trailing_comment_start = /** @type {any} */ (trailing_comment_node)?.start;
+	const trailing_comment_end = /** @type {any} */ (trailing_comment_node)?.end;
+	let trailing_comment =
+		trailing_comment_node && str.original.substring(trailing_comment_start, trailing_comment_end);
+
+	if (trailing_comment_node) {
+		str.update(trailing_comment_start, trailing_comment_end, '');
+	}
+
 	if (declarator.id.typeAnnotation) {
 		state.has_type_or_fallback = true;
 		let start = declarator.id.typeAnnotation.start + 1; // skip the colon
 		while (str.original[start] === ' ') {
 			start++;
 		}
-		return { type: str.original.substring(start, declarator.id.typeAnnotation.end), comment };
+		return {
+			type: str.original.substring(start, declarator.id.typeAnnotation.end),
+			comment,
+			trailing_comment
+		};
 	}
 
 	let cleaned_comment_arr = comment
@@ -1526,12 +1541,43 @@ function extract_type_and_comment(declarator, state, path) {
 		?.slice(0, first_at_comment !== -1 ? first_at_comment : cleaned_comment_arr.length)
 		.join('\n');
 
+	let cleaned_comment_arr_trailing = trailing_comment
+		?.split('\n')
+		.map((line) =>
+			line
+				.trim()
+				// replace `// ` for one liners
+				.replace(/^\/\/\s*/g, '')
+				// replace `\**` for the initial JSDoc
+				.replace(/^\/\*\*?\s*/g, '')
+				// migrate `*/` for the end of JSDoc
+				.replace(/\s*\*\/$/g, '')
+				// remove any initial `* ` to clean the comment
+				.replace(/^\*\s*/g, '')
+		)
+		.filter(Boolean);
+	const first_at_comment_trailing = cleaned_comment_arr_trailing?.findIndex((line) =>
+		line.startsWith('@')
+	);
+	let cleaned_comment_trailing = cleaned_comment_arr_trailing
+		?.slice(
+			0,
+			first_at_comment_trailing !== -1
+				? first_at_comment_trailing
+				: cleaned_comment_arr_trailing.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: cleaned_comment };
+			return {
+				type: match[1],
+				comment: cleaned_comment,
+				trailing_comment: cleaned_comment_trailing
+			};
 		}
 	}
 
@@ -1540,11 +1586,19 @@ 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: state.uses_ts ? comment : cleaned_comment };
+			return {
+				type,
+				comment: state.uses_ts ? comment : cleaned_comment,
+				trailing_comment: state.uses_ts ? trailing_comment : cleaned_comment_trailing
+			};
 		}
 	}
 
-	return { type: 'any', comment: state.uses_ts ? comment : cleaned_comment };
+	return {
+		type: 'any',
+		comment: state.uses_ts ? comment : cleaned_comment,
+		trailing_comment: state.uses_ts ? trailing_comment : cleaned_comment_trailing
+	};
 }
 
 // Ensure modifiers are applied in the same order as Svelte 4
@@ -1779,10 +1833,13 @@ function handle_identifier(node, state, path) {
 						comment = state.str.original.substring(comment_node.start, comment_node.end);
 					}
 
+					const trailing_comment = member.trailingComments?.at(0)?.value;
+
 					if (prop) {
 						prop.type = type;
 						prop.optional = member.optional;
 						prop.comment = comment ?? prop.comment;
+						prop.trailing_comment = trailing_comment ?? prop.trailing_comment;
 					} else {
 						state.props.push({
 							local: member.key.name,
@@ -1792,6 +1849,7 @@ function handle_identifier(node, state, path) {
 							optional: member.optional,
 							type,
 							comment,
+							trailing_comment,
 							type_only: true
 						});
 					}

diff --git a/packages/svelte/tests/migrate/samples/jsdoc-with-comments/input.svelte b/packages/svelte/tests/migrate/samples/jsdoc-with-comments/input.svelte
@@ -24,5 +24,20 @@
 	/**
 	 * This is optional
 	 */
-	 export let optional = {stuff: true};
+	export let optional = {stuff: true};
+	
+	export let inline_commented; // this should stay a comment
+
+	/**
+	 * This comment should be merged
+	 */
+	export let inline_commented_merged; // with this inline comment
+
+	/*
+	* this is a same-line leading multiline comment
+	**/ export let inline_multiline_leading_comment = 'world';
+
+	export let inline_multiline_trailing_comment = 'world'; /*
+	* this is a same-line trailing multiline comment
+	**/
 </script>
diff --git a/packages/svelte/tests/migrate/samples/jsdoc-with-comments/output.svelte b/packages/svelte/tests/migrate/samples/jsdoc-with-comments/output.svelte
@@ -9,6 +9,12 @@
 	
 
 	
+	
+
+	
+
+	
+
 	/**
 	 * @typedef {Object} Props
 	 * @property {string} comment - My wonderful comment
@@ -17,6 +23,10 @@
 	 * @property {any} no_comment
 	 * @property {boolean} type_no_comment
 	 * @property {any} [optional] - This is optional
+	 * @property {any} inline_commented - this should stay a comment
+	 * @property {any} inline_commented_merged - This comment should be merged - with this inline comment
+	 * @property {string} [inline_multiline_leading_comment] - this is a same-line leading multiline comment
+	 * @property {string} [inline_multiline_trailing_comment] - this is a same-line trailing multiline comment
 	 */
 
 	/** @type {Props} */
@@ -26,6 +36,10 @@
 		one_line,
 		no_comment,
 		type_no_comment,
-		optional = {stuff: true}
+		optional = {stuff: true},
+		inline_commented,
+		inline_commented_merged,
+		inline_multiline_leading_comment = 'world',
+		inline_multiline_trailing_comment = 'world'
 	} = $props();
 </script>
diff --git a/packages/svelte/tests/migrate/samples/props-ts/input.svelte b/packages/svelte/tests/migrate/samples/props-ts/input.svelte
@@ -6,6 +6,16 @@
 	export let bindingOptional: string | undefined = 'bar';
 	/** this should stay a comment */
 	export let no_type_but_comment = 0;
+	export let type_and_inline_comment:number; // this should also stay a comment
+	export let no_type_and_inline_comment = 0;         // this should stay as well
+
+	/*
+	* this is a same-line leading multiline comment
+	**/ export let inline_multiline_leading_comment = 'world';
+
+	export let inline_multiline_trailing_comment = 'world'; /*
+	* this is a same-line trailing multiline comment
+	**/
 </script>
 
 {readonly}

diff --git a/packages/svelte/tests/migrate/samples/props-ts/output.svelte b/packages/svelte/tests/migrate/samples/props-ts/output.svelte
@@ -1,6 +1,9 @@
 <script lang="ts">
 	
 	
+
+	
+
 	interface Props {
 		/** some comment */
 		readonly: number;
@@ -9,18 +12,31 @@
 		bindingOptional?: string | undefined;
 		/** this should stay a comment */
 		no_type_but_comment?: number;
+		type_and_inline_comment: number; // this should also stay a comment
+		no_type_and_inline_comment?: number; // this should stay as well
+		/*
+	* this is a same-line leading multiline comment
+	**/
+		inline_multiline_leading_comment?: string;
+		inline_multiline_trailing_comment?: string; /*
+	* this is a same-line trailing multiline comment
+	**/
 	}
 
 	let {
 		readonly,
 		optional = 'foo',
 		binding = $bindable(),
 		bindingOptional = $bindable('bar'),
-		no_type_but_comment = 0
+		no_type_but_comment = 0,
+		type_and_inline_comment,
+		no_type_and_inline_comment = 0,
+		inline_multiline_leading_comment = 'world',
+		inline_multiline_trailing_comment = 'world'
 	}: Props = $props();
 </script>
 
 {readonly}
 {optional}
 <input bind:value={binding} />
-<input bind:value={bindingOptional} />
+<input bind:value={bindingOptional} />