npm / cli

the package manager for JavaScript
https://docs.npmjs.com/cli/
Other
8.34k stars 3.07k forks source link

[BUG] npm 8 allows incorrect peer dependencies when upgrading a v1 lockfile to v2. #5051

Open everett1992 opened 2 years ago

everett1992 commented 2 years ago

Is there an existing issue for this?

This issue exists in the latest npm version

Current Behavior

When npm 8 installs from a v1 lockfile it does not error on incorrect peer dependencies. It doesn't print any peer dependency warnings and exits successfully.

Expected Behavior

I expect npm 8 to error on invalid peer dependencies.

Steps To Reproduce

  1. Create a npm package with incorrect peer dependencies. ts-node depends on typescript@>=2.7, I choose this arbitrarily.

    {
    "dependencies": {
    "typescript": "1.8.0",
    "ts-node": "9.1.1"
    }
    }
  2. Run npm 6 install to create a v1 lockfile.

    
    $ npx npm@6 install
    npm notice created a lockfile as package-lock.json. You should commit this file.
    npm WARN ts-node@9.1.1 requires a peer of typescript@>=2.7 but none is installed. You must install peer dependencies yourself.

added 10 packages from 44 contributors in 2.371s

> exit 0

2. run npm 8 (8.12.2 at time of writing) to update to a v2 lockfile.

$ npx npm@8 install npm WARN old lockfile npm WARN old lockfile The package-lock.json file was created with an old version of npm, npm WARN old lockfile so supplemental metadata must be fetched from the registry. npm WARN old lockfile npm WARN old lockfile This is a one-time fix-up, please be patient... npm WARN old lockfile

up to date in 2s

> exit 0.

This is unexpected. I expect npm 8 to error on incorrect peer dependencies. A subsequent install _will_ error

$ npx npm@8 install npm ERR! code ERESOLVE npm ERR! ERESOLVE could not resolve


> exit 1

### Environment

- npm: 8.12.2
- Node.js: 16.15.1
- OS Name: Ubuntu
- npm config: none
everett1992 commented 2 years ago

I think this block needs to be repeated (or delayed) till after the lockfile is inflated. There is no edge from ts-node to typescript when we first check for invalid edges in _initTree.

https://github.com/npm/cli/blob/latest/workspaces/arborist/lib/arborist/build-ideal-tree.js#L361-L375 https://github.com/npm/cli/blob/latest/workspaces/arborist/lib/arborist/build-ideal-tree.js#L803

siemhesda commented 9 months ago

Having tested upgrades from npm versions, (npm@6 > npm@7 > npm@8 > npm@9 > npm@10) the only issue is upgrade from npm@6. The v1 lock files were used in npm@6. Since the current version is npm@10, it would seem that this issue is outdated.

siemhesda commented 8 months ago

TL;DR: The behavior being observed here is actually expected because in that step (installing v10 on a v6 lockfile), the lockfile is still old (v1) and is only just being upgraded. npm actually tells you it is a v1 lockfile and is being upgraded. A subsequent install will behave correctly.

Following the steps to reproduce, it's clear why npm cannot produce the desired output. Follow along ...

Step 1

Create an npm package with incorrect peer dependencies. ts-node depends on typescript@>=2.7, I choose this arbitrarily.

{
  "dependencies": {
    "typescript": "1.8.0",
    "ts-node": "9.1.1"
  }
}
Step 2

Run npm 6 install to create a v1 lockfile.

$ npx npm@6 install

npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN ts-node@9.1.1 requires a peer of typescript@>=2.7 but none is installed. You must install peer dependencies yourself.
npm WARN npmtest No description
npm WARN npmtest No repository field.
npm WARN npmtest No license field.

added 10 packages from 44 contributors and audited 10 packages in 3.284s
found 0 vulnerabilities
Step 3

Run npm 10 (10.2.5 at time of writing) to update to a v2 lockfile.

$ npx npm@10 install

npm WARN old lockfile
npm WARN old lockfile The package-lock.json file was created with an old version of npm,
npm WARN old lockfile so supplemental metadata must be fetched from the registry.
npm WARN old lockfile
npm WARN old lockfile This is a one-time fix-up, please be patient...
npm WARN old lockfile

up to date, audited 11 packages in 4s

found 0 vulnerabilities

[!IMPORTANT] This is actually expected because in this step, the lockfile is still old and is only just being upgraded. A subsequent install will behave correctly. Since npm actually warns you of an old lockfile, this shouldn't be considered a bug but just how npm works. Instead, the warning should inform you that v1 lockfile will not capture incorrect dependancies and you need to do a second npm install with the new lockfile. Something like:

npm WARN old lockfile V1 lockfiles do not catch incorrect dependencies. Instead,
npm WARN old lockfile run install again to catch them on the generated v2 lockfile

npx npm@10 install

npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR!
npm ERR! While resolving: ts-node@9.1.1
npm ERR! Found: typescript@1.8.0
npm ERR! node_modules/typescript
npm ERR!   typescript@"1.8.0" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer typescript@">=2.7" from ts-node@9.1.1
npm ERR! node_modules/ts-node
npm ERR!   ts-node@"9.1.1" from the root project
npm ERR!
npm ERR! Conflicting peer dependency: typescript@5.3.3
npm ERR! node_modules/typescript
npm ERR!   peer typescript@">=2.7" from ts-node@9.1.1
npm ERR!   node_modules/ts-node
npm ERR!     ts-node@"9.1.1" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR!
npm ERR! For a full report see:
npm ERR! /Users/mbaloo/.npm/_logs/2023-12-19T18_54_52_359Z-eresolve-report.txt

npm ERR! A complete log of this run can be found in: /Users/mbaloo/.npm/_logs/2023-12-19T18_54_52_359Z-debug-0.log