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

Incorrect selection of package version #71

Closed
erhhung opened this issue Feb 27, 2020 · 12 comments · Fixed by #126
Closed

Incorrect selection of package version #71

erhhung opened this issue Feb 27, 2020 · 12 comments · Fixed by #126
Labels

Comments

@erhhung
Copy link

erhhung commented Feb 27, 2020

I have an eslint-config-myconfig package in a private registry having the following peer dependencies:

  "peerDependencies": {
    "@typescript-eslint/eslint-plugin": ">=2.21 <3",
    "@typescript-eslint/parser": ">=2.21 <3",
    "eslint": ">=6.8 <7",
    "eslint-config-airbnb-typescript": ">=7 <8",
    "eslint-config-prettier": ">=6.10 <7",
    "eslint-plugin-import": ">=2.20 <3",
    "eslint-plugin-prettier": ">=3.1 <4",
    "eslint-plugin-security": ">=1.4 <2",
    "prettier": ">=1.19 <2",
    "typescript": ">=3.8 <4"
  },

When I run install-peerdeps, I'm getting a "no matching version" error:

npm ERR! code ETARGET
npm ERR! notarget No matching version found for typescript@3.8.0.
npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! notarget a package version that doesn't exist.

It appears that install-peerdeps is not using the exact same semver expression when issuing the "npm install" command. So, instead of npm install typescript@">=3.8 <4", it's just reading the first literal a.b.c number.

A quick debug shows the command being executed:

npm [
  'install',
  'eslint-config-insight@0.1.0',
  '@typescript-eslint/eslint-plugin@2.21.0',
  '@typescript-eslint/parser@2.21.0',
  'eslint@6.8.0',
  'eslint-config-airbnb-typescript@7.0.0',
  'eslint-config-prettier@6.10.0',
  'eslint-plugin-import@2.20.0',
  'eslint-plugin-prettier@3.1.0',
  'eslint-plugin-security@1.4.0',
  'prettier@1.19.0',
  'typescript@3.8.0',
  '--save-dev'
]
@ljharb
Copy link
Collaborator

ljharb commented Feb 27, 2020

You could use ^3.8 for that tho, any reason you’re not?

@erhhung
Copy link
Author

erhhung commented Feb 27, 2020

@ljharb I think ^3.8 would still get parsed into @3.8.0, which is a non-existent version. Regardless of the semver syntax, my point is that this tool should handle whatever that is considered valid in a package.json file.

@erhhung
Copy link
Author

erhhung commented Feb 27, 2020

BTW, my workaround to this issue seems to completely obviate the need for install-peerdeps:

npm info <package> peerDependencies --json | jq -r 'keys[] as $k | "\($k)@\"\(.[$k])\""' | xargs npm install --save-dev

@ljharb
Copy link
Collaborator

ljharb commented Feb 27, 2020

No, it wouldn’t - the caret is an identical (and simpler and more widely understood) range than the one you have.

@ocean90
Copy link

ocean90 commented Jul 6, 2020

I think I have a similar case where ^7 is transformed to 7.0.0 instead of the latest in the 7.x branch.

Example:

❯ npx install-peerdeps --dev @wordpress/eslint-plugin --dry-run
install-peerdeps v2.0.3
This command would have been run to install @wordpress/eslint-plugin@latest:
npm install @wordpress/eslint-plugin@7.1.0 eslint@7.0.0 --save-dev --registry https://registry.npmjs.org

Source: https://github.com/WordPress/gutenberg/blob/acc6dbea78ab3b8dd5436a57bc0c2d4808ec6756/packages/eslint-plugin/package.json#L44

I'm expecting to get eslint@7.4.0 here. Is that expectation incorrect?

@ljharb
Copy link
Collaborator

ljharb commented Jul 6, 2020

I would expect the npm install command to be @^7 and not @7.0.0 or @7.4.0 - that seems like a bug.

@karlhorky
Copy link

Yeah, I believe this is a bug - I think it may be causing facebook/create-react-app#10465 reported by @uebriges.

@karlhorky
Copy link

Just tested with @uebriges, and it seems this is indeed a problem.

We tested on a Windows 10 machine - not sure if this is a relevant detail.

Failing Repro

Reproduction 1 uses install-peerdeps to install the peer dependencies of eslint-config-react-app. When react-scripts start is run using yarn start, create-react-app crashes, saying that the versions of babel-eslint and eslint do not match the version from react-scripts.

mkdir reproduction
cd reproduction
npx create-react-app .
npx install-peerdeps --yarn --dev eslint-config-react-app
yarn start

Succeeding Repro

Reproduction 2 uses a manual yarn add --dev installation command to install the version ranges from the package.json for eslint-config-react-app. The start script runs without any problems:

mkdir reproduction-two
cd reproduction-two
npx create-react-app .
yarn add --dev eslint-config-react-app @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-jsx-a11y@^6.3.1 eslint-plugin-react@^7.20.3 eslint-plugin-react-hooks@^4.0.8 eslint-plugin-jest@^24.0.0
yarn start

@nathanhleung
Copy link
Owner

@karlhorky really appreciate your testing here, glad we're squashing all these bugs! Opened #126 which should hopefully fix -- can you let me know if that works for you?

@karlhorky
Copy link

Sure! @uebriges and I will test: #126 (comment)

@karlhorky
Copy link

karlhorky commented Feb 4, 2021

Ah seems like this solution in #126 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.

@zhiqingchen
Copy link

same problem on a Windows 11 machine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants