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] Cannot set property 'peer' of null #5007

Open
2 tasks done
minecrawler opened this issue Jun 10, 2022 · 48 comments
Open
2 tasks done

[BUG] Cannot set property 'peer' of null #5007

minecrawler opened this issue Jun 10, 2022 · 48 comments
Labels
Bug thing that needs fixing Priority 1 high priority issue

Comments

@minecrawler
Copy link

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

I can only hit this particular error in a certain directory with a certain package, but I can reproduce it over several npm versions and it's 100% of the time.

There are more examples available in #3711 (the author refuses to reopen the bug even though it is not fixed and people are hitting it!).

After having a working npm for years and not changing the config, I just ran into it on npm v8.1.4. Upgraded to v8.5.4 and the problem did not go away. Here's the log with the latest version (8.12.1):

0 verbose cli C:\Program Files\nodejs\node.exe C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js
1 info using npm@8.12.1
2 info using node@v17.2.0
3 timing npm:load:whichnode Completed in 0ms
4 timing config:load:defaults Completed in 1ms
5 timing config:load:file:C:\Users\<user>\AppData\Roaming\nvm\v17.2.0\node_modules\npm\npmrc Completed in 5ms
6 timing config:load:builtin Completed in 5ms
7 timing config:load:cli Completed in 1ms
8 timing config:load:env Completed in 1ms
9 timing config:load:file:C:\Users\<user>\tmp\.npmrc Completed in 1ms
10 timing config:load:project Completed in 8ms
11 timing config:load:file:C:\Users\<user>\.npmrc Completed in 3ms
12 timing config:load:user Completed in 3ms
13 timing config:load:file:C:\Program Files\nodejs\etc\npmrc Completed in 3ms
14 timing config:load:global Completed in 3ms
15 timing config:load:validate Completed in 2ms
16 timing config:load:credentials Completed in 1ms
17 timing config:load:setEnvs Completed in 1ms
18 timing config:load Completed in 29ms
19 timing npm:load:configload Completed in 30ms
20 timing npm:load:mkdirpcache Completed in 3ms
21 timing npm:load:mkdirplogs Completed in 3ms
22 verbose title npm i <package>
23 verbose argv "i" "<package>"
24 timing npm:load:setTitle Completed in 2ms
25 timing config:load:flatten Completed in 4ms
26 timing npm:load:display Completed in 4ms
27 verbose logfile logs-max:10 dir:C:\Users\<user>\AppData\Local\npm-cache\_logs
28 verbose logfile C:\Users\<user>\AppData\Local\npm-cache\_logs\2022-06-10T07_31_26_930Z-debug-0.log
29 timing npm:load:logFile Completed in 8ms
30 timing npm:load:timers Completed in 0ms
31 timing npm:load:configScope Completed in 0ms
32 timing npm:load Completed in 52ms
33 timing arborist:ctor Completed in 1ms
34 silly logfile start cleaning logs, removing 1 files
35 silly logfile done cleaning log files
36 timing idealTree:init Completed in 81ms
37 timing idealTree:userRequests Completed in 2ms
38 silly idealTree buildDeps
39 silly fetch manifest <package>
40 http fetch GET 200 <repository> 327ms (cache revalidated)
41 silly placeDep ROOT <package> REPLACE for:  want: <version>
42 timing idealTree:#root Completed in 349ms
43 timing idealTree:node_modules/<package> Completed in 0ms
44 timing idealTree:node_modules/<package>/vendor/angular Completed in 0ms
45 timing idealTree:buildDeps Completed in 351ms
46 timing idealTree Completed in 436ms
47 timing command:i Completed in 446ms
48 verbose stack TypeError: Cannot set properties of null (setting 'peer')
48 verbose stack     at visit (C:\Users\<user>\AppData\Roaming\nvm\v17.2.0\node_modules\npm\node_modules\@npmcli\arborist\lib\calc-dep-flags.js:104:54)
48 verbose stack     at visitNode (C:\Users\<user>\AppData\Roaming\nvm\v17.2.0\node_modules\npm\node_modules\treeverse\lib\depth-descent.js:58:25)
48 verbose stack     at next (C:\Users\<user>\AppData\Roaming\nvm\v17.2.0\node_modules\npm\node_modules\treeverse\lib\depth-descent.js:44:19)
48 verbose stack     at depth (C:\Users\<user>\AppData\Roaming\nvm\v17.2.0\node_modules\npm\node_modules\treeverse\lib\depth-descent.js:83:10)
48 verbose stack     at depth (C:\Users\<user>\AppData\Roaming\nvm\v17.2.0\node_modules\npm\node_modules\treeverse\lib\depth.js:27:12)
48 verbose stack     at unsetFlag (C:\Users\<user>\AppData\Roaming\nvm\v17.2.0\node_modules\npm\node_modules\@npmcli\arborist\lib\calc-dep-flags.js:99:5)
48 verbose stack     at C:\Users\<user>\AppData\Roaming\nvm\v17.2.0\node_modules\npm\node_modules\@npmcli\arborist\lib\calc-dep-flags.js:65:7
48 verbose stack     at Map.forEach (<anonymous>)
48 verbose stack     at calcDepFlagsStep (C:\Users\<user>\AppData\Roaming\nvm\v17.2.0\node_modules\npm\node_modules\@npmcli\arborist\lib\calc-dep-flags.js:41:17)
48 verbose stack     at visit (C:\Users\<user>\AppData\Roaming\nvm\v17.2.0\node_modules\npm\node_modules\@npmcli\arborist\lib\calc-dep-flags.js:12:20)
49 verbose cwd C:\Users\<user>\tmp
50 verbose Windows_NT 10.0.19042
51 verbose node v17.2.0
52 verbose npm  v8.12.1
53 error Cannot set properties of null (setting 'peer')
54 verbose exit 1
55 timing npm Completed in 1259ms
56 verbose unfinished npm timer reify 1654846300239
57 verbose unfinished npm timer reify:loadTrees 1654846300299
58 verbose unfinished npm timer idealTree:fixDepFlags 1654846306426
59 verbose code 1
60 error A complete log of this run can be found in:
60 error     C:\Users\<user>\AppData\Local\npm-cache\_logs\2022-05-30T07_24_36_788Z-debug-0.log

I redacted the user and package name as they are company internals. I use a company artifactory with npm registry. However, this has worked for me before and is working for thousands of our developers as we speak, so I think it's a local problem with my npm.

Expected Behavior

npm i <package> installs the package successfully

Steps To Reproduce

  1. In this environment...
  2. With this config...
  3. Run '...'
  4. See error...

Environment

  • npm: v8.12.1
  • Node.js: v17.2.0
  • OS Name: Windows 10 x64
  • npm config:
; copy and paste output from `npm config ls` here

; "user" config from C:\Users\<user>\.npmrc

registry = "<registry>"
strict-ssl = false

; node bin location = C:\Program Files\nodejs\node.exe
; node version = v17.2.0
; npm local prefix = C:\Users\<user>\tmp
; npm version = 8.12.1
; cwd = C:\Users\<user>\tmp
; HOME = C:\Users\<user>
; Run `npm config ls -l` to show all defaults
@minecrawler minecrawler added Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release labels Jun 10, 2022
@gthb
Copy link

gthb commented Jun 21, 2022

Specific steps to reproduce:

git clone git@github.com:oguimbal/pgsql-ast-parser.git
cd pgsql-ast-parser
npm i; npm run build # this makes a dist build with a `package.json` in ./lib
cd ..
mkdir npm-cli-issue-5007-repro
cd npm-cli-issue-5007-repro
npm init -y
npm i -S pg-mem ../pgsql-ast-parser/lib

... then edit package.json to insert this:

  "overrides": {
    "pg-mem": {
      "pgsql-ast-parser": "$pgsql-ast-parser"
    }
  }

... then:

npm i
npm i # yes, again

The second (and any subsequent) npm i execution fails as shown above. Specifically, the unsetFlag node visitor encounters this node which has target: null:

ArboristLink {
  name: 'pgsql-ast-parser',
  packageName: 'lib',
  location: 'node_modules/pg-mem/node_modules/pgsql-ast-parser',
  path: '/Users/gthb/git/npm-cli-issue-5007-repro/node_modules/pg-mem/node_modules/pgsql-ast-parser',
  realpath: '/Users/gthb/git/npm-cli-issue-5007-repro/node_modules/pgsql-ast-parser/lib',
  resolved: 'file:../../pgsql-ast-parser/lib',
  dev: true,
  optional: true,
  overrides: Map(2) { 'pgsql-ast-parser' => '$pgsql-ast-parser', 'pg-mem' => '' },
  edgesIn: Set(1) { { node_modules/pg-mem prod pgsql-ast-parser@^10.5.2 } },
  target: null
}

