Skip to content

feat: added app-state migration into svelte-5 migration #494

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 4 commits into from
Mar 23, 2025

Conversation

garikAsplund
Copy link
Contributor

@garikAsplund garikAsplund commented Mar 20, 2025

Fixes #485

This is the simplest approach I could think of--I know it can get a little hairy with all the versions of Svelte and SvelteKit out there. Still, I believe most of the logic is already in place with the CLI to make sure the user has the right versioning heading into the migration.

This only runs the app-state migration script if the user has SvelteKit. It doesn't really log anything or add any error handling--I'll leave those up to you if desired.

Also, I know you guys have a contributing guide on the docket, but I was wondering how exactly you test this CLI on your own projects. I ask because I ended up using npm link which is pretty standard, but I had to edit these lines of code to make sure that when I tested my fixes locally that it linked to my scripts instead of pulling @latest from npx 🙏

Copy link

changeset-bot bot commented Mar 20, 2025

🦋 Changeset detected

Latest commit: 6c7d125

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

This PR includes changesets to release 1 package
Name Type
svelte-migrate Minor

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

@garikAsplund garikAsplund marked this pull request as ready for review March 20, 2025 06:11
Copy link

pkg-pr-new bot commented Mar 22, 2025

Open in Stackblitz

npm i https://pkg.pr.new/sveltejs/cli/sv@494
npm i https://pkg.pr.new/sveltejs/cli/svelte-migrate@494

commit: 6c7d125

@manuel3108
Copy link
Member

You can actually just run node ./packages/migrate/bin.js to just execute the migrate cli. The script you mentioned is basically doing the same thing. Really need to get that contributing guide out.

Thanks for the PR btw!
I think this is the correct way to go forward, and I don't think a prompt or log is personally necessary. But i would love to get @benmccann feedback on this.

Bu I don't think we should merge this as long as #368 is still open, looks like it's only finding some of the occurrences and not all.

@manuel3108 manuel3108 changed the title added app-state migrate into svelte-5 script feat: added app-state migration into svelte-5 migration Mar 22, 2025
@manuel3108 manuel3108 added enhancement New feature or request pkg:migrate svelte-migrate labels Mar 22, 2025
@benmccann
Copy link
Member

I think this is a good PR. I don't know if we need to block on that issue. It would be great to get it fixed, but migrating some stuff is better than migrating none.

Copy link
Member

@manuel3108 manuel3108 left a comment

Choose a reason for hiding this comment

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

In that case, let's go!

@manuel3108 manuel3108 merged commit 1184c1a into sveltejs:main Mar 23, 2025
7 checks passed
@github-actions github-actions bot mentioned this pull request Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pkg:migrate svelte-migrate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically run app-state migration as part of svelte-5 migration
3 participants