Chore: Replace the inquirer dependency with enquirer #13254

Siilwyn · 2020-05-03T16:08:09Z

Prerequisites checklist

I have read the contributing guidelines.
The team has reached consensus on the changes proposed in this pull request. If not, I understand that the evaluation process will begin with this pull request and won't be merged until the team has reached consensus.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:
See #13208: replace dependency to decrease package size.

What changes did you make? (Give an overview)

Replace the dependency and changed the equivalent methods to work.

Is there anything you'd like reviewers to focus on?

Mostly if edge cases work, I've tested some paths but not certain if everything went over like how it was supposed to work in the first place. (never used eslint --init myself before)

Closes #13208

Siilwyn · 2020-05-03T16:09:12Z

lib/init/config-initializer.js

+            result(input) {
+                return this.skip ? null : input;


This result method is needed since enquirer will even return the initial value when the question is skipped, unlike inquirer.

Siilwyn · 2020-05-03T16:09:54Z

lib/init/config-initializer.js

            name: "purpose",
            message: "How would you like to use ESLint?",
-            default: "problems",
+            initial: 1,


The returned number matches the name value of nth in the choices array.

Can you add that as a comment?

@Siilwyn can you add this as a comment in the file?

Yes 👍
(Didn't get a notification for your first comment.)

Siilwyn · 2020-05-03T16:11:46Z

lib/init/config-initializer.js

+            skip() {
+                return this.state.answers.purpose !== "style";
+            },


The skip method is the reverse of when so the conditional statement is reversed.

yeonjuan

Hi. @Siilwyn Thanks for working this. 😀
I tried it and found two issues. (I don't know about "enquirer" well)

default value representation: question use character -(y/n) / answer use boolean - (true/false)
- I think we should use the same representation for the default value ( maybe "(y/n)"?)
Does your project use TypeScript? (Y/n) - this question shows us the default value as Y but the default value of the answer is false
- both should be semantic "No"

Siilwyn · 2020-05-04T20:04:59Z

@yeonjuan alright, I've replaced all confirm prompts to have more humane answers.

(I see the commit-message CI fail, but I think this is alright since when this PR is ready to merge it can be squashed to use my first commit which is compliant right?)

yeonjuan · 2020-05-05T13:15:42Z

@Siilwyn Thanks for the change :)

(I see the commit-message CI fail, but I think this is alright since when this PR is ready to merge it can be squashed to use my first commit which is compliant right?)

Yes but, the PR message should follow the guideline.
Because we use the PR title as a commit message when it squashed(If there are multiple commits)

So, Would you mind change the PR title?

yeonjuan · 2020-05-05T13:38:36Z

lib/init/config-initializer.js

+                return this.state.answers.purpose !== "style";
+            },
+            result(input) {
+                return this.skip ? null : input;


@Siilwyn
It seems not working as we expected.
When I answered with the "guide," in this step. but the selecting styleguide step was skipped.

✔ How would you like to use ESLint? · style
✔ What type of modules does your project use? · none
✔ Which framework does your project use? · none
✔ Does your project use TypeScript? · No / Yes
✔ Where does your code run? · browser
✔ How would you like to define a style for your project? · guide
✔ What format do you want your config file to be in? · JavaScript
✔ What style of indentation do you use? · tab
✔ What quotes do you use for strings? · double
✔ What line endings do you use? · unix
✔ Do you require semicolons? · No / Yes
Successfully created .eslintrc.js file in

It seems the this.skip is a function (truthy value), so the result always be a null
Maybe we should change all the logic which uses this.skip

Good catch! That was a typo sorry about that. I was testing it on one question with skipped got it working but then messed up the copy paste on all somehow.

lib/init/config-initializer.js

Co-authored-by: YeonJuan <yeonjuan93@naver.com>

yeonjuan

@Siilwyn Thanks for contributing! LGTM!

Siilwyn · 2020-05-12T12:35:34Z

What happens now?

nzakas · 2020-05-12T16:50:34Z

@Siilwyn we still need some other reviews before this can be merged. Stay tuned.

nzakas

Sorry we lost track of this! There’s just one unresolved request but it’s for adding a comment so I won’t hold this up if you can’t get to it.

@kaicataldo can you take a look? I’d like another set of eyes.

nzakas · 2020-06-09T01:50:07Z

lib/init/config-initializer.js

            name: "purpose",
            message: "How would you like to use ESLint?",
-            default: "problems",
+            initial: 1,


@Siilwyn can you add this as a comment in the file?

nzakas · 2020-06-09T14:48:22Z

Oops, looks like that last commit introduced a conflict. Can you take a look?

Siilwyn · 2020-06-09T14:59:25Z

@nzakas should be fixed now!

kaicataldo · 2020-06-09T23:53:32Z

Thanks for contributing!

Update: Replace dependency inquirer with enquirer (fixes #13208)

9f623e7

Closes #13208

eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label May 3, 2020

Siilwyn commented May 3, 2020

View reviewed changes

yeonjuan suggested changes May 4, 2020

View reviewed changes

Replace confirm prompts with toggle prompts to match inquirer behaviour

c95a7d9

yeonjuan reviewed May 5, 2020

View reviewed changes

lib/init/config-initializer.js Show resolved Hide resolved

Siilwyn added 2 commits May 5, 2020 18:05

Fix skip property typo

7390661

Add help description

58aeeb5

yeonjuan reviewed May 5, 2020

View reviewed changes

lib/init/config-initializer.js Outdated Show resolved Hide resolved

aladdin-add changed the title ~~Replace the inquirer dependency with enquirer~~ Chore: Replace the inquirer dependency with enquirer May 5, 2020

Use hint instead of message for usage informatino

bd396b7

Co-authored-by: YeonJuan <yeonjuan93@naver.com>

yeonjuan approved these changes May 6, 2020

View reviewed changes

lydell mentioned this pull request May 24, 2020

smaller alternative to RxJS? SBoudrias/Inquirer.js#913

Closed

aladdin-add self-requested a review June 1, 2020 16:41

nzakas approved these changes Jun 9, 2020

View reviewed changes

Add comment about initial select prompt

0f0b186

Siilwyn added 2 commits June 9, 2020 16:51

Merge branch 'master' into replace-inquirer

31278e3

Update config-initializer.js

fe79cbb

Update config-initializer.js

67088f2

kaicataldo approved these changes Jun 9, 2020

View reviewed changes

kaicataldo merged commit 7ce7988 into eslint:master Jun 9, 2020

eslint-deprecated bot locked and limited conversation to collaborators Dec 7, 2020

eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Dec 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore: Replace the inquirer dependency with enquirer #13254

Chore: Replace the inquirer dependency with enquirer #13254

Siilwyn commented May 3, 2020

Siilwyn May 3, 2020

Siilwyn May 3, 2020

nzakas May 12, 2020

nzakas Jun 9, 2020

Siilwyn Jun 9, 2020

Siilwyn May 3, 2020

yeonjuan left a comment •

edited

Siilwyn commented May 4, 2020 •

edited

yeonjuan commented May 5, 2020 •

edited

yeonjuan May 5, 2020 •

edited

Siilwyn May 5, 2020

yeonjuan left a comment

Siilwyn commented May 12, 2020

nzakas commented May 12, 2020

nzakas left a comment

nzakas Jun 9, 2020

nzakas commented Jun 9, 2020

Siilwyn commented Jun 9, 2020

kaicataldo commented Jun 9, 2020

Chore: Replace the inquirer dependency with enquirer #13254

Chore: Replace the inquirer dependency with enquirer #13254

Conversation

Siilwyn commented May 3, 2020

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeonjuan left a comment • edited

Choose a reason for hiding this comment

Siilwyn commented May 4, 2020 • edited

yeonjuan commented May 5, 2020 • edited

yeonjuan May 5, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeonjuan left a comment

Choose a reason for hiding this comment

Siilwyn commented May 12, 2020

nzakas commented May 12, 2020

nzakas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nzakas commented Jun 9, 2020

Siilwyn commented Jun 9, 2020

kaicataldo commented Jun 9, 2020

yeonjuan left a comment •

edited

Siilwyn commented May 4, 2020 •

edited

yeonjuan commented May 5, 2020 •

edited

yeonjuan May 5, 2020 •

edited