deps: replace inquirer with enquirer #12317
Conversation
| const prompt = new Confirm({ | ||
| name: 'isErrorReportingEnabled', | ||
| initial: false, | ||
| message: MESSAGE, |
There was a problem hiding this comment.
as mentioned in #6658 (comment), the question part of the prompt will have to be manually bolded if we want to preserve the old behavior (which I think still looks good and makes the question stand out).
I can't add a suggestion to L18 in the github UI, but it should just need to change to
` ${log.bold}May we anonymously report runtime exceptions to improve the tool over time?${log.reset} `; // eslint-disable-line max-lenThere was a problem hiding this comment.
Thank you for your kindly review 😍. I made the message bold again.
| name: 'isErrorReportingEnabled', | ||
| initial: false, | ||
| message: MESSAGE, | ||
| }); |
There was a problem hiding this comment.
to take care of enquirer preventing ctrl-c from working, I think the best approach (based on #6658 (comment) and a quick debug session...there may be a better way but I don't see it) is adding
| }); | |
| actions: {ctrl: {}}, | |
| }); |
(and types/enquirer/index.d.ts will also have to be updated)
This gets rid of all the ctrl-* key handling, but since we're only using a y/N prompt, I don't think we're going to miss any of it (e.g. if it was free-form text entry we'd probably want to keep ctrl-a and ctrl-e, but we don't have that).
It's still a bit weird. ctrl-c during the prompt still cancels the whole run, but answering yes or no and then doing ctrl-c during the lighthouse run kills the lighthouse process but not the Chrome one. That's less than ideal, but it's better than ctrl-c not doing anything, and this is only on the first run when the user is prompted.
There was a problem hiding this comment.
I added actions to the option. Also, I updated the path of .d.ts as suggested at #12305 (comment) (from types/enquirer/index.d.ts to types/enquirer.d.ts).
adamraine
changed the title
takenspc
force-pushed
the
replace-inquirer-with-enquirer
branch
from
4608e31
to
99d1cdc
Compare
…takenspc:replace-inquirer-with-enquirer
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I resolved the |
brendankenny
changed the title
brendankenny
changed the title
Summary
The inquirer is one of the big dependencies as it depends on rxjs. Replacing inquirer with enquirer would reduce the install size.
My motivation is to make CI setup which runs lighthouse faster so that reducing dependencies is more important than devDependencies.
Screenshots of CLI with this change
Initial rendering:
Timeout reached (rejected):
Accepted:
Related Issues/PRs
#12305