Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Commit

Permalink
Do not conflict on meta-peers that will not be replaced
Browse files Browse the repository at this point in the history
When in a situation where a project has dependencies on several members
of a peer set, it was encountering a spurious ERESOLVE error if the
entire set was being replaced, but _one_ of the members did not need to
be.

For example:

```
root -> (a@1, b@1, c@1)
a@1 -> PEER(b@1, c@1)
a@2 -> PEER(b@2, c@1)
b@1 -> PEER(c@1)
b@2 -> PEER(c@1)
```

resulting in the tree:

```
root
+-- a@1
+-- b@1
+-- c@1
```

If we try to upgrade both `a` and `b` to version 2, however, we would
check the set of peers for each dependency node being replaced, and find
that there was a `c` node with a non-peer dependency from the root node,
and treat it as a conflict, even though there was no need for `c` to be
modified at all!

Resolve this by skipping the check when not doing `canPlacePeers` (since
it's either already a conflict, or already set to be overridden by the
version from the virtualRoot), and ignoring the conflict when a
non-overridden node exists in the virtualRoot which meets the non-peer
edge keeping the current dependency node in place.  In those cases,
either we will replace or keep the current node anyway, so there's no
need to conflict on it.

Fix: npm/cli#2000
  • Loading branch information
isaacs committed Oct 21, 2020
1 parent 1d710cb commit 9bb36dc
Show file tree
Hide file tree
Showing 13 changed files with 8,101 additions and 716 deletions.
42 changes: 29 additions & 13 deletions lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -1339,20 +1339,36 @@ This is a one-time fix-up, please be patient...
// If strict or ours, conflict. Otherwise, keep.
if (isSource) {
// check to see if the current module could go deeper in the tree
const peerSet = getPeerSet(current)
let canReplace = true
OUTER: for (const p of peerSet) {
// if any have a non-peer dep from the target, or a peer dep if
// the target is root, then cannot safely replace and dupe deeper.
for (const edge of p.edgesIn) {
if (peerSet.has(edge.from))
continue

// only respect valid edges, however, since we're likely trying
// to fix the very one that's currently broken!
if (edge.from === target && edge.valid) {
canReplace = false
break OUTER
// only do this check when we're placing peers. when we're placing
// the original in the source, we know that the edge from the source
// is the thing we're trying to place, so its peer set will need to be
// placed here as well. the virtualRoot already has the appropriate
// overrides applied.
if (peerEntryEdge) {
const peerSet = getPeerSet(current)
OUTER: for (const p of peerSet) {
// if any have a non-peer dep from the target, or a peer dep if
// the target is root, then cannot safely replace and dupe deeper.
for (const edge of p.edgesIn) {
if (peerSet.has(edge.from))
continue

// only respect valid edges, however, since we're likely trying
// to fix the very one that's currently broken! If the virtual
// root's replacement is ok, and doesn't have any invalid edges
// indicating that it was an overridden peer, then ignore the
// conflict and continue. If it WAS an override, then we need
// to get the conflict here so that we can decide whether to
// accept the current dep node, clobber it, or fail the install.
if (edge.from === target && edge.valid) {
const rep = dep.parent.children.get(edge.name)
const override = rep && ([...rep.edgesIn].some(e => !e.valid))
if (!rep || !rep.satisfies(edge) || override) {
canReplace = false
break OUTER
}
}
}
}
}
Expand Down
Loading

0 comments on commit 9bb36dc

Please sign in to comment.