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] npm dedupe fails with TypeError: Cannot read property 'path' of null #3565

Closed
1 task done
zfeher opened this issue Jul 21, 2021 · 0 comments
Closed
1 task done
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release

Comments

@zfeher
Copy link

zfeher commented Jul 21, 2021

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

npm dedupe fails with an error TypeError: Cannot read property 'path' of null

npm debug.log

Expected Behavior

npm dedupe should not fail

Steps To Reproduce

  1. In this environment: Ubuntu 20.04, Node 14.17.3, npm 7.20.0

  2. With this config:

// package.json
{
  "devDependencies": {
    "@storybook/vue": "6.3.4"
  }
}
  1. Run npm i, then run npm dedupe

  2. See error: TypeError: Cannot read property 'path' of null

Environment

  • OS: Ubuntu 20.04
  • Node: 14.17.3
  • npm: 7.20.0
@zfeher zfeher 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 Jul 21, 2021
isaacs added a commit to npm/arborist that referenced this issue Aug 12, 2021
When a dependency graph cycles back on itself incompatibly like this:

```
a@1 -> b@1
b@1 -> a@2
a@2 -> b@2
b@2 -> a@1
```

We would find ourselves unable to handle the conflict via nesting.  For
example:

```
root
+-- a@1 -> b@1
+-- b@1 -> a@2
    +-- a@2 -> b@2
    +-- b@2 -> a@1
        +-- a@1 -> b@1
        +-- b@1 -> a@2
            +-- a@2 -> b@2
            +-- b@2 -> a@1 (cycling forever)
```

In order to address this, we create a link when such a cycle is
detected.

```
root
+-- a@1 -> b@1
+-- b@1 -> a@2
    +-- a@2 -> b@2
    +-- b@2 -> a@1
        +-- a@1 -> b@1
        +-- b@1 -> link to root/node_modules/b@1
```

Prior to the recent refactor to move much of the dependency placement
logic out of Arborist.buildIdealTree and into the PlaceDep class, this
link was created right at the moment when a new dependency was created
in a temp tree.

However, if we feed that Link object into the PlaceDep flow, it will
(correctly) see that the Link does not match the Node further up the
tree, and attempt to replace it.

Compounding the problem (and why it appeared in `npm dedupe` and not
`npm install`) is the fact that explicitly named updates are _always_
treated as a "problem edge", so that they can be re-evaluated.

So, rather than creating a Node to be tested against the tree, it was
creating a Link object, and then attempting to replace the Link's target
with the Link itself, which caused some havoc.

This patch moves the loop detection and remediating Link creation into
the PlaceDep class, which is the more proper place for it, as that class
owns the "put deps into the tree" logic, and this is clearly a "put deps
into the tree" type of situation.

Via: @ParadoxInfinite
Close: npm/cli#3632
Close: #308
Fix: npm/cli#3565
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release
Projects
None yet
1 participant