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.

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

CLI: Replace inquirer with prompts #13225

Merged
merged 8 commits into from Dec 4, 2020

Conversation

thebuilder
Copy link
Contributor

@thebuilder thebuilder commented Nov 23, 2020

Issue: #13226

What I did

Replaced all instances of https://github.com/SBoudrias/Inquirer.js with https://github.com/terkelg/prompts

This is inspired by facebook/create-react-app#10083

They are mostly interchangeable, altho the "separator" option is missing from prompts. It can be emulated by using the "disabled" option, to prevent an item from being selected.

Based on the usage pattern, I opted for a some changes to how the prompts are presented.

bootstrap
This still uses a separator.
example_1

test
Splits the choice in to two different questions.
example_2

build
Prompts you upfront if this is a watch mode or not, before presenting the list of packages.
Packages can also be selected using autocomplete.
example_3

How to test

Run the commands relying on inquirere locally.

The value is false, so could also argue it should just be removed.
@shilman
Copy link
Member

shilman commented Nov 24, 2020

Awesome @thebuilder! I'll merge this next week as part of 6.2-alpha. This week I'm doing hotfixes to 6.1.x and freely merging back and forth between master and next. Thanks for your contribution & patience!

@thebuilder
Copy link
Contributor Author

Are the failing build an issue? 🙂 It's a bit overwhelming trying to understand what causes error, when it happens right after checkout.

@shilman
Copy link
Member

shilman commented Nov 24, 2020

@thebuilder our CI is completely borked at the moment. we're working with Circle CI to try to get it back in order. but you should only look at the required checks anyway. So build and test in this case.

@shilman
Copy link
Member

shilman commented Dec 1, 2020

@thebuilder Looks like there's a TS error that's breaking CI. Can you fix it?

@storybook/cli: src/initiate.ts(252,11): error TS2322: Type '{ type: "confirm"; name: "manual"; message: string; default: false; }' is not assignable to type 'PromptObject<"manual">'.
@storybook/cli:   Object literal may only specify known properties, and 'default' does not exist in type 'PromptObject<"manual">'.
@storybook/cli: src/initiate.ts(262,9): error TS2322: Type 'string[]' is not assignable to type 'Choice[]'.

6.2's in alpha now so otherwise ready to test & merge.

@thebuilder
Copy link
Contributor Author

@shilman Sorry about - Not sure how I could miss that. Editor might have been overloaded with types. 🙈
Corrected it, and synced with next branch.

@gaetanmaisse
Copy link
Member

@thebuilder it looks like const groups defined in scripts/build-package.js:L56 isn't used anymore, can you remove it?

@shilman shilman changed the title Replace inquirer with prompts CLI: Replace inquirer with prompts Dec 4, 2020
@shilman shilman merged commit 887ae02 into storybookjs:next Dec 4, 2020
@thebuilder thebuilder deleted the replace-inquirer branch December 4, 2020 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants