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.

Sign up for GitHub

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

Jump to bottom

chore: align dependencies with sv #376

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

chore: align dependencies with sv #376

wants to merge 12 commits into from

Conversation

manuel3108
Copy link
Member

@manuel3108 manuel3108 commented Dec 24, 2024 •
edited
Loading

Initial goal here was to unify the UX between stuff like sv create and svelte-migrate

done:

  • use picocolors instead of kleur
  • use @clack/prompts instead of prompts

todo:

  • print package version and intro similar to sv
  • print outro (also if action is cancelled or process is exited)
  • remove most console.log as they look ugly inside the prompts and convert them to notes or information etc.
  • unify code (there is so much trivially duplicated code)
  • closes List out options for npx sv migrate #302

I opted to not use our own @sveltejs/clack-prompts package and instead use the public one. Using our own package would either require us to publish the package, or include svelte-migrate inside our own rollup config to bundle it. Not a fan of both options, thus decided to use the public package instead.

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Dec 24, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 1bcfb2f

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

@manuel3108 manuel3108 changed the title chore: unify migrate chore: unify svelte-migrate to sv Dec 24, 2024
@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Dec 24, 2024 •
edited
Loading

Open in Stackblitz

npm i https://pkg.pr.new/sveltejs/cli/sv@376

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

commit: 969433e

@manuel3108 manuel3108 marked this pull request as ready for review December 27, 2024 10:13
@Rich-Harris
Copy link
Member

preview: https://svelte-dev-git-preview-cli-376-svelte.vercel.app/

this is an automated message

@manuel3108
Copy link
Member Author

There are obviously lots of potential improvements that could still be done here. The main goal here was to unify the UX and this is a huge step forward imo.

The following things might be done at a later stage:

  • use package-manager-detector from {pm} install messages
  • further deduplicate and unify source code
  • deprecate old migrations / remove the respective source code and make the users use [email protected]

benmccann
benmccann reviewed Dec 31, 2024
View reviewed changes
Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

awesome!

the prompts migration is really nice. I'm fine with the picocolors migration though sveltekit still uses kelur. But hopefully we can migrate everything to use styleText after the next node release

.changeset/dull-toys-share.md Outdated Show resolved Hide resolved
documentation/docs/20-commands/40-sv-migrate.md Show resolved Hide resolved
packages/migrate/README.md Outdated Show resolved Hide resolved
packages/migrate/README.md Outdated Show resolved Hide resolved
packages/migrate/bin.js Outdated Show resolved Hide resolved
packages/migrate/migrations/app-state/index.js Show resolved Hide resolved
@manuel3108
Copy link
Member Author

But hopefully we can migrate everything to use styleText after the next node release

I doubt this will happen as fast, as we will also need to wait until the upcoming release is installed on a majority of the users machines. But I don't think the difference matters at all, I just wanted to align everything in this repo.

@manuel3108 manuel3108 changed the title chore: unify svelte-migrate to sv chore: align dependencies with sv Jan 1, 2025
@benmccann
Copy link
Member

I doubt this will happen as fast, as we will also need to wait until the upcoming release is installed on a majority of the users machines.

It's in Node 20, so really we just have to wait for Node 18 to go EOL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benmccann benmccann benmccann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

List out options for npx sv migrate
3 participants
@manuel3108 @Rich-Harris @benmccann