Revert "install/dedupe: fix hoisting of packages with peerDeps (#147)"

This reverts commit 91314e7.

Fixes: https://npm.community/t/npm-6-8-0-next-0-regression-in-maximally-flat-install/5118

Author: zkat

lib/dedupe.js

@@ -142,9 +142,8 @@ function hoistChildren_ (tree, diff, seen, next) {
         [andComputeMetadata(tree)]
       ], done)
     }
-    // find a location where it's installable
-    var hoistTo = earliestInstallable(tree, tree, child.package, log, child)
-    if (hoistTo && hoistTo !== tree) {
+    var hoistTo = earliestInstallable(tree, tree.parent, child.package, log)
+    if (hoistTo) {
       move(child, hoistTo, diff)
       chain([
         [andComputeMetadata(hoistTo)],

lib/install/deps.js

@@ -650,7 +650,7 @@ function resolveWithNewModule (pkg, tree, log, next) {
   return isInstallable(pkg, (err) => {
     let installable = !err
     addBundled(pkg, (bundleErr) => {
-      var parent = earliestInstallable(tree, tree, pkg, log, null) || tree
+      var parent = earliestInstallable(tree, tree, pkg, log) || tree
       var isLink = pkg._requested.type === 'directory'
       var child = createChild({
         package: pkg,
@@ -755,11 +755,11 @@ function preserveSymlinks () {
 // Find the highest level in the tree that we can install this module in.
 // If the module isn't installed above us yet, that'd be the very top.
 // If it is, then it's the level below where its installed.
-var earliestInstallable = exports.earliestInstallable = function (requiredBy, tree, pkg, log, ignoreChild) {
-  validate('OOOOZ|OOOOO', arguments)
+var earliestInstallable = exports.earliestInstallable = function (requiredBy, tree, pkg, log) {
+  validate('OOOO', arguments)
 
   function undeletedModuleMatches (child) {
-    return child !== ignoreChild && !child.removed && moduleName(child) === pkg.name
+    return !child.removed && moduleName(child) === pkg.name
   }
   const undeletedMatches = tree.children.filter(undeletedModuleMatches)
   if (undeletedMatches.length) {
@@ -778,7 +778,7 @@ var earliestInstallable = exports.earliestInstallable = function (requiredBy, tr
   // If any of the children of this tree have conflicting
   // binaries then we need to decline to install this package here.
   var binaryMatches = pkg.bin && tree.children.some(function (child) {
-    if (child === ignoreChild || child.removed || !child.package.bin) return false
+    if (child.removed || !child.package.bin) return false
     return Object.keys(child.package.bin).some(function (bin) {
       return pkg.bin[bin]
     })
@@ -804,23 +804,6 @@ var earliestInstallable = exports.earliestInstallable = function (requiredBy, tr
 
   if (tree.phantomChildren && tree.phantomChildren[pkg.name]) return null
 
-  // if any of the peer dependencies is a dependency of the current
-  // tree locations, we place the package here. This is a safe location
-  // where we don't violate the peer dependencies contract.
-  // It may not be the perfect location in some cases, but we don't know
-  // if dependencies are hoisted to another location yet, as they
-  // may not be loaded into the tree yet.
-  // We can ignore dev deps here as they are only installed on top-level
-  // and we can't hoist further than that anyway.
-  var peerDeps = pkg.peerDependencies
-  if (peerDeps) {
-    if (Object.keys(peerDeps).some(function (name) {
-      return deps[name]
-    })) {
-      return tree
-    }
-  }
-
   if (tree.isTop) return tree
   if (tree.isGlobal) return tree
 
@@ -829,5 +812,5 @@ var earliestInstallable = exports.earliestInstallable = function (requiredBy, tr
 
   if (!preserveSymlinks() && /^[.][.][\\/]/.test(path.relative(tree.parent.realpath, tree.realpath))) return tree
 
-  return (earliestInstallable(requiredBy, tree.parent, pkg, log, ignoreChild) || tree)
+  return (earliestInstallable(requiredBy, tree.parent, pkg, log) || tree)
 }

test/tap/hoist-peer-dependencies.js

@@ -67,7 +67,7 @@ test('earliestInstallable should consider devDependencies', function (t) {
   dep2a.parent = dep1
   dep2.parent = pkg
 
-  var earliest = earliestInstallable(dep1, dep1, dep2a.package, log, null)
+  var earliest = earliestInstallable(dep1, dep1, dep2a.package, log)
   t.isDeeply(earliest, dep1, 'should hoist package when an incompatible devDependency is present')
   t.end()
 })
@@ -108,58 +108,7 @@ test('earliestInstallable should reuse shared prod/dev deps when they are identi
   dep1.parent = pkg
   dep2.parent = pkg
 
-  var earliest = earliestInstallable(dep1, dep1, dep2.package, log, null)
+  var earliest = earliestInstallable(dep1, dep1, dep2.package, log)
   t.isDeeply(earliest, pkg, 'should reuse identical shared dev/prod deps when installing both')
   t.end()
 })
-
-test('earliestInstallable should consider peerDependencies', function (t) {
-  var dep1 = {
-    children: [],
-    package: {
-      name: 'dep1',
-      dependencies: {
-        dep2: '1.0.0',
-        dep3: '1.0.0'
-      }
-    },
-    path: '/dep1',
-    realpath: '/dep1'
-  }
-
-  var dep2 = {
-    package: {
-      name: 'dep2',
-      version: '1.0.0',
-      peerDependencies: {
-        dep3: '1.0.0'
-      },
-      _requested: npa('dep2@^1.0.0')
-    },
-    parent: dep1,
-    path: '/dep1/node_modules/dep2',
-    realpath: '/dep1/node_modules/dep2'
-  }
-
-  var pkg = {
-    isTop: true,
-    children: [dep1],
-    package: {
-      name: 'pkg',
-      dependencies: { dep1: '1.0.0' }
-    },
-    path: '/',
-    realpath: '/'
-  }
-
-  dep1.parent = pkg
-
-  var earliest = earliestInstallable(dep1, dep1, dep2.package, log, null)
-  t.isDeeply(earliest, dep1, 'should not be able to hoist the package to top-level')
-
-  dep1.children.push(dep2)
-
-  var earliestWithIgnore = earliestInstallable(dep1, dep1, dep2.package, log, dep2)
-  t.isDeeply(earliestWithIgnore, dep1, 'should not be able to hoist the package to top-level')
-  t.end()
-})