start adding tests for CanPlaceDep

Also, this includes the fix for npm/cli#3411

lib/can-place-dep.js

@@ -52,7 +52,17 @@ class CanPlaceDep {
   // in a node_modules folder.
   // edge is the edge that we're trying to satisfy with this placement.
   // parent is the CanPlaceDep object of the entry node when placing a peer.
-  constructor ({ dep, target, edge, preferDedupe, parent = null, peerPath = [] }) {
+  constructor (options) {
+    const {
+      dep,
+      target,
+      edge,
+      preferDedupe,
+      parent = null,
+      peerPath = [],
+      explicitRequest = false,
+    } = options
+
     debug(() => {
       if (!dep)
         throw new Error('no dep provided to CanPlaceDep')
@@ -75,6 +85,8 @@ class CanPlaceDep {
     this.dep = dep
     this.target = target
     this.edge = edge
+    this.explicitRequest = explicitRequest
+
     // preventing cycles when we check peer sets
     this.peerPath = peerPath
     // we always prefer to dedupe peers, because they are trying
@@ -100,12 +112,14 @@ class CanPlaceDep {
     debug(() => {
       const nodeSnapshot = JSON.stringify(dep)
       const treeSnapshot = JSON.stringify(target.root)
+      /* istanbul ignore if */
       if (this._nodeSnapshot !== nodeSnapshot) {
         throw Object.assign(new Error('dep changed in CanPlaceDep'), {
           expect: this._nodeSnapshot,
           actual: nodeSnapshot,
         })
       }
+      /* istanbul ignore if */
       if (this._treeSnapshot !== treeSnapshot) {
         throw Object.assign(new Error('tree changed in CanPlaceDep'), {
           expect: this._treeSnapshot,
@@ -139,11 +153,10 @@ class CanPlaceDep {
   // we know that the target has a dep by this name in its node_modules
   // already.  Can return KEEP, REPLACE, or CONFLICT.
   checkCanPlaceCurrent () {
-    const { current, target, edge, dep } = this
-    // XXX: shouldn't this be REPLACE if it's an explicitRequest?
+    const { preferDedupe, explicitRequest, current, target, edge, dep } = this
     if (dep.matches(current)) {
       if (current.satisfies(edge) || this.edgeOverride)
-        return KEEP
+        return explicitRequest ? REPLACE : KEEP
     }
 
     const { version: curVer } = current
@@ -160,11 +173,11 @@ class CanPlaceDep {
     }
 
     // ok, can't replace the current with new one, but maybe current is ok?
-    if (current.satisfies(edge))
+    if (current.satisfies(edge) && (!explicitRequest || preferDedupe))
       return KEEP
 
     // if we prefer deduping, then try replacing newer with older
-    if (this.preferDedupe && !tryReplace && dep.canReplace(current)) {
+    if (preferDedupe && !tryReplace && dep.canReplace(current)) {
       const cpp = this.canPlacePeers(REPLACE)
       if (cpp !== CONFLICT)
         return cpp
@@ -182,7 +195,7 @@ class CanPlaceDep {
 
     // if we are not checking a peerDep, then we MUST place it here, in the
     // target that has a non-peer dep on it.
-    if (!this.parent && target === edge.from)
+    if (!edge.peer && target === edge.from)
       return this.canPlacePeers(REPLACE)
 
     // now we have a parent, or it's a peer of a link target,
@@ -329,6 +342,8 @@ class CanPlaceDep {
         parent: this,
         edge: peerEdge,
         peerPath,
+        // always place peers in preferDedupe mode
+        preferDedupe: true,
       })
       this.children.push(cpp)
 

lib/place-dep.js

@@ -116,6 +116,7 @@ class PlaceDep {
         parent: this.parent && this.parent.canPlace,
         target,
         preferDedupe,
+        explicitRequest: this.explicitRequests.has(edge),
       })
       checks.set(target, cpd)