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

[BUG] Some packages can no longer be installed with npm v7 #1734

Closed
debel27 opened this issue Aug 27, 2020 · 7 comments
Closed

[BUG] Some packages can no longer be installed with npm v7 #1734

debel27 opened this issue Aug 27, 2020 · 7 comments
Labels
Bug thing that needs fixing Release 7.x work is associated with a specific npm 7 release

Comments

@debel27
Copy link

debel27 commented Aug 27, 2020

Current Behavior:

Some packages can no longer be installed with NPM v7 (in my case, it's react-tooltip@3.11.6)

To me, the install fails for a pretty legitimate reason. Yet, this behavior is not backward compatible.

Here is the relevant log

npm verb stack Error: Invalid tag name ">=^16.0.0": Tags may not have any characters that encodeURIComponent encodes.
npm verb stack     at invalidTagName (/usr/lib/node_modules/npm/node_modules/npm-package-arg/npa.js:91:15)
npm verb stack     at fromRegistry (/usr/lib/node_modules/npm/node_modules/npm-package-arg/npa.js:296:13)
npm verb stack     at Function.resolve (/usr/lib/node_modules/npm/node_modules/npm-package-arg/npa.js:81:12)
npm verb stack     at Arborist.[nodeFromEdge] (/usr/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:680:22)
npm verb stack     at Promise.all.peerEdges.map.edge (/usr/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:778:48)
npm verb stack     at Array.map (<anonymous>)
npm verb stack     at Arborist.[loadPeerSet] (/usr/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:778:17)
npm verb stack     at (anonymous function).then.node (/usr/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:692:32)
npm verb stack     at process._tickCallback (internal/process/next_tick.js:68:7)
npm verb cwd /home/user/Desktop/test
npm verb Linux 3.13.0-24-generic
npm verb argv "/usr/bin/node" "/usr/bin/npm" "i" "react-tooltip@3.11.6" "-dd"
npm verb node v10.14.0
npm verb npm  v7.0.0-beta.7
npm ERR! code EINVALIDTAGNAME
npm ERR! Invalid tag name ">=^16.0.0": Tags may not have any characters that encodeURIComponent encodes.
npm verb exit 1

Expected Behavior:

npm install works flawlessly

Steps To Reproduce:

npm init -y
npm i react-tooltip@3.11.6
@debel27 debel27 added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Aug 27, 2020
@mjeanroy
Copy link
Contributor

Hi,

This issue is due to a bug in cache.js, line 83, the ternary expression seems wrong:

const spec = args[0] +
-     (args[1] === undefined || args[1] === null ? `@${args[1]}` : '')
+     (args[1] === undefined || args[1] === null ? '' : `@${args[1]}`)

I would be happy to submit a PR fixing the issue ;)

mjeanroy added a commit to mjeanroy/cli that referenced this issue Aug 28, 2020
@ruyadorno ruyadorno added beta-bug and removed Needs Triage needs review for next steps labels Sep 3, 2020
isaacs pushed a commit that referenced this issue Sep 4, 2020
Close #1734

PR-URL: #1738
Credit: @mjeanroy
Close: #1738
Reviewed-by: @isaacs

EDIT(@isaacs): Removed test, pending full test coverage of lib/cache.js
@ruyadorno ruyadorno reopened this Sep 4, 2020
@ruyadorno
Copy link
Contributor

note: I can still reproduce the issue in v7.0.0-beta.9 - we're def going to be following up with this one 😊

@mjeanroy
Copy link
Contributor

mjeanroy commented Sep 13, 2020

Hi @ruyadorno,

I made a mistake, my PR fixed a bug with npm cache add command and I made a mistake because the error message was the same.

I took the time to debug what happened here and it looks like react-tooltip@3.11.6 define react as a peer dependency with an invalid semver constraint: >=^16.0.0, instead it should be >=16.0.0 or ^16.0.0 (note that it has been fixed with react-tooltip@4.0.1 and the the peer dependency is now defined with >=16.0.0 constraint).

In this case, the tag name verification made by npm-package-args dependency fails here (both version and range variable are null, so the last else is executed).

