diff --git a/lib/dedupe.js b/lib/dedupe.js index fae409fea572d..325faeaabcd43 100644 --- a/lib/dedupe.js +++ b/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)], diff --git a/lib/install/deps.js b/lib/install/deps.js index 1cf4de15dc3f1..c36265093b090 100644 --- a/lib/install/deps.js +++ b/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) } diff --git a/test/tap/hoist-peer-dependencies.js b/test/tap/hoist-peer-dependencies.js deleted file mode 100644 index 75d7d3e778ada..0000000000000 --- a/test/tap/hoist-peer-dependencies.js +++ /dev/null @@ -1,130 +0,0 @@ -'use strict' -const path = require('path') -const fs = require('fs') -const test = require('tap').test -const Tacks = require('tacks') -const File = Tacks.File -const Dir = Tacks.Dir -const common = require('../common-tap.js') - -const basedir = path.join(__dirname, path.basename(__filename, '.js')) -const testdir = path.join(basedir, 'testdir') -const cachedir = path.join(basedir, 'cache') -const globaldir = path.join(basedir, 'global') -const tmpdir = path.join(basedir, 'tmp') - -const conf = { - cwd: testdir, - env: common.newEnv().extend({ - npm_config_cache: cachedir, - npm_config_tmp: tmpdir, - npm_config_prefix: globaldir, - npm_config_registry: common.registry, - npm_config_loglevel: 'warn' - }) -} - -const fixture = new Tacks(Dir({ - cache: Dir(), - global: Dir(), - tmp: Dir(), - testdir: Dir({ - package: Dir({ - 'package.json': File({ - name: 'package', - version: '1.0.0', - dependencies: { - 'base-dep': '../packages/base-dep-1.0.0.tgz', - 'plugin-dep': '../packages/plugin-dep-1.0.0.tgz' - } - }) - }), - 'package.json': File({ - version: '1.0.0', - dependencies: { - package: 'file:./package', - 'base-dep': './packages/base-dep-2.0.0.tgz' - } - }), - // file: dependencies do not work as they are symlinked and behave - // differently. Instead installation from tgz files is used. - packages: Dir({ - 'base-dep-1.0.0.tgz': File(Buffer.from( - '1f8b080000000000000a2b484cce4e4c4fd52f80d07a59c5f9790c540606' + - '06066666660ad8c4c1c0d45c81c1d8d4ccc0d0d0ccccc0448101c8303505' + - 'd1d4760836505a5c925804740aa5e640bca200a78708a8e6525050ca4bcc' + - '4d55b252504a4a2c4ed54d492d50d2018996a5161567e6e781240cf50cf4' + - '0c94b86ab906dab9a360148c8251300aa80400a44d97d100080000', - 'hex' - )), - 'base-dep-2.0.0.tgz': File(Buffer.from( - '1f8b080000000000000a2b484cce4e4c4fd52f80d07a59c5f9790c540606' + - '06066666660ad8c4c1c0d45c81c1d8d4ccc0d0d0ccccc0448101c8303505' + - 'd1d4760836505a5c925804740aa5e640bca200a78708a8e6525050ca4bcc' + - '4d55b252504a4a2c4ed54d492d50d2018996a5161567e6e781248cf40cf4' + - '0c94b86ab906dab9a360148c8251300aa80400aebbeeba00080000', - 'hex' - )), - 'plugin-dep-1.0.0.tgz': File(Buffer.from( - '1f8b080000000000000aed8f3d0e83300c8599394594b904476d3274e622' + - '295888fe8488408756dcbd0e513bb115a9aa946f79ce7bb1653b535f4c8b' + - 'a58b2acebeb7d9c60080d69aadf90119b2bdd220a98203cba8504a916ebd' + - 'c81a931fcd40ab7c3b27dec23efa273c73c6b83537e447c6dd756a3b5b34' + - 'e8f82ef8771c7cd7db10490102a2eb10870a1dda066ddda1a7384ca1e464' + - '3c2eddd42044f97e164bb318db07a77f733ee7bfbe3a914824122f4e04e9' + - '2e00080000', - 'hex' - )) - }) - }) -})) - -function setup () { - cleanup() - fixture.create(basedir) -} - -function cleanup () { - fixture.remove(basedir) -} - -test('setup', t => { - setup() - return common.fakeRegistry.listen() -}) - -test('example', t => { - return common.npm(['install'], conf).then(([code, stdout, stderr]) => { - t.is(code, 0, 'command ran ok') - t.comment(stdout.trim()) - t.comment(stderr.trim()) - t.ok(fs.existsSync(path.join(testdir, 'node_modules')), 'did install') - var packageLock = JSON.parse(fs.readFileSync(path.join(testdir, 'package-lock.json'), 'utf8')) - t.similar(packageLock, { - dependencies: { - 'base-dep': { - version: 'file:packages/base-dep-2.0.0.tgz' - }, - 'package': { - version: 'file:package', - dependencies: { - 'base-dep': { - version: 'file:packages/base-dep-1.0.0.tgz' - }, - // plugin-dep must not placed on top-level - 'plugin-dep': { - version: 'file:packages/plugin-dep-1.0.0.tgz' - } - } - } - } - }, 'locks dependencies as unhoisted') - t.similar(Object.keys(packageLock.dependencies), ['base-dep', 'package'], 'has correct packages on top-level') - }) -}) - -test('cleanup', t => { - common.fakeRegistry.close() - cleanup() - t.done() -}) diff --git a/test/tap/unit-deps-earliestInstallable.js b/test/tap/unit-deps-earliestInstallable.js index 136e6478c586a..47d1ab4119b1e 100644 --- a/test/tap/unit-deps-earliestInstallable.js +++ b/test/tap/unit-deps-earliestInstallable.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() -})