Skip to content
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

Migrate script removes JSDoc comments when there are default values #15530

Closed
rgieseke opened this issue Mar 17, 2025 · 5 comments
Closed

Migrate script removes JSDoc comments when there are default values #15530

rgieseke opened this issue Mar 17, 2025 · 5 comments

Comments

@rgieseke
Copy link
Contributor

rgieseke commented Mar 17, 2025

Describe the bug

When running npx sv migrate svelte-5 JSDoc comments of the following form in a Svelte 4 app

<script>
	/** @type {number} [default_value=1] */
	export let default_value = 1;

	/** @type {number} [comment_default_value=1] - This has a comment and an optional value. */
	export let comment_default_value = 1;
</script>

get converted to the following, the comments and default value are not included after the conversion.

<script>
	/**
	 * @typedef {Object} Props
	 * @property {number} [default_value]
	 * @property {number} [comment_default_value]
	 */

	/** @type {Props} */
	let { default_value = 1, comment_default_value = 1 } = $props();
</script>

From running npx vitest packages/svelte/tests/migrate -t="jsdoc-with-comments" and looking around in the migrate script it seems that cleaned_comment is empty here which I think it shouldn't?

let cleaned_comment = cleaned_comment_arr

Happy to help fix this but would need some further pointers how.

I tried updating Layer Cake components to Svelte 5 syntax.

Reproduction

I have failing test cases here: rgieseke@634ccc6

System Info

Node: 22.13.1 - ~/.local/bin/node
    npm: 10.9.2 - ~/.local/bin/npm
    pnpm: 9.4.0 - ~/.local/bin/pnpm
npmPackages:
    svelte: workspace:^ => 5.23.1

Severity

annoyance

@paoloricciuti
Copy link
Member

Technically speaking the [comment_default_value = 1] is not a thing for the @type JSDoc comment so that will also be interpreted as a comment. We could fix it by also appending the rest of the sentence after the @type but this means your code will be converted to

<script>
	

	
	/**
	 * @typedef {Object} Props
	 * @property {number} [default_value] -  [default_value=1] 
	 * @property {number} [comment_default_value] -  [comment_default_value=1] - This has a comment and an optional value. 
	 */

	/** @type {Props} */
	let { default_value = 1, comment_default_value = 1 } = $props();
</script>

I guess there's no harm in fixing it if people use this still of comment for @type

@rgieseke
Copy link
Contributor Author

Personally, I wouldn't mind not having the duplicated default value declaration, but I guess it's helpful to have it in the JSDoc declaration for API docs and editor hints.

On https://jsdoc.app/tags-type the definition with an equal sign is included:

An optional parameter named foo.

@param {number} [foo]
// or:
@param {number=} foo

An optional parameter foo with default value 1.

@param {number} [foo=1]

@paoloricciuti
Copy link
Member

But that's for @param not for @type

@rgieseke
Copy link
Contributor Author

True, but it's used as component property/parameter, so that's probably why it's used that way. I don't know if the usage is common.

@rgieseke
Copy link
Contributor Author

Fixed in #15567 and released as 5.24.0. (The repeated default value declaration in the JSDoc comment is removed though.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants