Don't coerce version strings to semver, quote package names #126

nathanhleung · 2021-01-29T03:39:43Z

install-peerdeps uses the semver package to coerce peerdep versions specified in package.json to specific, "valid"/literal versions of the form x.y.z to generate the peer dependency install command. However, this coercion means that a version specified as ^7 in peerDependencies is coerced to 7.0.0, which might not necessarily exist.

Instead, we should just use the version directly specified in package.json. Resolves #71.

Also started quoting package names to resolve #64

Previous Behavior

$ npx install-peerdeps --dev @wordpress/eslint-plugin --dry-run

install-peerdeps v3.0.1
This command would have been run to install @wordpress/eslint-plugin@latest:
npm install @wordpress/eslint-plugin@8.0.1 eslint@7.0.0 --save-dev

New Behavior

$ npx install-peerdeps --dev @wordpress/eslint-plugin --dry-run

install-peerdeps v3.0.1
This command would have been run to install @wordpress/eslint-plugin@latest:
npm install "@wordpress/eslint-plugin@8.0.1" "eslint@^7" --save-dev

@karlhorky could you test this branch?

@ljharb should this change be classified as a bugfix (bump semver patch version)? it changes prior behavior but should result in more flexibility rather than less (i.e. the install command will install @^7 etc rather than @7.0.0)

ljharb · 2021-01-29T03:41:08Z

src/install-peerdeps.js

-    // Take each version in the range and find the maxSatisfying
-    const rangeSplit = version
-      .split(" ")
-      .map(v => coerce(v))


it does seem important to still check range validity here? altho then i suppose dist-tags like “next” wouldn’t work.

If the range is invalid wouldn't that be a bug in the other package? Do you know how NPM handles invalid ranges here?

Based on the package.json docs on the NPM website, simply taking the last element of the range should work.

Related PR: #4

Thanks for reviewing so fast and sorry for the messy diff — I think my prettier extension settings have changed since I first started writing this package. Force-pushed without the formatting so it's hopefully easier to read.

Yeah i think it makes the most sense to just let the package manager handle it.

karlhorky · 2021-01-29T08:06:02Z

@karlhorky could you test this branch?

Sure, @uebriges and I can test today with create-react-app!

karlhorky · 2021-01-29T08:30:32Z

Yep, looks good! 🚀

$ git clone https://github.com/nathanhleung/install-peerdeps.git
$ cd install-peerdeps
$ git checkout nathanhleung-version-fix
$ yarn
$ yarn build && node . --yarn --dev eslint-config-react-app  --dry-run
yarn run v1.22.5
$ npm run clean
> install-peerdeps@3.0.1 clean
> rimraf lib
$ babel src --out-dir lib --ignore *.test.js
Successfully compiled 9 files with Babel (1444ms).
✨  Done in 3.34s.
install-peerdeps v3.0.1
This command would have been run to install eslint-config-react-app@latest:
yarn add eslint-config-react-app@6.0.0 @typescript-eslint/eslint-plugin@^4.0.0 @typescript-eslint/parser@^4.0.0 babel-eslint@^10.0.0 eslint@^7.5.0 eslint-plugin-flowtype@^5.2.0 eslint-plugin-import@^2.22.0 eslint-plugin-jest@^24.0.0 eslint-plugin-jsx-a11y@^6.3.1 eslint-plugin-react@^7.20.3 eslint-plugin-react-hooks@^4.0.8 eslint-plugin-testing-library@^3.9.0 --dev

nathanhleung · 2021-01-29T15:54:04Z

Yep, looks good! 🚀

Perfect, thanks!

nathanhleung · 2021-01-29T16:09:37Z

@karlhorky I just published and it looks like I'm getting an error. Can you try running npx install-peerdeps eslint-config-airbnb --dev without --dry-run and let me know if it works, sending me any error logs if it doesn't?

nathanhleung · 2021-01-29T16:21:46Z

@karlhorky I just published and it looks like I'm getting an error. Can you try running npx install-peerdeps eslint-config-airbnb --dev without --dry-run and let me know if it works, sending me any error logs if it doesn't?

Reverted the change that added quotes around the package name for now and published a new version, 3.0.3 that is working on my end. However, the version that wasn't working for me is still up as version 3.0.2.

karlhorky · 2021-01-29T16:37:19Z

Ah sorry! 🙈 I saw that you were using --dry-run, so I didn't end up testing it with a normal app (eg. without the --dry-run).

We'll try to do the full workflow with 3.0.2 - I guess if it's broken for you, then it'll be also broken for us.

karlhorky · 2021-01-29T16:57:12Z

3.0.2 on macOS results in a ERR undefined error (related to @kirlat's #128?)

$ mkdir repro
$ cd repro
$ npx create-react-app .
$ npx install-peerdeps@3.0.2 eslint-config-airbnb --dev
Need to install the following packages:
  install-peerdeps@3.0.2
Ok to proceed? (y) y
npm WARN deprecated @babel/polyfill@7.12.1: 🚨 This package has been deprecated in favor of separate inclusion of a polyfill and regenerator-runtime (when needed). See the @babel/polyfill docs (https://babeljs.io/docs/en/babel-polyfill) for more information.
npm WARN deprecated core-js@2.6.12: core-js@<3 is no longer maintained and not recommended for usage due to the number of issues. Please, upgrade your dependencies to the actual version of core-js@3.
install-peerdeps v3.0.2
It seems as if you are using Yarn. Would you like to use Yarn for the installation? (y/n) y
Installing peerdeps for eslint-config-airbnb@latest.
yarn add "eslint-config-airbnb@18.2.1" "eslint@^7.2.0" "eslint-plugin-import@^2.22.1" "eslint-plugin-jsx-a11y@^6.4.1" "eslint-plugin-react@^7.21.5" "eslint-plugin-react-hooks@^1.7.0" --dev

ERR undefined
npm ERR! code 1
npm ERR! path /Users/k/p/repro
npm ERR! command failed
npm ERR! command sh -c install-peerdeps eslint-config-airbnb --dev

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/k/.npm/_logs/2021-01-29T16_53_19_871Z-debug.log

karlhorky · 2021-01-29T16:58:01Z

3.0.3 on macOS (in the same CRA project) results in successful run:

npx install-peerdeps@3.0.3 eslint-config-airbnb --dev
Need to install the following packages:
  install-peerdeps@3.0.3
Ok to proceed? (y) y
npm WARN deprecated @babel/polyfill@7.12.1: 🚨 This package has been deprecated in favor of separate inclusion of a polyfill and regenerator-runtime (when needed). See the @babel/polyfill docs (https://babeljs.io/docs/en/babel-polyfill) for more information.
npm WARN deprecated core-js@2.6.12: core-js@<3 is no longer maintained and not recommended for usage due to the number of issues. Please, upgrade your dependencies to the actual version of core-js@3.
install-peerdeps v3.0.3
It seems as if you are using Yarn. Would you like to use Yarn for the installation? (y/n) y
Installing peerdeps for eslint-config-airbnb@latest.
yarn add eslint-config-airbnb@18.2.1 eslint@^7.2.0 eslint-plugin-import@^2.22.1 eslint-plugin-jsx-a11y@^6.4.1 eslint-plugin-react@^7.21.5 eslint-plugin-react-hooks@^1.7.0 --dev

SUCCESS eslint-config-airbnb
  and its peerDeps were installed successfully.

karlhorky · 2021-01-29T17:05:13Z

Ran on Windows 10 too: same result.

3.0.2 (fails with ERR undefined)

$ npx install-peerdeps@3.0.2 eslint-config-airbnb --dev
Need to install the following packages:
  install-peerdeps@3.0.2
Ok to proceed? (y) y
npm WARN deprecated @babel/polyfill@7.12.1: :rotating_light: This package has been deprecated in favor of separate inclusion of a polyfill
 and regenerator-runtime (when needed). See the @babel/polyfill docs (https://babeljs.io/docs/en/babel-polyfill) for more information.
npm WARN deprecated core-js@2.6.12: core-js@<3 is no longer maintained and not recommended for usage due to the number of issues. Please, upgrade your dependencies to the actual version of core-js@3.
install-peerdeps v3.0.2
It seems as if you are using Yarn. Would you like to use Yarn for the installation? (y/n) y
Installing peerdeps for eslint-config-airbnb@latest.
yarn add "eslint-config-airbnb@18.2.1" "eslint@^7.2.0" "eslint-plugin-import@^2.22.1" "eslint-plugin-jsx-a11y@^6.4.1" "eslint-plugin-react@^7.21.5" "eslint-plugin-react-hooks@^1.7.0" --dev
ERR undefined
npm ERR! code 1
npm ERR! path D:\upleveled-softwork\repro
npm ERR! command failed
npm ERR! command C:\WINDOWS\system32\cmd.exe /d /s /c install-peerdeps eslint-config-airbnb --dev
npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\joseh\AppData\Local\npm-cache\_logs\2021-01-29T17_03_07_138Z-debug.log

3.0.3 (succeeds)

$ npx install-peerdeps@3.0.3 eslint-config-airbnb --dev
Need to install the following packages:
  install-peerdeps@3.0.3
Ok to proceed? (y) y
npm WARN deprecated @babel/polyfill@7.12.1: :rotating_light: This package has been deprecated in favor of separate inclusion of a polyfill
 and regenerator-runtime (when needed). See the @babel/polyfill docs (https://babeljs.io/docs/en/babel-polyfill) for more information.
npm WARN deprecated core-js@2.6.12: core-js@<3 is no longer maintained and not recommended for usage due to the number of issues. Please, upgrade your dependencies to the actual version of core-js@3.
install-peerdeps v3.0.3
It seems as if you are using Yarn. Would you like to use Yarn for the installation? (y/n) y
Installing peerdeps for eslint-config-airbnb@latest.
yarn add eslint-config-airbnb@18.2.1 eslint@^7.2.0 eslint-plugin-import@^2.22.1 eslint-plugin-jsx-a11y@^6.4.1 eslint-plugin-react@^7.21.5 eslint-plugin-react-hooks@^1.7.0 --dev
SUCCESS eslint-config-airbnb
  and its peerDeps were installed successfully.

karlhorky · 2021-01-29T17:10:22Z

So it looks like quoting the versions is not going to be a working solution for @zdm's #64 ... at least not as they are in fa0b30e

nathanhleung · 2021-01-29T17:34:07Z

Thanks for testing all this for me! Reopening #64.

karlhorky · 2021-02-04T18:04:24Z

Ah seems like this solution for version ranges doesn't work after all (installing the eslint-config-react-app peer deps in a new create-react-app project with install-peerdeps@3.0.3 just failed on Windows).

You can see this with this example dependency:

`eslint-config-react-app`

    "@typescript-eslint/parser": "^4.0.0",

Source: https://unpkg.com/browse/eslint-config-react-app@6.0.0/package.json

Installed via `install-peerdeps@3.0.3`

    "@typescript-eslint/parser": "4.0.0",

Latest version of `@typescript-eslint/parser`

4.14.2

Source: https://www.npmjs.com/package/@typescript-eslint/parser

Let me know if I should open a new issue for this.

valeriirivne · 2021-02-12T14:20:40Z

Hey, I have a a package.json file, created one with npm init. But I failed to install everything needed by the config: npx install-peerdeps --dev eslint-config-wesbos. What am I doing wrong? Thank you.
node v15.8.0, npm v7.5.1

nathanhleung · 2021-02-13T18:51:10Z

Hi @valeriirivne, thanks for the report. For now, can you try copying the npm install command that was outputted?

(copy the log line that starts with npm install and ends with --save-dev)

valeriirivne · 2021-02-13T22:56:34Z

Hi, @nathanhleung , do you mean the output after I run the command npm install?

Please help, how can I solve this issue? I tried 10times and the same error after I try to install dependencies. Thank you

karlhorky · 2021-02-14T15:50:43Z

I think Nathan means the line in the first screenshot you posted, the line that starts with npm install eslint-config-wesbos@1.0.1 babel-eslint@^10.1.0 ...

valeriirivne · 2021-02-14T20:47:45Z

Sorry, npx install-peerdeps --dev eslint-config-wesbos. I took it from here https://github.com/wesbos/eslint-config-wesbos --- "Local / Per Project Install". I do exactly like is written there, but this bug doesn't allow me to load that dependency. Can somebody help me how can I solve that issue? Thanks

ljharb · 2021-02-14T21:42:02Z

@valeriirivne are you perhaps using npm 7, and perhaps you have a peer dep incompatibility?

valeriirivne · 2021-02-15T09:32:46Z

@ljharb yes, npm7. Please, give a hint, how can I solve that issue and install this dependenicies? Thank you

ljharb · 2021-02-15T15:35:15Z

It’s not related to this project - it means you already have a version of eslint, or one of those eslint plugins, that’s installed or required as a peer dep by another package, and they’re incompatible. upgrade to npm v7.5.4, and try installing the deps manually with npm, and it might give you more info.

valeriirivne · 2021-02-15T16:57:18Z

@ljharb Thanks for reply. I will try like you said.

fix: don't coerce version strings to semver. resolves #71

12dfcf8

nathanhleung requested a review from ljharb January 29, 2021 03:39

ljharb reviewed Jan 29, 2021

View reviewed changes

nathanhleung mentioned this pull request Jan 29, 2021

Incorrect selection of package version #71

Closed

fix: quote package names, resolves #64

fa0b30e

nathanhleung force-pushed the nathanhleung-version-fix branch from 3d3ee0b to fa0b30e Compare January 29, 2021 03:48

nathanhleung requested a review from ljharb January 29, 2021 03:50

ljharb approved these changes Jan 29, 2021

View reviewed changes

nathanhleung merged commit c85d48c into master Jan 29, 2021

nathanhleung deleted the nathanhleung-version-fix branch January 29, 2021 15:54

nathanhleung mentioned this pull request Jan 29, 2021

Doesn't work with create-react-app #63

Closed

karlhorky mentioned this pull request Jan 29, 2021

ERR undefined with v3.0.1 when running on Windows 10 #128

Open

nathanhleung mentioned this pull request Jan 29, 2021

Quote packages name, when calling npm #64

Closed

karlhorky mentioned this pull request Mar 31, 2021

Move tests to GitHub Actions, add Windows Tests #127

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't coerce version strings to semver, quote package names #126

Don't coerce version strings to semver, quote package names #126

nathanhleung commented Jan 29, 2021

ljharb Jan 29, 2021

nathanhleung Jan 29, 2021

ljharb Jan 29, 2021

karlhorky commented Jan 29, 2021

karlhorky commented Jan 29, 2021 •

edited

Loading

nathanhleung commented Jan 29, 2021

nathanhleung commented Jan 29, 2021

nathanhleung commented Jan 29, 2021 •

edited

Loading

karlhorky commented Jan 29, 2021

karlhorky commented Jan 29, 2021

karlhorky commented Jan 29, 2021

karlhorky commented Jan 29, 2021

karlhorky commented Jan 29, 2021

nathanhleung commented Jan 29, 2021

karlhorky commented Feb 4, 2021 •

edited

Loading

valeriirivne commented Feb 12, 2021 •

edited

Loading

nathanhleung commented Feb 13, 2021

valeriirivne commented Feb 13, 2021

karlhorky commented Feb 14, 2021

valeriirivne commented Feb 14, 2021

ljharb commented Feb 14, 2021

valeriirivne commented Feb 15, 2021

ljharb commented Feb 15, 2021

valeriirivne commented Feb 15, 2021

Don't coerce version strings to semver, quote package names #126

Don't coerce version strings to semver, quote package names #126

Conversation

nathanhleung commented Jan 29, 2021

Previous Behavior

New Behavior

ljharb Jan 29, 2021

Choose a reason for hiding this comment

nathanhleung Jan 29, 2021

Choose a reason for hiding this comment

ljharb Jan 29, 2021

Choose a reason for hiding this comment

karlhorky commented Jan 29, 2021

karlhorky commented Jan 29, 2021 • edited Loading

nathanhleung commented Jan 29, 2021

nathanhleung commented Jan 29, 2021

nathanhleung commented Jan 29, 2021 • edited Loading

karlhorky commented Jan 29, 2021

karlhorky commented Jan 29, 2021

karlhorky commented Jan 29, 2021

karlhorky commented Jan 29, 2021

karlhorky commented Jan 29, 2021

nathanhleung commented Jan 29, 2021

karlhorky commented Feb 4, 2021 • edited Loading

eslint-config-react-app

Installed via install-peerdeps@3.0.3

Latest version of @typescript-eslint/parser

valeriirivne commented Feb 12, 2021 • edited Loading

nathanhleung commented Feb 13, 2021

valeriirivne commented Feb 13, 2021

karlhorky commented Feb 14, 2021

valeriirivne commented Feb 14, 2021

ljharb commented Feb 14, 2021

valeriirivne commented Feb 15, 2021

ljharb commented Feb 15, 2021

valeriirivne commented Feb 15, 2021

karlhorky commented Jan 29, 2021 •

edited

Loading

nathanhleung commented Jan 29, 2021 •

edited

Loading

karlhorky commented Feb 4, 2021 •

edited

Loading

`eslint-config-react-app`

Installed via `install-peerdeps@3.0.3`

Latest version of `@typescript-eslint/parser`

valeriirivne commented Feb 12, 2021 •

edited

Loading