Replace entire dep sets, so peer loops don't lock

Discovered an interesting issue with the following dependency set:

```json
{
  "devDependencies": {
    "webpack-dev-server": "^3.11.0",
    "@pmmmwh/react-refresh-webpack-plugin": "^0.4.3"
  }
}
```

`webpack-dev-server` here has a peerDependency on webpack v4.  But, the
dependencies of `@pmmmwh/react-refresh-webpack-plugin` have peer
dependencies on webpack at `4 || 5`.

So, a resolution _should_ be possible.  Since the
`@pmmmwh/react-refresh-webpack-plugin` package is alphabetically first,
it loads its deps, and ends up placing webpack@5 to satisfy the dep with
the latest and greatest.  Then when `webpack-dev-server` gets to its
eventual peer dep on webpack, it can't replace it, because `webpack@5`
has a dependency on `terser-webpack-plugin@^5.0.3`, which in turn has a
peer dependency on `webpack@5.4.0`.

When we check to see if webpack 5 can be replaced, we find it has a
dependent, and reject the replacement.  But that dependent is only
present as a dependency of the thing being replaced, so should not be
considered.

Second, while the source of the `terser-webpack-plugin` is `webpack`, it
cannot be installed there, because it has peer deps that are also peer
deps of webpack.  So, we _must_ place it in the root node_modules, but
were not attempting the "source" location overrides, because the root is
not the source.  This was a second issue resulting in `ERESOLVE` errors
even when `--force` was applied, with this dependency set:

```json
{
  "devDependencies": {
    "webpack-dev-server": "^3.11.0",
    "webpack": "^5.0.0"
  }
}
```

Fixes: #180
Fixes: npm/cli#2123

PR-URL: https://github.com/pull/182
Credit: @isaacs
Close: #182
Reviewed-by: @darcyclarke

lib/arborist/build-ideal-tree.js

@@ -1085,14 +1085,22 @@ This is a one-time fix-up, please be patient...
 
     let target
     let canPlace = null
+    let isSource = false
+    const source = this[_peerSetSource].get(dep)
     for (let check = start; check; check = check.resolveParent) {
+      // we always give the FIRST place we possibly *can* put this a little
+      // extra prioritization with peer dep overrides and deduping
+      if (check === source)
+        isSource = true
+
       // if the current location has a peerDep on it, then we can't place here
       // this is pretty rare to hit, since we always prefer deduping peers.
       const checkEdge = check.edgesOut.get(edge.name)
       if (!check.isTop && checkEdge && checkEdge.peer)
         continue
 
-      const cp = this[_canPlaceDep](dep, check, edge, peerEntryEdge, peerPath)
+      const cp = this[_canPlaceDep](dep, check, edge, peerEntryEdge, peerPath, isSource)
+      isSource = false
 
       // anything other than a conflict is fine to proceed with
       if (cp !== CONFLICT) {
@@ -1164,7 +1172,7 @@ This is a one-time fix-up, please be patient...
       const oldDeps = []
       for (const [name, edge] of oldChild.edgesOut.entries()) {
         if (!newDep.edgesOut.has(name) && edge.to)
-          oldDeps.push(edge.to)
+          oldDeps.push(...gatherDepSet([edge.to], e => e.to !== edge.to))
       }
       newDep.replace(oldChild)
       this[_pruneForReplacement](newDep, oldDeps)
@@ -1265,14 +1273,17 @@ This is a one-time fix-up, please be patient...
     // deps that the new node doesn't depend on but the old one did.
     const invalidDeps = new Set([...node.edgesOut.values()]
       .filter(e => e.to && !e.valid).map(e => e.to))
-    for (const dep of oldDeps)
-      invalidDeps.add(dep)
+    for (const dep of oldDeps) {
+      const set = gatherDepSet([dep], e => e.to !== dep && e.valid)
+      for (const dep of set)
+        invalidDeps.add(dep)
+    }
 
     // ignore dependency edges from the node being replaced, but
     // otherwise filter the set down to just the set with no
     // dependencies from outside the set, except the node in question.
     const deps = gatherDepSet(invalidDeps, edge =>
-      edge.from !== node && edge.to !== node)
+      edge.from !== node && edge.to !== node && edge.valid)
 
     // now just delete whatever's left, because it's junk
     for (const dep of deps)
@@ -1299,16 +1310,24 @@ This is a one-time fix-up, please be patient...
   // checking, because either we're leaving it alone, or it won't work anyway.
   // When we check peers, we pass along the peerEntryEdge to track the
   // original edge that caused us to load the family of peer dependencies.
-  [_canPlaceDep] (dep, target, edge, peerEntryEdge = null, peerPath = []) {
+  [_canPlaceDep] (dep, target, edge, peerEntryEdge = null, peerPath = [], isSource = false) {
     /* istanbul ignore next */
     debug(() => {
       if (!dep)
         throw new Error('no dep??')
     })
     const entryEdge = peerEntryEdge || edge
     const source = this[_peerSetSource].get(dep)
-    const isSource = target === source
-    const { isRoot, isWorkspace } = source || {}
+    isSource = isSource || target === source
+    // if we're overriding the source, then we care if the *target* is
+    // ours, even if it wasn't actually the original source, since we
+    // are depending on something that has a dep that can't go in its own
+    // folder.  for example, a -> b, b -> PEER(a).  Even though a is the
+    // source, b has to be installed up a level, and if the root package
+    // depends on a, and it has a conflict, it's our problem.  So, the root
+    // (or whatever is bringing in a) becomes the "effective source" for
+    // the purposes of this calculation.
+    const { isRoot, isWorkspace } = isSource ? target : source || {}
     const isMine = isRoot || isWorkspace
 
     // Useful testing thingie right here.
@@ -1333,7 +1352,7 @@ This is a one-time fix-up, please be patient...
       const { version: newVer } = dep
       const tryReplace = curVer && newVer && semver.gte(newVer, curVer)
       if (tryReplace && dep.canReplace(current)) {
-        const res = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath)
+        const res = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath, isSource)
         /* istanbul ignore else - It's extremely rare that a replaceable
          * node would be a conflict, if the current one wasn't a conflict,
          * but it is theoretically possible if peer deps are pinned.  In
@@ -1353,7 +1372,7 @@ This is a one-time fix-up, please be patient...
       // a bit harder to be singletons.
       const preferDedupe = this[_preferDedupe] || edge.peer
       if (preferDedupe && !tryReplace && dep.canReplace(current)) {
-        const res = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath)
+        const res = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath, isSource)
         /* istanbul ignore else - It's extremely rare that a replaceable
          * node would be a conflict, if the current one wasn't a conflict,
          * but it is theoretically possible if peer deps are pinned.  In
@@ -1421,7 +1440,7 @@ This is a one-time fix-up, please be patient...
           }
         }
         if (canReplace) {
-          const ret = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath)
+          const ret = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath, isSource)
           /* istanbul ignore else - extremely rare that the peer set would
            * conflict if we can replace the node in question, but theoretically
            * possible, if peer deps are pinned aggressively. */
@@ -1482,14 +1501,14 @@ This is a one-time fix-up, please be patient...
     }
 
     // no objections!  ok to place here
-    return this[_canPlacePeers](dep, target, edge, OK, peerEntryEdge, peerPath)
+    return this[_canPlacePeers](dep, target, edge, OK, peerEntryEdge, peerPath, isSource)
   }
 
   // make sure the family of peer deps can live here alongside it.
   // this doesn't guarantee that THIS solution will be the one we take,
   // but it does establish that SOME solution exists at this level in
   // the tree.