There seems to be a lot going wrong there :-) ... (not all of it necessarily relevant to this bug, but some of it for sure):

  • the packageName is 'lib', presumably the last path element of the path I specified ... but that lib folder contains a package.json with "name": "pgsql-ast-parser" — should that not be the packageName value?
  • the location is 'node_modules/pg-mem/node_modules/pgsql-ast-parser' and that path does exist ... but it's a symlink pointing to ../../pgsql-ast-parser/lib which resolves to node_modules/pgsql-ast-parser/lib
  • ... and sure enough, the realpath is '/Users/gthb/git/npm-cli-issue-5007-repro/node_modules/pgsql-ast-parser/lib'
  • ... but that path doesn't exist, because node_modules/pgsql-ast-parser is populated out of the lib folder, it doesn't contain a lib folder.
  • dev and optional are true, but both this project's dependency, and pg-mem's dependency, on pgsql-ast-parser, are not dev dependencies and not optional.
  • overrides contains a second entry 'pg-mem' => '' ... what's that about? (Maybe it does make sense for some internal reason, or maybe it's part of the problem)
  • and of course, target is null (maybe because of that broken symlink?), while unsetFlag evidently assumes it can't be null

@gthb
Copy link

gthb commented Jun 22, 2022

The package-lock.json file contains:

    "node_modules/pg-mem/node_modules/pgsql-ast-parser": {
      "resolved": "node_modules/pgsql-ast-parser/lib",
      "link": true
    },

and if I manually change that "resolved" attribute to replace node_modules/ with ../:

    "node_modules/pg-mem/node_modules/pgsql-ast-parser": {
      "resolved": "../pgsql-ast-parser/lib",
      "link": true
    },

then the next npm i invocation completes without (reported errors):

changed 1 package, and audited 245 packages in 1s

8 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

... but it changes the package-lock.json back, undoing my manual change, so if I run npm i again, it fails like before.

@thany
Copy link

thany commented Jul 5, 2022

Running into this exact issue as well, trying to install Jest.

42 verbose stack TypeError: Cannot set properties of null (setting 'peer')
42 verbose stack     at visit (C:\Users\<user>\AppData\Roaming\nvm\v16.15.1\node_modules\npm\node_modules\@npmcli\arborist\lib\calc-dep-flags.js:104:54)
42 verbose stack     at visitNode (C:\Users\<user>\AppData\Roaming\nvm\v16.15.1\node_modules\npm\node_modules\treeverse\lib\depth-descent.js:58:25)
42 verbose stack     at next (C:\Users\<user>\AppData\Roaming\nvm\v16.15.1\node_modules\npm\node_modules\treeverse\lib\depth-descent.js:44:19)
42 verbose stack     at depth (C:\Users\<user>\AppData\Roaming\nvm\v16.15.1\node_modules\npm\node_modules\treeverse\lib\depth-descent.js:83:10)
42 verbose stack     at depth (C:\Users\<user>\AppData\Roaming\nvm\v16.15.1\node_modules\npm\node_modules\treeverse\lib\depth.js:27:12)
42 verbose stack     at unsetFlag (C:\Users\<user>\AppData\Roaming\nvm\v16.15.1\node_modules\npm\node_modules\@npmcli\arborist\lib\calc-dep-flags.js:99:5)
42 verbose stack     at C:\Users\<user>\AppData\Roaming\nvm\v16.15.1\node_modules\npm\node_modules\@npmcli\arborist\lib\calc-dep-flags.js:65:7
42 verbose stack     at Map.forEach (<anonymous>)
42 verbose stack     at calcDepFlagsStep (C:\Users\<user>\AppData\Roaming\nvm\v16.15.1\node_modules\npm\node_modules\@npmcli\arborist\lib\calc-dep-flags.js:41:17)
42 verbose stack     at visit (C:\Users\<user>\AppData\Roaming\nvm\v16.15.1\node_modules\npm\node_modules\@npmcli\arborist\lib\calc-dep-flags.js:12:20)

Node 16.15.1 (latest LTS at the time of writing)
NPM 8.13.2 (latest at the time of writing)

The problem only seems to occur for me in a project with workspaces. Doesn't matter if I execute an install from the main package.json directory, or from any of the individual workspaces. However, MUCH MUCH WORSE is that just executing a straight npm i also breaks with this issue. So installing packages is not only broken for Jest, but is now completely knackered.

This desperately requires a solution.

In a project without workspaces set up, it works fine for me, but I can't just willy-nilly remove the workspaces feature from our projects for obvious reasons.

