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] Dependencies with local file dependencies result in invalid lockfiles #6860

Open
2 tasks done
cascornelissen opened this issue Oct 3, 2023 · 2 comments
Open
2 tasks done
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 9.x work is associated with a specific npm 9 release Release 10.x

Comments

@cascornelissen
Copy link

cascornelissen commented Oct 3, 2023

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

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 [email protected]
2 info using [email protected]
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: [email protected] 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://[email protected]/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://[email protected]/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

  • npm: 10.1.0
  • Node.js: 20.5.0
  • OS Name: macOS Mojave (also confirmed on Windows 11 with WSL)
  • System Model Name: Macbook Pro
  • npm config:
; "builtin" config from /opt/homebrew/lib/node_modules/npm/npmrc

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 cascornelissen added Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x labels Oct 3, 2023
@cascornelissen
Copy link
Author

cascornelissen commented Oct 3, 2023

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 🐇

this.root = node.root

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.

this.placed.replace(this.oldDep)


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 💥

@lukekarrys lukekarrys added the Release 9.x work is associated with a specific npm 9 release label Oct 6, 2023
@mknj
Copy link

mknj commented Mar 5, 2024

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')

@milaninfy milaninfy added Priority 2 secondary priority issue and removed Needs Triage needs review for next steps labels Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 9.x work is associated with a specific npm 9 release Release 10.x
Projects
None yet
Development

No branches or pull requests

4 participants