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

New: add yarn support to --init (fixes #13645) #13756

Closed
wants to merge 1 commit into from
Closed

New: add yarn support to --init (fixes #13645) #13756

wants to merge 1 commit into from

Conversation

snitin315
Copy link
Contributor

@snitin315 snitin315 commented Oct 14, 2020

Prerequisites checklist

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:

Add support of the installation of dependencies via yarn when using eslint --init

What changes did you make? (Give an overview)

Fixes eslint/create-config#20

Screenshot at 2020-10-14 19-45-49

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

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Oct 14, 2020
@snitin315 snitin315 changed the title New: add yarn support to --init (fixes #13654) New: add yarn support to --init (fixes #13645) Oct 14, 2020
lib/init/npm-utils.js Outdated Show resolved Hide resolved
lib/init/npm-utils.js Outdated Show resolved Hide resolved
@snitin315 snitin315 marked this pull request as ready for review October 15, 2020 09:01
@mdjermanovic mdjermanovic added cli Relates to ESLint's command-line interface enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Oct 15, 2020
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I think the proposal is to simply add an "npm or yarn" question instead of trying to figure out that automatically, but let's wait for the issue to be accepted before doing any changes.

Please note that this enhancement isn't accepted yet. Per our process, it still needs a champion from the team.

lib/init/npm-utils.js Outdated Show resolved Hide resolved
@btmills btmills added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Nov 20, 2020
@btmills
Copy link
Member

btmills commented Nov 20, 2020

We approved this change in today's TSC meeting. The issue and this PR are now marked as Accepted. To reduce complexity, --init shouldn't try to guess the correct package manager. It should ask Which package manager does your project use? with npm as the default and yarn as an option. @snitin315 thanks for working on this, and hopefully that's the answer you needed to finish this up! 🚀

@g-plane
Copy link
Member

g-plane commented Nov 20, 2020

Can we add another package manager "pnpm"?

lib/init/config-initializer.js Outdated Show resolved Hide resolved
Comment on lines 443 to 452
{
type: "select",
name: "packageManager",
message: "Which package manager does your project use?",
initial: 0,
choices: ["npm", "yarn"]
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we skip this step if the previous answer was "No"?

Also, I'm still not sure would it be better to have just one step with "No"/"Yes, with npm"/"Yes, with yarn".

If user doesn't want npm or yarn but some third package manager, at this point the only choice for them would be to somehow exit the process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we skip this step if the previous answer was "No"?

Now skipped if No was selected. but we still have a different question for selecting the package manager.

lib/init/npm-utils.js Outdated Show resolved Hide resolved
@mdjermanovic
Copy link
Member

I tried this version with Yarn 2:

C:\testproject>yarn set version berry
Resolving berry to a url...
Downloading https://github.com/yarnpkg/berry/raw/master/packages/berry-cli/bin/berry.js...
Saving it into C:\testproject\.yarn\releases\yarn-berry.cjs...
Updating C:\testproject/.yarnrc.yml...
Done!

C:\testproject>yarn -v
2.4.0

C:\testproject>yarn init
{
  name: 'testproject'
}

C:\testproject>yarn add eslint@https://github.com/snitin315/eslint#feat/yarn-init -D
➤ YN0000: ┌ Resolution step
➤ YN0013: │ eslint@https://github.com/snitin315/eslint.git#commit=089c5a851879c0c67f7da828e754a13640e34f15 can't be found in the cache and will be fetched from GitHub
➤ YN0000: └ Completed in 4s 452ms
➤ YN0000: ┌ Fetch step
➤ YN0013: │ which@npm:2.0.2 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ word-wrap@npm:1.2.3 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ wrappy@npm:1.0.2 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ write@npm:1.0.3 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yallist@npm:4.0.0 can't be found in the cache and will be fetched from the remote registry
➤ YN0000: └ Completed in 2s 824ms
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed in 0s 541ms
➤ YN0000: Done in 7s 863ms

C:\testproject>yarn eslint --init
√ How would you like to use ESLint? · style
√ What type of modules does your project use? · esm
√ 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
√ Which style guide do you want to follow? · airbnb
√ What format do you want your config file to be in? · JavaScript
Checking peerDependencies of eslint-config-airbnb-base@latest
The config that you've selected requires the following dependencies:

eslint-config-airbnb-base@latest eslint@^5.16.0 || ^6.8.0 || ^7.2.0 eslint-plugin-import@^2.22.1
√ Would you like to install them now ? · No / Yes
√ Which package manager does your project use? · yarn
Installing eslint-config-airbnb-base@latest, eslint@^5.16.0 || ^6.8.0 || ^7.2.0, eslint-plugin-import@^2.22.1
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed in 1s 428ms
➤ YN0000: ┌ Fetch step
➤ YN0013: │ string.prototype.trimstart@npm:1.0.3 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ strip-bom@npm:3.0.0 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ tsconfig-paths@npm:3.9.0 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ validate-npm-package-license@npm:3.0.4 can't be found in the cache and will be fetched from the remote registry
➤ YN0019: │ eslint-https-9f3ff1734f-55dfae3889.zip appears to be unused - removing
➤ YN0000: └ Completed in 2s 335ms
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed in 0s 667ms
➤ YN0000: Done in 4s 501ms
[Warning] The runtime detected new informations in a PnP file; reloading the API instance (C:\testproject\.pnp.js)
C:\testproject\.pnp.js:6176
    throw firstError;
    ^

Error: Qualified path resolution failed - none of the candidates can be found on the disk.

Source path: C:\testproject\.yarn\cache\eslint-https-9f3ff1734f-55dfae3889.zip\node_modules\eslint\package.json
Rejected candidate: C:\testproject\.yarn\cache\eslint-https-9f3ff1734f-55dfae3889.zip\node_modules\eslint\package.json
Rejected candidate: C:\testproject\.yarn\cache\eslint-https-9f3ff1734f-55dfae3889.zip\node_modules\eslint\package.json.js
Rejected candidate: C:\testproject\.yarn\cache\eslint-https-9f3ff1734f-55dfae3889.zip\node_modules\eslint\package.json.json
Rejected candidate: C:\testproject\.yarn\cache\eslint-https-9f3ff1734f-55dfae3889.zip\node_modules\eslint\package.json.node

Require stack:
- C:\testproject\.yarn\cache\eslint-https-9f3ff1734f-55dfae3889.zip\node_modules\eslint\bin\eslint.js
    at internalTools_makeError (C:\testproject\.pnp.js:6640:34)
    at resolveUnqualified (C:\testproject\.pnp.js:7679:13)
    at resolveRequest (C:\testproject\.pnp.js:7703:14)
    at Object.resolveRequest (C:\testproject\.pnp.js:7775:26)
    at Function.external_module_.Module._resolveFilename (C:\testproject\.pnp.js:6153:34)
    at Function.external_module_.Module._load (C:\testproject\.pnp.js:6018:48)
    at Module.require (internal/modules/cjs/loader.js:1025:19)
    at require (internal/modules/cjs/helpers.js:72:18)
    at process.onFatalError (C:\testproject\.yarn\cache\eslint-https-9f3ff1734f-55dfae3889.zip\node_modules\eslint\bin\eslint.js:116:25)
    at process.emit (events.js:315:20)

It crashes probably because the github version of ESLint gets removed while installing a new version, but continues to execute. I'm not sure if the same will happen with the real package, and how to test that before publishing.

It seems that eslint --init always installs ESLint if a style guide config was chosen, because it's declared there as a peer dependency. Perhaps this wasn't the intended behavior in the case that the already installed local version satisfies the range, and maybe it isn't a good idea to continue executing local ESLint when it has been possibly overwritten with another version of itself (*), or removed/relocated.

Also, unlike npm, yarn declares dependencies exactly as they were given to yarn add, so package.json looks like this at the end:

{
  "name": "testproject",
  "devDependencies": {
    "eslint": "^5.16.0 || ^6.8.0 || ^7.2.0",
    "eslint-config-airbnb-base": "latest",
    "eslint-plugin-import": "^2.22.1"
  }
}

That looks different than dependencies eslint --init generates with npm:

"devDependencies": {
    "eslint": "^7.14.0",
    "eslint-config-airbnb-base": "^14.2.1",
    "eslint-plugin-import": "^2.22.1"
  }

(*) - I believe this causes problems with Yarn 1 and eslint-config-standard:

C:\testproject>yarn -v
1.22.5

C:\testproject>yarn init
yarn init v1.22.5
question name (testproject):
question version (1.0.0):
question description:
question entry point (index.js):
question repository url:
question author:
question license (MIT):
question private:
success Saved package.json
Done in 3.14s.

C:\testproject>yarn add eslint@https://github.com/snitin315/eslint#feat/yarn-init -D
yarn add v1.22.5
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...

success Saved lockfile.
success Saved 89 new dependencies.
info Direct dependencies
└─ [email protected]
info All dependencies
├─ @babel/[email protected]
├─ @babel/[email protected]
├─ @babel/[email protected]
├─ @eslint/[email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
└─ [email protected]
Done in 9.48s.

C:\testproject>yarn eslint --init
yarn run v1.22.5
$ C:\testproject\node_modules\.bin\eslint --init
√ How would you like to use ESLint? · style
√ What type of modules does your project use? · esm
√ 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
√ Which style guide do you want to follow? · standard
√ What format do you want your config file to be in? · JavaScript
Checking peerDependencies of eslint-config-standard@latest
The config that you've selected requires the following dependencies:

eslint-config-standard@latest eslint@^7.12.1 eslint-plugin-import@^2.22.1 eslint-plugin-node@^11.1.0 eslint-plugin-promise@^4.2.1
√ Would you like to install them now ? · No / Yes
√ Which package manager does your project use? · yarn
Installing eslint-config-standard@latest, eslint@^7.12.1, eslint-plugin-import@^2.22.1, eslint-plugin-node@^11.1.0, eslint-plugin-promise@^4.2.1
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...

success Saved lockfile.
success Saved 54 new dependencies.
info Direct dependencies
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
└─ [email protected]
info All dependencies
├─ @types/[email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
└─ [email protected]

Oops! Something went wrong! :(

ESLint: 7.13.0

Error: An error occurred while generating your JavaScript config file. A config file was still generated, but the config file itself may not follow your linting rules.
Error: Failed to load plugin 'node' declared in 'BaseConfig » eslint-config-standard': Cannot find module 'C:\testproject\node_modules\semver\index.js'. Please verify that the package.json has a valid "main" entry
    at tryPackage (internal/modules/cjs/loader.js:321:19)
    at Function.Module._findPath (internal/modules/cjs/loader.js:682:18)
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:952:27)
    at Function.Module._load (internal/modules/cjs/loader.js:841:27)
    at Module.require (internal/modules/cjs/loader.js:1025:19)
    at require (C:\testproject\node_modules\v8-compile-cache\v8-compile-cache.js:159:20)
    at Object.<anonymous> (C:\testproject\node_modules\eslint-plugin-node\lib\util\get-configured-node-version.js:7:19)
    at Module._compile (C:\testproject\node_modules\v8-compile-cache\v8-compile-cache.js:192:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1157:10)
    at Module.load (internal/modules/cjs/loader.js:985:32)
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@snitin315
Copy link
Contributor Author

(*) - I believe this causes problems with Yarn 1 and eslint-config-standard:

Works for me. It may be failed for you because you didn't create an index.js file as per main field in package.json created after yarn init.

┌─[snitin315@parrot]─[~/test/eslint-init]
└──╼ $yarn add eslint@https://github.com/snitin315/eslint#feat/yarn-init -D
yarn add v1.22.4
warning ../../package.json: No license field
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
success Saved 1 new dependency.
info Direct dependencies
└─ [email protected]
info All dependencies
└─ [email protected]
Done in 18.61s.
┌─[snitin315@parrot]─[~/test/eslint-init]
└──╼ $yarn eslint --init
yarn run v1.22.4
warning ../../package.json: No license field
$ /home/snitin315/test/eslint-init/node_modules/.bin/eslint --init
✔ How would you like to use ESLint? · style
✔ What type of modules does your project use? · esm
✔ 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
✔ Which style guide do you want to follow? · standard
✔ What format do you want your config file to be in? · JavaScript
Checking peerDependencies of eslint-config-standard@latest
The config that you've selected requires the following dependencies:

eslint-config-standard@latest eslint@^7.12.1 eslint-plugin-import@^2.22.1 eslint-plugin-node@^11.1.0 eslint-plugin-promise@^4.2.1
✔ Would you like to install them now? · No / Yes
✔ Which package manager does your project use? · yarn
Installing eslint-config-standard@latest, eslint@^7.12.1, eslint-plugin-import@^2.22.1, eslint-plugin-node@^11.1.0, eslint-plugin-promise@^4.2.1
warning ../../package.json: No license field
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
success Saved 2 new dependencies.
info Direct dependencies
├─ [email protected]
└─ [email protected]
info All dependencies
├─ [email protected]
└─ [email protected]
Successfully created .eslintrc.js file in /home/snitin315/test/eslint-init
Done in 31.43s.
┌─[snitin315@parrot]─[~/test/eslint-init]
└──╼ $

@mdjermanovic
Copy link
Member

It could be that this won't be happening often, but installing local packages initiated from an already installed local package still seems problematic, especially when that local package (ESLint) reinstalls itself with Yarn 2, which will then abort the current process right away (I think that's what happened in my test with Yarn 2).

It seems better to extract the --init functionality to another package, as proposed in eslint/rfcs#58. Maybe we should wait for that before adding yarn support?

The other problem is with package.json. Yarn 2 writes dependencies literally as they were given to yarn add, so it ends up being "eslint": "^5.16.0 || ^6.8.0 || ^7.2.0" and "eslint-config-airbnb-base": "latest". I'm not sure we want that.

@snitin315
Copy link
Contributor Author

It seems better to extract the --init functionality to another package, as proposed in eslint/rfcs#58. Maybe we should wait for that before adding yarn support?

Sounds good to me 👍🏻

@nzakas
Copy link
Member

nzakas commented Mar 17, 2021

@snitin315 are you still working on this?

@snitin315
Copy link
Contributor Author

I'm not sure what else do we need to update here. If anyone is interested feel free to take over the PR.

@mdjermanovic
Copy link
Member

I think we assumed that this will be more or less just running yarn add instead of npm install, but there are at least two non-trivial problems (both are described in #13756 (comment) and again in #13756 (comment)) that, I believe, should be addressed, so maybe this change needs an RFC.

@nzakas
Copy link
Member

nzakas commented Apr 21, 2021

It seems like there is consensus that it would be better to extract the init functionality into a separate package, so should we close this?

@btmills
Copy link
Member

btmills commented Apr 21, 2021

Closing this in favor of solving it in an extracted package is fine by me.

@mdjermanovic
Copy link
Member

Agreed, so closing this PR.

@snitin315 snitin315 deleted the feat/yarn-init branch June 6, 2021 17:20
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Oct 19, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion cli Relates to ESLint's command-line interface enhancement This change enhances an existing feature of ESLint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Yarn and pnpm support in eslint --init
7 participants