Skip to content

fix: Keep inlined JSDoc comments in property conversion of svelte-migrate #15567

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

Conversation

rgieseke
Copy link
Contributor

This addresses trailing inlined JSDoc property descriptions being deleted by svelte-migrate, see #15530.

Previously:

/** @type {number} [comment_default_value=1] - This has a comment and a default value. */
export let comment_default_value = 1;```

The comment would be removed in the migrated version:

/**
* @typedef {Object} Props
* @property {number} [comment_default_value].
*/

/** @type {Props} */
let { comment_default_value = 1 } = $props();

With the change in this PR it becomes the following:

/**
* @typedef {Object} Props
* @property {number} [comment_default_value] - This has a comment and a default value.
*/

/** @type {Props} */
let { comment_default_value = 1 } = $props();

The default value description in the JSDoc is still removed, but that's probably ok, as the default value is still in the declaration below.

Happy to change this if you can guide me to a different solution. The regex used here could also potentially be improved to handle more cases of (multiple) whitespace, but wanted to start a discussion first.

The style of JSDoc comments is used in Layer Cake. The components are vendored in other projects so there are lots of repos out there who use this kind of comment (e.g. GitHub search for AxisX.svelte)

Copy link

changeset-bot bot commented Mar 21, 2025

🦋 Changeset detected

Latest commit: 8cac71e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@svelte-docs-bot
Copy link

Copy link
Member

@Rich-Harris Rich-Harris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

@Rich-Harris Rich-Harris merged commit 1a5fb8f into sveltejs:main Mar 21, 2025
@github-actions github-actions bot mentioned this pull request Mar 20, 2025
@rgieseke rgieseke deleted the migrate-add-failing-jsdoc-test-cases branch March 21, 2025 14:17
@patriciatiffany
Copy link

Thanks for doing this!! This fix was what I'd been looking out for. Since I've been migrating to Svelte 5 one component at a time, I'm using the VScode extension-- how soon should I expect that to contain these change?

@rgieseke
Copy link
Contributor Author

rgieseke commented Apr 2, 2025

I think this should work from VS Code once the project is updated to a Svelte version with this change (from 5.24.0).

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

Successfully merging this pull request may close these issues.

3 participants