-  [_canPlacePeers] (dep, target, edge, ret, peerEntryEdge, peerPath) {
+  [_canPlacePeers] (dep, target, edge, ret, peerEntryEdge, peerPath, isSource) {
     // do not go in cycles when we're resolving a peer group
     if (!dep.parent || peerEntryEdge && peerPath.includes(dep))
       return ret
@@ -1501,7 +1520,7 @@ This is a one-time fix-up, please be patient...
       if (!peerEdge.peer || !peerEdge.to)
         continue
       const peer = peerEdge.to
-      const canPlacePeer = this[_canPlaceDep](peer, target, peerEdge, entryEdge, peerPath)
+      const canPlacePeer = this[_canPlaceDep](peer, target, peerEdge, entryEdge, peerPath, isSource)
       if (canPlacePeer !== CONFLICT)
         continue
 

lib/node.js

@@ -35,6 +35,7 @@ const {normalize} = require('read-package-json-fast')
 const {getPaths: getBinPaths} = require('bin-links')
 const npa = require('npm-package-arg')
 const debug = require('./debug.js')
+const gatherDepSet = require('./gather-dep-set.js')
 
 const {resolve, relative, dirname, basename} = require('path')
 const _package = Symbol('_package')
@@ -640,8 +641,14 @@ class Node {
     if (node.name !== this.name)
       return false
 
+    // gather up all the deps of this node and that are only depended
+    // upon by deps of this node.  those ones don't count, since
+    // they'll be replaced if this node is replaced anyway.
+    const depSet = gatherDepSet([this], e => e.to !== this && e.valid)
+
     for (const edge of this.edgesIn) {
-      if (!edge.satisfiedBy(node))
+      // only care about edges that don't originate from this node
+      if (!depSet.has(edge.from) && !edge.satisfiedBy(node))
         return false
     }
 

scripts/reify.js

@@ -15,6 +15,10 @@ process.on('timeEnd', name => {
   }
   const res = process.hrtime(timers[name])
   delete timers[name]
+
+  if (options.quiet)
+    return
+
   console.error(name, res[0] * 1e3 + res[1] / 1e6)
 })
 process.on('exit', () => {
@@ -104,6 +108,8 @@ process.on('log', (level, ...args) => {
   if (level === 'warn' && args[0] === 'ERESOLVE') {
     args[2] = require('util').inspect(args[2], { depth: Infinity })
   }
+  if (options.quiet)
+    return
   return console.error(level, ...args)
 })