Allow --force to override conflicted peerOptional

bin/ideal.js

@@ -1,59 +1,11 @@
 const Arborist = require('../')
 
+const { inspect } = require('util')
 const options = require('./lib/options.js')
 const print = require('./lib/print-tree.js')
 require('./lib/logging.js')
 require('./lib/timers.js')
 
-const c = require('chalk')
-
-const whichIsA = (name, dependents, indent = '  ') => {
-  if (!dependents || dependents.length === 0)
-    return ''
-  const str = `\nfor: ` +
-    dependents.map(dep => {
-      return dep.more ? `${dep.more} more (${dep.names.join(', ')})`
-        : `${dep.type} dependency ` +
-          `${c.bold(name)}@"${c.bold(dep.spec)}"` + `\nfrom:` +
-          (dep.from.location ? (dep.from.name
-            ? ` ${c.bold(dep.from.name)}@${c.bold(dep.from.version)} ` +
-              c.dim(`at ${dep.from.location}`)
-            : ' the root project')
-          : ` ${c.bold(dep.from.name)}@${c.bold(dep.from.version)}`) +
-          whichIsA(dep.from.name, dep.from.dependents, '  ')
-    }).join('\nand: ')
-
-  return str.split(/\n/).join(`\n${indent}`)
-}
-
-const explainEresolve = ({ dep, current, peerConflict, fixWithForce }) => {
-  return (!dep.whileInstalling ? '' : `While resolving: ` +
-  `${c.bold(dep.whileInstalling.name)}@${c.bold(dep.whileInstalling.version)}\n`) +
-
-  `Found: ` +
-    `${c.bold(current.name)}@${c.bold(current.version)} ` +
-    c.dim(`at ${current.location}`) +
-    `${whichIsA(current.name, current.dependents)}` +
-
-  `\n\nCould not add conflicting dependency: ` +
-    `${c.bold(dep.name)}@${c.bold(dep.version)} ` +
-    c.dim(`at ${dep.location}`) +
-   `${whichIsA(dep.name, dep.dependents)}\n` +
-
-  (!peerConflict ? '' :
-    `\nConflicting peer dependency: ` +
-      `${c.bold(peerConflict.name)}@${c.bold(peerConflict.version)} ` +
-      c.dim(`at ${peerConflict.location}`) +
-      `${whichIsA(peerConflict.name, peerConflict.dependents)}\n`
-  ) +
-
-  `\nFix the upstream dependency conflict, or
-run this command with  --legacy-peer-deps${
-  fixWithForce ? ' or --force' : ''}
-to accept an incorrect (and potentially broken) dependency resolution.
-`
-}
-
 const start = process.hrtime()
 new Arborist(options).buildIdealTree(options).then(tree => {
   const end = process.hrtime(start)
@@ -62,7 +14,7 @@ new Arborist(options).buildIdealTree(options).then(tree => {
   if (tree.meta && options.save)
     tree.meta.save()
 }).catch(er => {
-  console.error(er)
-  if (er.code === 'ERESOLVE')
-    console.error(explainEresolve(er))
+  const opt = { depth: Infinity, color: true }
+  console.error(er.code === 'ERESOLVE' ? inspect(er, opt) : er)
+  process.exitCode = 1
 })

bin/lib/logging.js

@@ -26,7 +26,7 @@ if (loglevel !== 'silent') {
       return
     const pref = `${process.pid} ${level} `
     if (level === 'warn' && args[0] === 'ERESOLVE')
-      args[2] = inspect(args[2], { depth: Infinity })
+      args[2] = inspect(args[2], { depth: 10 })
     const msg = pref + format(...args).trim().split('\n').join(`\n${pref}`)
     console.error(msg)
   })

lib/arborist/build-ideal-tree.js

@@ -806,6 +806,7 @@ This is a one-time fix-up, please be patient...
       // a virtual root of whatever brought in THIS node.
       // so we VR the node itself if the edge is not a peer
       const source = edge.peer ? peerSource : node
+
       const virtualRoot = this[_virtualRoot](source, true)
       // reuse virtual root if we already have one, but don't
       // try to do the override ahead of time, since we MAY be able
@@ -827,13 +828,17 @@ This is a one-time fix-up, please be patient...
       // +-- z@1
       // But if x and y are loaded in the same virtual root, then they will
       // be forced to agree on a version of z.
+      const required = new Set([edge.from])
+      const parent = edge.peer ? virtualRoot : null
       const dep = vrDep && vrDep.satisfies(edge) ? vrDep
-        : await this[_nodeFromEdge](edge, edge.peer ? virtualRoot : null)
+        : await this[_nodeFromEdge](edge, parent, null, required)
+
       /* istanbul ignore next */
       debug(() => {
         if (!dep)
           throw new Error('no dep??')
       })
+
       tasks.push({edge, dep})
     }
 
@@ -870,7 +875,7 @@ This is a one-time fix-up, please be patient...
 
   // loads a node from an edge, and then loads its peer deps (and their
   // peer deps, on down the line) into a virtual root parent.