Edit: same results with NPM 8.11 and 8.10, so this bug has existed for at least a while. This should give it elevated priority to get it fixed, since downgrading NPM is unrealistic.

@thany
Copy link

thany commented Jul 5, 2022

For me the node where it breaks:

ArboristLink {
  name: 'renewi-campaign',
  version: '2.0.0',
  location: 'node_modules/renewi-campaign',
  path: 'C:\\Projects\\Renewi\\src\\VGW.Web\\Frontend\\node_modules\\renewi-campaign',
  realpath: 'c:\\Projects\\Renewi\\src\\VGW.Web\\Frontend\\Campaign',
  resolved: 'file:../Campaign',
  dev: true,
  optional: true,
  isWorkspace: true,
  edgesIn: Set(1) {
    { "" workspace renewi-campaign@file:C:\Projects\Renewi\src\VGW.Web\Frontend\Campaign }
  },
  target: null
}
  • This is a workspace link, as you can see.
  • path and realpath both exist.
  • I don't know what causes dev and optional to be true. This is a workspace, not a depedency.
  • And as said before, target is null.

I tried to monkey-patch the offending package by inserting various defenses to get node.target to be not null, but it always results in other errors indicating to missing or incorrect values within the target object (I have no way of knowing what it expects of course).

@thany
Copy link

thany commented Jul 5, 2022

Here's a workaround so that at least we can get back to work. In the arborist package, I've made these changes in the calc-dep-flags.js file:

@@ -30,7 +30,7 @@ const calcDepFlagsStep = (node) => {
   resetParents(node, 'optional')

   // for links, map their hierarchy appropriately
-  if (node.isLink) {
+  if (node.isLink && node.target) {
     node.target.dev = node.dev
     node.target.optional = node.optional
     node.target.devOptional = node.devOptional
@@ -100,11 +100,11 @@ const unsetFlag = (node, flag) => {
       tree: node,
       visit: node => {
         node.extraneous = node[flag] = false
-        if (node.isLink) {
+        if (node.isLink && node.target) {
           node.target.extraneous = node.target[flag] = false
         }
       },
-      getChildren: node => [...node.target.edgesOut.values()]
+      getChildren: node => [...(node.target?.edgesOut.values() ?? [])]
         .filter(edge => edge.to && edge.to[flag] &&
           (flag !== 'peer' && edge.type === 'peer' || edge.type === 'prod'))
         .map(edge => edge.to),

This makes installs go without errors, but of course I don't know if this is the right way to go. The root cause is the fact that target is null, or the fact that target is assumed (blindly, might I add) to be an object.

@BobFrankston
Copy link

running into this again and it's a big problem.

What is the status of implementing a fix?

@minecrawler
Copy link
Author

This bug is even still in triage, so no fix in sight :(

@stormmuller
Copy link

stormmuller commented Jul 25, 2022

I've been running into this now too.

We also have workspaces (single package-lock.json). This seems to work in linux (WSL more specifically) but not on windows.

I've tested on versions 8.15.0 and 8.11.0, both are broken.

@stormmuller
Copy link

stormmuller commented Jul 25, 2022

I reinstalled node and still had the same issue. Only once I deleted C:\Users\<user>\AppData\Local\npm-cache was I able to install my packages.

I'm not saying this is the right thing to do, or that anyone should do it. I'm not entirely sure what the side effects might be.

@thany
Copy link

thany commented Jul 28, 2022

Still the same. A new version has been released since I posted my diff with a fix, and it appears it hasn't been included. The bug still exists.

Let me reiterate how severe this is: we cannot install, update, or uninstall anything. We cannot even do an audit fix which is super important to be able to do!!

This DESPERATELY needs a proper fix.

Who can we tag to escalate this issue to critical/blocking priority?

@minecrawler
Copy link
Author

It seems that a lot of bugs remain in triage for a very long time (up to 10 months as far as I saw) while other issues are removed from triage within hours. Even when we are not in triage anymore I don't think that would guarantee a fast resolution. Especially since there are only a few people affected while millions don't have the bug.

So, I am not sure what the best way is with such a long bug tracker....

My solution is to change my workflow so I don't have to use NPM (or other package managers) anymore for the case in which I triggered the bug. It's even better / faster without NPM, so I'm not sad to have dropped it. It's not something everyone can do, though.

@thany
Copy link

thany commented Jul 28, 2022

I'd rather not go with a workaround. Besides, the number of people complaining is far smaller than the number of people affected. And if everyone would go with a workaround, NPM would not have any incentive to fix the bug. This would lead to hackery "solutions", like we've already seen way too many of in other fields of tech.

The bug is provable, demonstrable, repeatable, persistent, and totally cripples NPM. It could potentially affect millions of people, because there seem to be several kinds of setups that may be a cause for this bug. I honestly don't see why this isn't enough to get someone to fix it properly, and release it within a week or so. There's no shame in having to perform a hotfix release, but there is definitely a lot more shame in letting crippling bugs linger.

@cobaltt7
Copy link

cobaltt7 commented Aug 1, 2022

Ran in to this on NPM 8.13.2 after manually editing the package.json to change a remote dependency to use a local version instead (I changed "dep": "1.2.3" to "dep": "file:...") as well as manually bumping a few other package versions, then running npm i. Removing the 3 dependencies that I made local from my package.json manually, then installing them using npm i fixed the issue.
Here's the error log I got:
2022-07-29T14_26_46_259Z-debug-0.log

@mycarrysun
Copy link

Seems like the problem is actually in the package-lock.json file - when I revert my changes in package-lock after the first install of the new package, it installs again. I didn't have to remove node_modules.

@thany
Copy link

thany commented Sep 28, 2022

Seems like the problem is actually in the package-lock.json file - when I revert my changes in package-lock after the first install of the new package, it installs again. I didn't have to remove node_modules.

That's great, but when never having manually edited the package-lock, it should not cause any runtime errors like this. There for I would consider a manual edit of the package-lock to be a workaround, not a solution.

@mycarrysun
Copy link

Seems like the problem is actually in the package-lock.json file - when I revert my changes in package-lock after the first install of the new package, it installs again. I didn't have to remove node_modules.

That's great, but when never having manually edited the package-lock, it should not cause any runtime errors like this. There for I would consider a manual edit of the package-lock to be a workaround, not a solution.

Yup I agree with you 100% - just wanted to provide more details in case anyone knew how to fix it

@thany
Copy link

thany commented Oct 20, 2022

NPM devs - just ran into this again with the latest npm (8.19.2) that comes with Node.js 16.18.0.

Can we please get an explanation of why this isn't bloody fixed yet? This issue BREAKS NPM ENTIRELY in these scenarios, how does that not give this issue highest possible priority?!

Sorry about the tone of voice, but I'm sure you can understand why I'm getting a little bit cross on the appalling amount of progress being made.

@thany
Copy link

thany commented Nov 14, 2022

Oy! Any and all NPM devs!

Why do you keep releasing new versions of NPM without fixing this issues that COMPLETELY BREAKS NPM.

It's been way too long since this issue got reported, and it hasn't recieved the priority it deserves. Can you tell I'm getting a bit cross? Of course I am. I feel like this issue is getting ignored on purpose.

This issue COMPLETELY AND UTTERLY BREAKS ALL NPM COMMANDS for certain codebases. How does this not get the utmost highest possible priority? Sorry guys/gals, but I would be deeply ashamed for a bug like this, and would probably drop everything and fix this before anything else gets done. I feel this is completely reasonable. Especially after FIVE BLOODY MONTHS of having this BLOCKING issue open without resolution.

But instead, the opposite happens. Explain yourselves please.

@gthb
Copy link

gthb commented Dec 12, 2022

While all-caps shouting and “explain yourselves” is a little over the top 😊, the total silence here is discouraging.

I mean literally it is now discouraging me from putting in work to make a useful bug report for the new npm bug I've found, because here I extracted minimal steps to reproduce and summarized the specific things that are going wrong, and ... no response. For half a year. That puts quite the damper on people's drive to help out the project.

@thany
Copy link

thany commented Dec 13, 2022

I think it's completely justified at this point. This is ridiculous.

@nlf
Copy link
Contributor

nlf commented Dec 13, 2022

first off, i'd like to thank the folks who exercised restraint when expressing their frustration here. i know it's difficult, but your efforts to help us understand this bug as well as the proposed fix above are extremely appreciated.

secondly, i would like to kindly remind folks that we are a very small team responsible for a large number of packages in a rapidly evolving code base. we do our best to triage issues as they come in, but due to the amount of issues we have as well developing new features and processes we often fall behind. while that unfortunately means that some issues do not receive the priority we would like, it does not mean that we are ignoring the issues or the people reporting them.

i'm working on related parts of arborist this week and plan to take a deeper look at this. bear with us a bit longer and we'll get this fixed.

@nlf
Copy link
Contributor

nlf commented Dec 13, 2022

@gthb your reproduction isn't working for me in npm@9, i actually get an unrelated error having to do with resolving the override in your example. i've fixed the unrelated error here and after that i'm still unable to make your reproduction work.

does anyone else have a step-by-step reproduction they can share? i'm not convinced the pull request linked above fixes this issue

@gthb
Copy link

gthb commented Dec 13, 2022

@gthb your reproduction isn't working for me in npm@9

I took a look and yep, it reproduces with those instructions up to and including v8.15.1 but not in v8.16.0 onwards. I git-bisected my way to 050284d which is the commit that causes this to no longer reproduce (EDIT: with my particular repro instructions). Before that commit, npm i changes the package-lock.json this way:

--- package-lock-noproblem.json	2022-12-13 22:10:53
+++ package-lock.json	2022-12-13 22:36:34
@@ -1859,6 +1859,10 @@
         }
       }
     },
+    "node_modules/pg-mem/node_modules/pgsql-ast-parser": {
+      "resolved": "node_modules/pgsql-ast-parser/lib",
+      "link": true
+    },
     "node_modules/pg-pool": {
       "version": "3.5.2",
       "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.5.2.tgz",
@@ -1904,6 +1908,7 @@
       "resolved": "../pgsql-ast-parser/lib",
       "link": true
     },
+    "node_modules/pgsql-ast-parser/lib": {},
     "node_modules/picomatch": {
       "version": "2.3.1",
       "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-2.3.1.tgz",
@@ -3957,6 +3962,11 @@
         "moment": "^2.27.0",
         "object-hash": "^2.0.3",
         "pgsql-ast-parser": "file:../pgsql-ast-parser/lib"
+      },
+      "dependencies": {
+        "pgsql-ast-parser": {
+          "version": "file:node_modules/pgsql-ast-parser/lib"
+        }
       }
     },
     "pg-pool": {

after which the next npm i will fail as described in the repro instructions.

But it's important to note that once package-lock.json is poisoned in this way, the problem will persist, i.e. newer versions of npm will not fix it. They just won't introduce it in a package-lock.json that doesn't already have the problem. So if I nuke my package-lock.json (or copy the OK version back over it like I did in my bisecting expedition here), then the problem is gone (at least as manifested in my repro case). But versions v8.15.1 and earlier will reintroduce it.

So, others who are still experiencing this in npm versions v8.16.0 and newer (e.g. @thany), do you get it also with a package-lock.json generated from scratch? Or only with one that already has the problem?

@lukekarrys lukekarrys added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Mar 8, 2023
@Swivelgames
Copy link

@ixalon This is still not resolved, unfortunately. Looks like your fix was published in npm@9.6.0. I'm currently running npm@9.6.1 and still experiencing this issue. It does appear to be related to workspaces, however.

@Swivelgames
Copy link

Looks like there was a nefarious node_modules that was nested inside a sub-package. Removing that still errors out, but at least now I get Cannot read properties of null (reading 'edgesOut').

@shirotech
Copy link

Still seeing this error for 9.6.3 a workaround for now to get things moving in our project is to use pnpm publish instead. But would be nice to use npm directly.

@KEMBL
Copy link

KEMBL commented Apr 14, 2023

the same issue
npm v9.6.4
node v18.15.0
monorepo

The npm command is run from the command line of Windows v10 x64:

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

what helps: not convenient but removing all node_modules folders from monorepo and npm install

@BobFrankston
Copy link

I made an attempt to trace down the source of the problem, but not knowing much about the structure of the code, there is only so much I can do in a finite time. Somewhere the value of node.target (in link.js?) is being set to null or not being set. It's the latter that is the most difficult to isolate.

I usually find a workaround, but what I really need is to find out where node.target is not getting set yet is still being used.

@BobFrankston
Copy link

I just posted this on a Microsoft discussion. It may give a clue to what may be going on though probably not in itself. Even then it indicates another nasty problem that has no easy solutions.

...

I just figured out a nasty problem with WSL, but I don't have an answer.

Windows and Linux have fundamentally different take on symbolic links.

Windows has both junctions and symlinks (SYMLINKD). If I create a symbolic link in Linux, it is turned into a relative SYMLINKD. This is OK as long as I'm in the same directory but if I'm in node and install a link to another module, the SYMLINKD fails because it is in the wrong context!

npm creates JUNCTIONS (absolute pointers) when run in Windows, but because it uses links in Linux, they become SYMLINKD in Windows and fail when evaluated in the wrong context with very nonobvious error messages.

So I tried to be smart and create an absolute /mnt... link in Linux and it created a JUNCTION in Windows to "...." which obviously didn't work.

JUNCTIONS have their own problems (I need to write about the wormhole bug I ran into due to them) but at least I can create tools if I move directories). But the SYMLINKD problem is harder to work around because I sometimes need to use WSL NPM when npm fails in Windows (which may be due to the problem I'm working around).

This a textbook case of an impedance mismatch that is hard to work around without modifying the semantics of the file systems. And, even then, who knows what implicit dependencies are strewn around.

@SystemParadox
Copy link

To be clear this is absolutely not a Windows or WSL specific issue and has nothing to do with symlinks or npm link.

This error occurs in all environments including plain Linux and without any symlinks involved. It's to do with npm getting confused whilst loading the package tree - especially in a monorepo (workspaces), but it can also happen without workspaces.

@BobFrankston
Copy link

You're probably right. But it is an issue that is difficult to solve so I may report it on its own

@bluwy
Copy link

bluwy commented Oct 31, 2023

pnpm 8.10.0 now uses @npmcli/arborist so this bug also affects pnpm users. In Vite, our publishing step is failing due to this error . It fails locally running npm pack or pnpm pack in packages/vite. Within that directory, I can also reproduce by simply running this:

import Arborist from '@npmcli/arborist'
const a = new Arborist()
await a.loadActual()

I don't understand what the right fix is, but I'm able to workaround the issue by wrapping this part with a if (root !== this) {}:

// break all linksIn, we're going to re-set them if needed later
for (const link of this.linksIn) {
link[_target] = null
this.linksIn.delete(link)
}

Because it seems that when this part below kicks in, it does not re-set the _target anymore:

if (root === this) {
this[_refreshLocation]()
} else {

zkochan added a commit to pnpm/npm-cli that referenced this issue Nov 1, 2023
zkochan added a commit to pnpm/pnpm that referenced this issue Nov 1, 2023
Related issues:
- npm/cli#5007
Fixed via: pnpm/npm-cli@0af133e

To reproduce in the pnpm repo, run `pnpm pack` in `store/store-path`.
@haoqunjiang
Copy link

Vue is facing this issue, too. But I'm not sure if it's our fault or npm's. Because the direct cause is a circular dependency:
https://github.com/vuejs/core/blob/9ca468c68bb1b84ce3ab5b4bedcabad3c2a7b618/packages/server-renderer/package.json#L34-L36
https://github.com/vuejs/core/blob/9ca468c68bb1b84ce3ab5b4bedcabad3c2a7b618/packages/vue/package.json#L98-L104
@vue/server-renderer depends on vue as a peer dependency, while vue depends on @vue/server-renderer as a direct dependency. After removing this circular dependency, the error's gone.

@lukekarrys lukekarrys self-assigned this Nov 4, 2023
@mwe
Copy link

mwe commented Nov 9, 2023

I am experiencing this issue with npm pack and on an npm install when i have a package lock and a node_modules folder.

The package json contains file dependencies { dependencies : { "mymodule": "file:.." }} resulting in junction folders.

I have debugged the aborist code and the node that it visiting and throwing on is access to an arborist Link class. When the link class is constructed, the target property is set. Somewhere in the code the Link.target is set to null, i am not able to find where that happens and why.

Deleting the node_modules resolves the problem, running an npm install a second time causes the error again

@kant2002
Copy link

I see this issue in the project with links to two sibling modules one which has dependency on anyther
be-eslint-config -> be-eslint-plugin -> be-configs

mkdir diia
cd diia
# be-configs
git clone https://github.com/diia-open-source/be-configs
pushd be-configs
npm i
npm run build
npm test
npm link
popd

# be-eslint-plugin
git clone https://github.com/diia-open-source/be-eslint-plugin
pushd be-eslint-plugin
npm link @diia-inhouse/configs
npm i
npm run build
npm test
npm link
popd

# be-eslint-config
git clone https://github.com/diia-open-source/be-eslint-config
pushd be-eslint-config
npm link @diia-inhouse/configs @diia-inhouse/eslint-plugin
# Here you will run into https://github.com/npm/cli/issues/5007
npm i

ArboristLink which trigger this issue is following


ArboristLink {
  name: '@diia-inhouse/configs',
  version: '1.27.1',
  location: '../be-eslint-plugin/node_modules/@diia-inhouse/configs',
  path: 'D:\\d\\kant\\GitHub\\diia\\be-eslint-plugin\\node_modules\\@diia-inhouse\\configs',
  realpath: 'D:\\d\\kant\\GitHub\\diia\\be-configs',
  resolved: 'file:../../../be-configs',
  dev: true,
  optional: true,
  peer: true,
  edgesIn: Set(1) { { ../be-eslint-plugin dev @diia-inhouse/configs@^1.26.3 } },
  target: null
}

additional information

node inspect and setBreakpoint('build-ideal-tree.js', 204) which is await this.#fixDepFlags()

exec('console.log(this.idealTree.edgesOut.get("@diia-inhouse/configs"))')
exec('console.log(this.idealTree.edgesOut.get("@diia-inhouse/eslint-plugin"))')

Results in

ArboristEdge {
  name: '@diia-inhouse/configs',
  spec: '1.26.3',
  type: 'dev',
  from: '',
  to: 'node_modules/@diia-inhouse/configs',
  error: 'INVALID'
}
ArboristEdge {
  name: '@diia-inhouse/eslint-plugin',
  spec: '1.6.0',
  type: 'prod',
  from: '',
  to: 'node_modules/@diia-inhouse/eslint-plugin'
}

at the same time

exec('console.log(this.idealTree.children.get("@diia-inhouse/eslint-plugin").target.edgesOut.get("@diia-inhouse/configs"))')

results in

ArboristEdge {
  name: '@diia-inhouse/configs',
  spec: '^1.26.3',
  type: 'dev',
  from: '../be-eslint-plugin',
  to: '../be-eslint-plugin/node_modules/@diia-inhouse/configs'
}

but

exec('console.log(this.idealTree.children.get("@diia-inhouse/configs"))')

results in

ArboristLink {
  name: '@diia-inhouse/configs',
  version: '1.27.1',
  location: 'node_modules/@diia-inhouse/configs',
  path: 'D:\\d\\kant\\GitHub\\diia\\be-eslint-config\\node_modules\\@diia-inhouse\\configs',
  realpath: 'D:\\d\\kant\\GitHub\\diia\\be-configs',
  resolved: 'file:../../../be-configs',
  dev: true,
  edgesIn: Set(1) { { "" dev @diia-inhouse/configs@1.26.3 INVALID } },
  target: ArboristNode {
    name: 'be-configs',
    packageName: '@diia-inhouse/configs',
    version: '1.27.1',
    location: '../be-configs',
    path: 'D:\\d\\kant\\GitHub\\diia\\be-configs',
    dev: true,
    edgesOut: Map(33) {
    ,,,,
    }
    ....
}

Notice the difference in versions.

@BobFrankston
Copy link

Thanks very much for the additional details. It's worth noting that when I use WSL to run the identical command in Linux it works. The caveat is that Linux creates symlinks which do not work when dynamically importing to another app, so I have to work around it by converting the symlinks to junctions.

@luckened
Copy link

luckened commented May 1, 2024

I don't think it's solely an npm issue, but I've discovered a workaround for my workspace/monorepo setup.

We were declaring workspaces layout like this:

// root package.json
  "workspaces": [
    "./apps/**",
  ],

this ** glob pattern matches all subfolders of everything under apps/. This includes possible nested node_modules/, which is our case.

a single * solved the issue

// root package.json
  "workspaces": [
    "./apps/*",
  ],

If your use case truly requires matching all subfolders (e.g. apps/some-subfolder/my-nested-app), then you'll likely need additional glob pattern entries to selectively include or exclude them as needed

@haggholm
Copy link

haggholm commented May 1, 2024

It definitely isn't limited to workspaces with wildcards, though; we are seeing this in a monorepo where all the child projects are listed explicitly with no globs of any kind, neither ** nor *.

@lukekarrys lukekarrys removed their assignment May 17, 2024
@lukekarrys lukekarrys removed the Release 8.x work is associated with a specific npm 8 release label May 17, 2024
wraithgar pushed a commit that referenced this issue Jul 1, 2024
…#7579)

<!-- What / Why -->
If a node represents a symbolic link or a file dep (node.isLink is
true), its target is expected to reference another node in the
dependency tree. If the linking is not done correctly or is incomplete,
node.target might be null.
<!-- Describe the request in detail. What it does and why it's being
changed. -->
in this PR, a null check is added to ensure node.target is not null or
before proceeding, which will prevent causing errors like:
`npm error Cannot set properties of null (setting 'peer')` 

## References
  Related to #7065, 
  Fixes #6622, #5007,
  Closes #6622, #5007
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 1 high priority issue
Projects
None yet
Development

No branches or pull requests