replace enquirer+inquirer with @clack/prompts+launch-editor - #1879
Conversation
🦋 Changeset detectedLatest commit: 7c81b29 The changes in this PR will be included in the next version bump. 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 💥 An error occurred when fetching the changed packages and changesets in this PR |
|
It would be great to include some before/after screenshots in the PR summary + call out behavior differences (if there are any) |
yep, will add more info when theres more progress but for now i have one recording and will add more later |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## next #1879 +/- ##
==========================================
+ Coverage 79.64% 81.09% +1.44%
==========================================
Files 52 53 +1
Lines 2412 2348 -64
Branches 710 714 +4
==========================================
- Hits 1921 1904 -17
+ Misses 441 402 -39
+ Partials 50 42 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Do we still need to handle |
|
iI think it's safe to remove any mentions of v1 configs from the codebase |
There was a problem hiding this comment.
i'm thinking that the log silencers for the tests should probably always be on and the functions removed, but that would be a ton of changes so i left them as-is in this PR
enquirer+inquirer with @clack/prompts+launch-editorenquirer+inquirer with @clack/prompts+launch-editor
|
@beeequeue could you sync this with next? |
|
i added a section in the pr body for the current follow-ups that will be needed if we don't do them in this pr |
|
One extra thing we should do before landing this is to check out how the "delegated auth mode" renders now. It's when we spawn npm so it can handle OTP-related prompting. Would you be able to test that out? |
|
im not sure how to trigger it, but when i tried to publish a package and it failed i got a very weird looking output: after i logged in and tried again it worked and looks a bit weird but okay-ish i think this is because i added the spinner after i migrated the npm publishing files, and didn't check how it affected them. i would love if we could use |
|
Hm, that's weird that both runs ended up being indented differently... 🤔 As an experiment, I also wanted to take a look how this would look like completely without the "guide". For that, I had to open a PR here: bombshell-dev/clack#500 I think it's worth waiting a couple of days for that PR to possibly get merged in so we could test both versions in the repo here |
|
presumably the different indentation is from the npm command's output being logged without going through clack's formatting, unlike in the error one where they are handled and printed with them |
|
Overall, this looks great. The only thing I'd like to do now is to properly compare the |
|
imo the publish command needs to be redone, but i would also be okay with leaving it as-is for another pr |
|
What kind of a makeover do u think about? |
bluwy
left a comment
There was a problem hiding this comment.
Since this merges to next, we'd need to manually close the issue after merge.
|
we can add them manually via the |
|
i'm gonna do a final check that each command still looks okay after all the rebasing, then i'll merge it. |
|
Maybe I'm misremembering, but I think the auto-close only works when merging to the default branch. So next wouldn't work. |


closes #1814
closes #1043
closes #1861
closes #1814
part of #1869
replaces
enquirerwith the slightly smaller and more modern@clack/prompts, whose API is also a lot nicer in my opinionit also replaces
@inquirer/external-editorwithlaunch-editor, which is quite a lot smaller, works better in my experience, and is more flexible allowing us to remove the hack inpackages/cli/src/commands/add/index.tsthings to do in follow-up PRs
mritocleyeorgunshi#Xrecordings / screenshots
Recordings
cancelling at any point: https://asciinema.org/a/HxMUViS932Pwi5Oj
changeset add: https://asciinema.org/a/1JFbvyuuVbbfOjuychangeset initwith existing config:changeset versionwith pre-release:changeset tag: https://asciinema.org/a/aVKLczi02OzZWHrkchangeset status --verbose:changeset publish:non-interactive (otp, oidc): https://asciinema.org/a/7tjKcMe3d1NNshUc
interactive (passkey): https://asciinema.org/a/yF9PEOwG2sROT5Zz
changes to messages/texts
changes
general rules i tried to follow
texts
legend:
add
${error.toString()}