-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: prompt SvelteKit 2 migration during Svelte 5 migration if necessary #12748
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
Conversation
🦋 Changeset detectedLatest commit: 5fcde5d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
benmccann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally I would just bail if SvelteKit 1 is detected rather than trying to run both migrations because I think it would be good for the user to run the SvelteKit 2 migration and then commit the result to git before trying to run the next migration
| } | ||
|
|
||
| const kit_dep = pkg.devDependencies?.['@sveltejs/kit'] ?? pkg.dependencies?.['@sveltejs/kit']; | ||
| if (kit_dep && semver.validRange(kit_dep) && semver.gtr('2.0.0', kit_dep)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a little surprising to me to see 2 > kit_dep vs kit_dep < 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yoda-speak - it's done like that in all other migration, I don't care honestly.
|
agree with Ben, just log the command for the migration and ask them to do that first, commit and TEST the result before starting with svelte5 migration. otherwise we'll get a few strange bug reports regarding svelte5 migration that are really about kit |
closes sveltejs/svelte#13462