-  async [_nodeFromEdge] (edge, parent_, secondEdge = null) {
+  async [_nodeFromEdge] (edge, parent_, secondEdge, required) {
     // create a virtual root node with the same deps as the node that
     // is requesting this one, so that we can get all the peer deps in
     // a context where they're likely to be resolvable.
@@ -901,6 +906,11 @@ This is a one-time fix-up, please be patient...
     // ensure the one we want is the one that's placed
     node.parent = parent
 
+    if (required.has(edge.from) && edge.type !== 'peerOptional' ||
+        secondEdge && (
+          required.has(secondEdge.from) && secondEdge.type !== 'peerOptional'))
+      required.add(node)
+
     // handle otherwise unresolvable dependency nesting loops by
     // creating a symbolic link
     // a1 -> b1 -> a2 -> b2 -> a1 -> ...
@@ -914,7 +924,7 @@ This is a one-time fix-up, please be patient...
     // keep track of the thing that caused this node to be included.
     const src = parent.sourceReference
     this[_peerSetSource].set(node, src)
-    return this[_loadPeerSet](node)
+    return this[_loadPeerSet](node, required)
   }
 
   [_virtualRoot] (node, reuse = false) {
@@ -1059,7 +1069,7 @@ This is a one-time fix-up, please be patient...
   // gets placed first.  In non-strict mode, we behave strictly if the
   // virtual root is based on the root project, and allow non-peer parent
   // deps to override, but throw if no preference can be determined.
-  async [_loadPeerSet] (node) {
+  async [_loadPeerSet] (node, required) {
     const peerEdges = [...node.edgesOut.values()]
       // we typically only install non-optional peers, but we have to
       // factor them into the peerSet so that we can avoid conflicts
@@ -1074,10 +1084,12 @@ This is a one-time fix-up, please be patient...
       const parentEdge = node.parent.edgesOut.get(edge.name)
       const {isProjectRoot, isWorkspace} = node.parent.sourceReference
       const isMine = isProjectRoot || isWorkspace
+      const conflictOK = this[_force] || !isMine && !this[_strictPeerDeps]
+
       if (!edge.to) {
         if (!parentEdge) {
           // easy, just put the thing there
-          await this[_nodeFromEdge](edge, node.parent)
+          await this[_nodeFromEdge](edge, node.parent, null, required)
           continue
         } else {
           // if the parent's edge is very broad like >=1, and the edge in
@@ -1088,14 +1100,16 @@ This is a one-time fix-up, please be patient...
           // a conflict.  this is always a problem in strict mode, never
           // in force mode, and a problem in non-strict mode if this isn't
           // on behalf of our project.  in all such cases, we warn at least.
-          await this[_nodeFromEdge](parentEdge, node.parent, edge)
+          const dep = await this[_nodeFromEdge](parentEdge, node.parent, edge, required)
 
           // hooray! that worked!
           if (edge.valid)
             continue
 
-          // allow it
-          if (this[_force] || !isMine && !this[_strictPeerDeps])
+          // allow it.  either we're overriding, or it's not something
+          // that will be installed by default anyway, and we'll fail when
+          // we get to the point where we need to, if we need to.
+          if (conflictOK || !required.has(dep))
             continue
 
           // problem
@@ -1108,7 +1122,7 @@ This is a one-time fix-up, please be patient...
       // in non-strict mode if it's not our fault.  don't warn here, because
       // we are going to warn again when we place the deps, if we end up
       // overriding for something else.
-      if (this[_force] || !isMine && !this[_strictPeerDeps])
+      if (conflictOK)
         continue
 
       // ok, it's the root, or we're in unforced strict mode, so this is bad
@@ -1204,8 +1218,25 @@ This is a one-time fix-up, please be patient...
         break
     }
 
-    if (!target)
-      this[_failPeerConflict](edge)
+    // if we can't find a target, that means that the last placed checked
+    // (and all the places before it) had a copy already.  if we're in
+    // --force mode, then the user has explicitly said that they're ok
+    // with conflicts.  This can only occur in --force mode in the case
+    // when a node was added to the tree with a peerOptional dep that we
+    // ignored, and then later, that edge became invalid, and we fail to
+    // resolve it.  We will warn about it in a moment.
+    if (!target) {
+      if (this[_force]) {
+        // we know that there is a dep (not the root) which is the target
+        // of this edge, or else it wouldn't have been a conflict.
+        target = edge.to.resolveParent
+        canPlace = KEEP
+      } else
+        this[_failPeerConflict](edge)
+    } else {
+      // it worked, so we clearly have no peer conflicts at this point.
+      this[_peerConflict] = null
+    }
 
     this.log.silly(
       'placeDep',
@@ -1216,9 +1247,6 @@ This is a one-time fix-up, please be patient...
       `want: ${edge.spec || '*'}`
     )
 
-    // it worked, so we clearly have no peer conflicts at this point.
-    this[_peerConflict] = null
-
     // Can only get KEEP here if the original edge was valid,
     // and we're checking for an update but it's already up to date.
     if (canPlace === KEEP) {
@@ -1404,6 +1432,7 @@ This is a one-time fix-up, please be patient...
     })
     const entryEdge = peerEntryEdge || edge
     const source = this[_peerSetSource].get(dep)
+
     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