I don't think this is really a bug since a non valid semver constraint was probably not officially supported with npm < 7, but the error message can probably be improved :)

@debel27
Copy link
Author

debel27 commented Sep 13, 2020

I don't think this is really a bug since a non valid semver constraint was probably not officially supported with npm < 7, but the error message can probably be improved :)

I disagree. Again, I believe the error is valid but it shouldn't be improved, it should be removed altogether.

Packages that were installable with NPM v6 are no longer installable with NPM v7. This means upgrading NPM may break projects. This might have been fine if there were a clear migration procedure, but in this case there is none. There may be packages whose latest version contains an invalid semver specifier, so the developer cannot upgrade its dependency to a higher version that fixes the problem.

@isaacs
Copy link
Contributor

isaacs commented Sep 22, 2020

So, the handling of this broken semver range is not actually a breaking change. What changed is that we handle peerDependencies at all, so any broken garbage in a peerDependencies set would have been overlooked in npm v6, and no longer is.

Observe:

$ cat package.json
{
  "dependencies": {
    "react": ">=^16.0.0"
  }
}

$ npm i     # <-- using latest v7 beta
npm ERR! code EINVALIDTAGNAME
npm ERR! Invalid tag name ">=^16.0.0": Tags may not have any characters that encodeURIComponent encodes.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/isaacs/.npm/_logs/2020-09-22T21_01_51_852Z-debug.log

$ node ~/dev/npm/cli-v6 i   # <-- my checkout of v6
npm ERR! code EINVALIDTAGNAME
npm ERR! Invalid tag name ">=^16.0.0": Tags may not have any characters that encodeURIComponent encodes.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/isaacs/.npm/_logs/2020-09-22T21_01_59_215Z-debug.log

A more extreme example:

$ cat package.json
{
  "peerDependencies": {
    "react": "this is not a valid semver or dist-tag !$@!#$%#@$%!@#$!@#$%"
  }
}

$ npm i
npm ERR! code EINVALIDTAGNAME
npm ERR! Invalid tag name "this is not a valid semver or dist-tag !$@!#$%#@$%!@#$!@#$%": Tags may not have any characters that encodeURIComponent encodes.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/isaacs/.npm/_logs/2020-09-22T21_06_30_064Z-debug.log

$ node ~/dev/npm/cli-v6 i
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN bad-dep-version requires a peer of react@this is not a valid semver or dist-tag !$@!#$%#@$%!@#$!@#$% but none is installed. You must install peer dependencies yourself.
npm WARN bad-dep-version No description
npm WARN bad-dep-version No repository field.
npm WARN bad-dep-version No license field.

up to date in 0.307s
found 0 vulnerabilities


$ npm i --legacy-peer-deps

up to date in 245ms

found 0 vulnerabilities

I'm tempted to say that this is working as designed. If you want npm v7 to work when peerDependencies contain invalid data, you can use the --legacy-peer-deps switch to tell npm v7 to not look at peerDependencies at all.

The only alternative here would be to catch any invalid data in peerDependencies (but only in peerDependencies), and ignore those peer deps. So then we'd be in a situation where the contract that we install peerDeps is violated.

Another alternative would be that we figure out what >=^16.0.0 means, and add support for that syntax in node-semver. But now we're playing whack-a-mole with whatever unfiltered garbage happened to land in that previously ignored field.

Yes, packages using this dependency will break, but in this case, I don't really know if the alternatives are reasonable. Hopefully the users of react-tooltip can update to the fixed version swiftly.

@mcqj
Copy link

mcqj commented May 24, 2021

Could the error message be improved to offer a suggestion on where to look? The error message is displayed without context. One would assume that the error is in the package.json of the current project, not in a dependency because it doesn't say it's in a dependency. If it told you which dependency, it would be helpful.

@jakecastelli
Copy link

@mcqj agreed, and I had the exact same issue and nearly drove me on the wall. Luckily it looks like you can search what it complains in the package-lock.json file and it will tell you what package is doing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

No branches or pull requests

6 participants