diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js index 96e19a025d41f..7bf9bc1fd6b5e 100644 --- a/workspaces/arborist/lib/node.js +++ b/workspaces/arborist/lib/node.js @@ -1074,7 +1074,7 @@ class Node { // return true if it's safe to remove this node, because anything that // is depending on it would be fine with the thing that they would resolve // to if it was removed, or nothing is depending on it in the first place. - canDedupe (preferDedupe = false) { + canDedupe (preferDedupe = false, explicitRequest = false) { // not allowed to mess with shrinkwraps or bundles if (this.inDepBundle || this.inShrinkwrap) { return false @@ -1117,6 +1117,11 @@ class Node { return true } + // if the other version was an explicit request, then prefer to take the other version + if (explicitRequest) { + return true + } + return false } diff --git a/workspaces/arborist/lib/place-dep.js b/workspaces/arborist/lib/place-dep.js index fca36eb685613..532b529b28b41 100644 --- a/workspaces/arborist/lib/place-dep.js +++ b/workspaces/arborist/lib/place-dep.js @@ -423,7 +423,7 @@ class PlaceDep { // is another satisfying node further up the tree, and if so, dedupes. // Even in installStategy is nested, we do this amount of deduplication. pruneDedupable (node, descend = true) { - if (node.canDedupe(this.preferDedupe)) { + if (node.canDedupe(this.preferDedupe, this.explicitRequest)) { // gather up all deps that have no valid edges in from outside // the dep set, except for this node we're deduping, so that we // also prune deps that would be made extraneous. diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index 65148c9993f80..7b62684a209cb 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/workspaces/arborist/test/arborist/reify.js @@ -3427,3 +3427,83 @@ t.test('install stategy linked', async (t) => { t.ok(abbrev.isSymbolicLink(), 'abbrev got installed') }) }) + +t.test('workspace installs retain existing versions with newer package specs', async t => { + const path = t.testdir({ + 'package.json': JSON.stringify({ + workspaces: [ + 'packages/*', + ], + overrides: { + 'doesnt-matter-can-be-anything': '1.2.3', + }, + }), + packages: { + 'my-cool-package': { + 'package.json': JSON.stringify({}), + }, + 'another-cool-package': { + 'package.json': JSON.stringify({}), + }, + }, + }) + + createRegistry(t, true) + + // Step 1: Install abbrev@1.0.4 in my-cool-package + await reify(path, { + add: ['abbrev@1.0.4'], + // setting savePrefix to '' is exactly what the --save-exact flag does in definitions.js + savePrefix: '', + workspaces: ['my-cool-package'], + }) + + // Verify hoisted installation + const rootNodeModules = resolve(path, 'node_modules/abbrev/package.json') + t.ok(fs.existsSync(rootNodeModules), 'abbrev should be hoisted to root node_modules') + + const hoistedPkg = JSON.parse(fs.readFileSync(rootNodeModules, 'utf8')) + t.equal(hoistedPkg.version, '1.0.4', 'hoisted version should be 1.0.4') + + // Check my-cool-package package.json + const myPackageJson = JSON.parse(fs.readFileSync( + resolve(path, 'packages/my-cool-package/package.json'), 'utf8')) + t.same(myPackageJson.dependencies, { abbrev: '1.0.4' }, + 'my-cool-package should have abbrev@1.0.4 in dependencies') + + // Step 2: Install abbrev@1.1.1 in another-cool-package + await reify(path, { + add: ['abbrev@1.1.1'], + savePrefix: '', + workspaces: ['another-cool-package'], + }) + + // Verify un-hoisted installation + const anotherNodeModules = resolve(path, 'packages/another-cool-package/node_modules/abbrev/package.json') + t.ok(fs.existsSync(anotherNodeModules), 'abbrev@1.1.1 should be installed in another-cool-package/node_modules') + + const unhoistedPkg = JSON.parse(fs.readFileSync(anotherNodeModules, 'utf8')) + t.equal(unhoistedPkg.version, '1.1.1', 'unhoisted version should be 1.1.1') + + // Check another-cool-package package.json + const anotherPackageJson = JSON.parse(fs.readFileSync( + resolve(path, 'packages/another-cool-package/package.json'), 'utf8')) + t.same(anotherPackageJson.dependencies, { abbrev: '1.1.1' }, + 'another-cool-package should have abbrev@1.1.1 in dependencies') + + // Step 3: Install abbrev@1.0.4 in another-cool-package + await reify(path, { + add: ['abbrev@1.0.4'], + savePrefix: '', + workspaces: ['another-cool-package'], + }) + + t.ok(fs.existsSync(rootNodeModules), 'abbrev@1.0.4 should still be hoisted to root node_modules') + t.notOk(fs.existsSync(anotherNodeModules), 'abbrev@1.1.1 should be removed from another-cool-package/node_modules') + + // Check another-cool-package package.json - should now be updated to 1.0.4 + const updatedPackageJson = JSON.parse(fs.readFileSync( + resolve(path, 'packages/another-cool-package/package.json'), 'utf8')) + t.same(updatedPackageJson.dependencies, { abbrev: '1.0.4' }, + 'another-cool-package package.json should be updated to abbrev@1.0.4') +}) diff --git a/workspaces/arborist/test/node.js b/workspaces/arborist/test/node.js index 9a9882ac115a7..7eb6c4eacce01 100644 --- a/workspaces/arborist/test/node.js +++ b/workspaces/arborist/test/node.js @@ -2522,6 +2522,43 @@ t.test('canDedupe()', t => { t.end() }) +t.test('canDedupe returns true when explicitRequest is true regardless of other conditions', t => { + // Create a minimal tree with a valid resolveParent + const root = new Node({ + pkg: { name: 'root', version: '1.0.0' }, + path: '/root', + realpath: '/root', + }) + + // Create a duplicate candidate node in the tree + const duplicate = new Node({ + pkg: { name: 'dup', version: '1.0.0' }, + parent: root, + }) + + // Create a node with the same name but a higher version so that normally dedupe would not occur + const node = new Node({ + pkg: { name: 'dup', version: '2.0.0' }, + parent: duplicate, + }) + + // Manually add an incoming edge so that node.edgesIn is non-empty + node.edgesIn.add({ + from: duplicate, + satisfiedBy () { + return true + }, + }) + + const preferDedupe = false + let explicitRequest = false + t.notOk(node.canDedupe(preferDedupe, explicitRequest), 'without explicit request, dedupe is not allowed') + + explicitRequest = true + t.ok(node.canDedupe(preferDedupe, explicitRequest), 'explicit request forces dedupe to return true') + t.end() +}) + t.test('packageName getter', t => { const node = new Node({ pkg: { name: 'foo' },