npm / cli

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

[BUG] Dependencies with local file dependencies result in invalid lockfiles #6860

Open cascornelissen opened 1 year ago

cascornelissen commented 1 year ago

Is there an existing issue for this?

This issue exists in the latest npm version

Current Behavior

Installing a dependency that contains a local file dependency works but results in an invalid (?) package-lock.json. Installing the same dependency again results in the following error:

npm ERR! Cannot set properties of null (setting 'peer')

Expected Behavior

Installing and re-installing/updating dependencies that contain local file dependencies works without issues.

Steps To Reproduce

I've set up an example package at cascornelissen/test-npm-package. All it contains is a single local file dependency that's in the subdirectory of the repository.

  1. Setup: mkdir ./test-npm && cd ./test-npm && npm init -y
  2. Install the dependency the first time (this works): npm install github:cascornelissen/test-npm-package
  3. Install the same dependency again (this fails): npm install github:cascornelissen/test-npm-package

install-links The steps above assume the install-links option is set to false (which is the current default value). It does work when executing the npm install command with --install-links=true.

Error The stack trace (included below) refers to this line in calc-dep-flags.js where it's assumed that node.target exists but it does not in this scenario. I've tried patching in a check to make sure node.target exists but this happens in at least 5 different places, both in this file and in other files, so my assumption then became that the target should exist.

Stack trace
0 verbose cli /opt/homebrew/Cellar/node/20.5.0/bin/node /opt/homebrew/bin/npm
1 info using npm@10.1.0
2 info using node@v20.5.0
3 timing npm:load:whichnode Completed in 0ms
4 timing config:load:defaults Completed in 0ms
5 timing config:load:file:/opt/homebrew/lib/node_modules/npm/npmrc Completed in 1ms
6 timing config:load:builtin Completed in 1ms
7 timing config:load:cli Completed in 1ms
8 timing config:load:env Completed in 0ms
9 timing config:load:file:/Users/cas/projects/test-npm/.npmrc Completed in 0ms
10 timing config:load:project Completed in 1ms
11 timing config:load:file:/Users/cas/.npmrc Completed in 0ms
12 timing config:load:user Completed in 0ms
13 timing config:load:file:/opt/homebrew/etc/npmrc Completed in 0ms
14 timing config:load:global Completed in 0ms
15 timing config:load:setEnvs Completed in 1ms
16 timing config:load Completed in 4ms
17 timing npm:load:configload Completed in 4ms
18 timing config:load:flatten Completed in 1ms
19 timing npm:load:mkdirpcache Completed in 0ms
20 timing npm:load:mkdirplogs Completed in 0ms
21 verbose title npm i github:cascornelissen/test-npm-package
22 verbose argv "i" "github:cascornelissen/test-npm-package" "--install-links" "false"
23 timing npm:load:setTitle Completed in 2ms
24 timing npm:load:display Completed in 0ms
25 verbose logfile logs-max:10 dir:/Users/cas/.npm/_logs/2023-10-03T08_25_44_046Z-
26 verbose logfile /Users/cas/.npm/_logs/2023-10-03T08_25_44_046Z-debug-0.log
27 timing npm:load:logFile Completed in 5ms
28 timing npm:load:timers Completed in 0ms
29 timing npm:load:configScope Completed in 0ms
30 timing npm:load Completed in 20ms
31 timing arborist:ctor Completed in 1ms
32 timing idealTree:init Completed in 7ms
33 silly logfile start cleaning logs, removing 1 files
34 timing arborist:ctor Completed in 0ms
35 silly logfile done cleaning log files
36 timing idealTree:userRequests Completed in 1100ms
37 silly idealTree buildDeps
38 silly fetch manifest test-npm-package@github:cascornelissen/test-npm-package
39 silly placeDep ROOT test-npm-package@ REPLACE for: test-npm@1.0.0 want: github:cascornelissen/test-npm-package
40 timing idealTree:#root Completed in 666ms
41 timing idealTree:node_modules/test-npm-package Completed in 0ms
42 timing idealTree:buildDeps Completed in 668ms
43 timing idealTree Completed in 1775ms
44 timing command:i Completed in 1778ms
45 verbose stack TypeError: Cannot set properties of null (setting 'peer')
45 verbose stack     at visit (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/calc-dep-flags.js:101:54)
45 verbose stack     at visitNode (/opt/homebrew/lib/node_modules/npm/node_modules/treeverse/lib/depth-descent.js:58:25)
45 verbose stack     at next (/opt/homebrew/lib/node_modules/npm/node_modules/treeverse/lib/depth-descent.js:44:19)
45 verbose stack     at depth (/opt/homebrew/lib/node_modules/npm/node_modules/treeverse/lib/depth-descent.js:83:10)
45 verbose stack     at depth (/opt/homebrew/lib/node_modules/npm/node_modules/treeverse/lib/depth.js:27:12)
45 verbose stack     at unsetFlag (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/calc-dep-flags.js:96:5)
45 verbose stack     at /opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/calc-dep-flags.js:63:7
45 verbose stack     at Map.forEach ()
45 verbose stack     at calcDepFlagsStep (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/calc-dep-flags.js:41:17)
45 verbose stack     at visit (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/calc-dep-flags.js:12:20)
46 verbose cwd /Users/cas/projects/test-npm
47 verbose Darwin 22.6.0
48 verbose node v20.5.0
49 verbose npm  v10.1.0
50 error Cannot set properties of null (setting 'peer')
51 verbose exit 1
52 timing npm Completed in 1924ms
53 verbose unfinished npm timer reify 1696321544192
54 verbose unfinished npm timer reify:loadTrees 1696321544193
55 verbose unfinished npm timer idealTree:fixDepFlags 1696321545968
56 verbose code 1
57 error A complete log of this run can be found in: /Users/cas/.npm/_logs/2023-10-03T08_25_44_046Z-debug-0.log

Lockfile Looking into the lockfile I noticed there's a package listed at node_modules/test-npm-package/subdependency which contains no data.

"node_modules/test-npm-package": {
  "resolved": "git+ssh://git@github.com/cascornelissen/test-npm-package.git#0ae7cc600e142043e0ad819dcc045c4a2a9899ec",
  "dependencies": {
    "test-npm-package-subdependency": "file:subdependency"
  }
},
"node_modules/test-npm-package-subdependency": {
  "resolved": "node_modules/test-npm-package/subdependency",
  "link": true
},
"node_modules/test-npm-package/subdependency": {}

For good measure, here's what the same part of the lockfile looks like with --install-links set to true.

"node_modules/test-npm-package": {
  "resolved": "git+ssh://git@github.com/cascornelissen/test-npm-package.git#0ae7cc600e142043e0ad819dcc045c4a2a9899ec",
  "dependencies": {
    "test-npm-package-subdependency": "file:subdependency"
  }
},
"node_modules/test-npm-package-subdependency": {
  "resolved": "file:node_modules/test-npm-package/subdependency"
}

Conclusion To me it feels like there's an issue in the generation of the lockfile, node_modules/test-npm-package/subdependency should be pointing to something, no? I'll investigate a bit further but wanted to create this ticket already, since maybe someone with more contextual knowledge has an answer.

Related issues

Environment

prefix = "/opt/homebrew"

; "user" config from /Users/cas/.npmrc

; node bin location = /opt/homebrew/Cellar/node/20.5.0/bin/node ; node version = v20.5.0 ; npm local prefix = /Users/cas/projects/test-npm ; npm version = 10.1.0 ; cwd = /Users/cas/projects/test-npm ; HOME = /Users/cas ; Run npm config ls -l to show all defaults.

cascornelissen commented 1 year ago

After going down this rabbit hole for a bit, I've found that commenting out the following line results in an install that passes. Not sure why just yet, but I did want to document the end of this rabbit hole 🐇

https://github.com/npm/cli/blob/aa6728b1d003f0fc620b074ba0396a3e07f2db6a/workspaces/arborist/lib/node.js#L1178

The node.replace() method is called when placing the dependency on this line where I'm assuming it's trying to replace the old dependency with the new dependency.

https://github.com/npm/cli/blob/aa6728b1d003f0fc620b074ba0396a3e07f2db6a/workspaces/arborist/lib/place-dep.js#L387


I've also tried the different install-strategies:

hoisted (default) nested shallow linked (experimental)
First install 💥 fails on target being undefined at isolated-reifier.js#L405
Second install 💥
mknj commented 8 months ago

Similar behaviour without installing the same package twice. Tested with different node and npm versions on debian latest.

mkdir testingworkspaces
cd testingworkspaces
npm init -y
npm pkg set workspaces[]=appa
npm pkg set workspaces[]=appb
npm pkg set workspaces[]=lib1
npm pkg set workspaces[]=lib2
npm pkg set workspaces[]=sharedlib
npm pkg set scripts.buildA="npm run build -w sharedlib -w lib1 -w appb" # build appA and its deps
mkdir lib1
mkdir lib2
mkdir appa
mkdir appb
mkdir sharedlib
for x in lib* app* sharedlib ;do pushd $x;npm init -y;popd;done
cd appa
npm i ../lib1
cd ../appb
npm i ../lib2 # npm ERR! Cannot set properties of null (setting 'dev')