Skip to content

Commit 88a8067

Browse files
committed
fix(arborist): workspaces respect overrides on subsequent installs
1 parent 2f5392a commit 88a8067

File tree

2 files changed

+120
-8
lines changed

2 files changed

+120
-8
lines changed

workspaces/arborist/lib/arborist/build-ideal-tree.js

+44-8
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const { lstat, readlink } = require('node:fs/promises')
1313
const { depth } = require('treeverse')
1414
const { log, time } = require('proc-log')
1515
const { redact } = require('@npmcli/redact')
16+
const semver = require('semver')
1617

1718
const {
1819
OK,
@@ -279,14 +280,23 @@ module.exports = cls => class IdealTreeBuilder extends cls {
279280
// When updating all, we load the shrinkwrap, but don't bother
280281
// to build out the full virtual tree from it, since we'll be
281282
// reconstructing it anyway.
282-
.then(root => this.options.global ? root
283-
: !this[_usePackageLock] || this[_updateAll]
284-
? Shrinkwrap.reset({
285-
path: this.path,
286-
lockfileVersion: this.options.lockfileVersion,
287-
resolveOptions: this.options,
288-
}).then(meta => Object.assign(root, { meta }))
289-
: this.loadVirtual({ root }))
283+
.then(root => {
284+
if (this.options.global) {
285+
return root
286+
} else if (!this[_usePackageLock] || this[_updateAll]) {
287+
return Shrinkwrap.reset({
288+
path: this.path,
289+
lockfileVersion: this.options.lockfileVersion,
290+
resolveOptions: this.options,
291+
}).then(meta => Object.assign(root, { meta }))
292+
} else {
293+
return this.loadVirtual({ root })
294+
.then(tree => {
295+
this.#applyRootOverridesToWorkspaces(tree)
296+
return tree
297+
})
298+
}
299+
})
290300

291301
// if we don't have a lockfile to go from, then start with the
292302
// actual tree, so we only make the minimum required changes.
@@ -1475,6 +1485,32 @@ This is a one-time fix-up, please be patient...
14751485
timeEnd()
14761486
}
14771487

1488+
#applyRootOverridesToWorkspaces (tree) {
1489+
const rootOverrides = tree.root.package.overrides || {}
1490+
1491+
for (const node of tree.root.inventory.values()) {
1492+
if (!node.isWorkspace) {
1493+
continue
1494+
}
1495+
1496+
for (const depName of Object.keys(rootOverrides)) {
1497+
const edge = node.edgesOut.get(depName)
1498+
const rootNode = tree.root.children.get(depName)
1499+
1500+
// safely skip if either edge or rootNode doesn't exist yet
1501+
if (!edge || !rootNode) {
1502+
continue
1503+
}
1504+
1505+
const resolvedRootVersion = rootNode.package.version
1506+
if (!semver.satisfies(resolvedRootVersion, edge.spec)) {
1507+
edge.detach()
1508+
node.children.delete(depName)
1509+
}
1510+
}
1511+
}
1512+
}
1513+
14781514
#idealTreePrune () {
14791515
for (const node of this.idealTree.inventory.values()) {
14801516
if (node.extraneous) {

workspaces/arborist/test/arborist/build-ideal-tree.js

+76
Original file line numberDiff line numberDiff line change
@@ -3984,6 +3984,82 @@ t.test('overrides', async t => {
39843984
t.equal(fooBarEdge.valid, true)
39853985
t.equal(fooBarEdge.to.version, '2.0.0')
39863986
})
3987+
3988+
t.test('root overrides should be respected by workspaces on subsequent installs', async t => {
3989+
// • The root package.json declares a workspaces field, a direct dependency on "abbrev" with version constraint "^1.1.1", and an overrides field for "abbrev" also "^1.1.1".
3990+
// • The workspace "onepackage" depends on "abbrev" at "1.0.3".
3991+
const rootPkg = {
3992+
name: 'root',
3993+
version: '1.0.0',
3994+
workspaces: ['onepackage'],
3995+
dependencies: {
3996+
abbrev: '^1.1.1',
3997+
wrappy: '1.0.1',
3998+
},
3999+
overrides: {
4000+
abbrev: '^1.1.1',
4001+
wrappy: '1.0.1',
4002+
},
4003+
}
4004+
const workspacePkg = {
4005+
name: 'onepackage',
4006+
version: '1.0.0',
4007+
dependencies: {
4008+
abbrev: '1.0.3',
4009+
wrappy: '1.0.1',
4010+
},
4011+
}
4012+
4013+
createRegistry(t, true)
4014+
4015+
const dir = t.testdir({
4016+
'package.json': JSON.stringify(rootPkg, null, 2),
4017+
onepackage: {
4018+
'package.json': JSON.stringify(workspacePkg, null, 2),
4019+
},
4020+
})
4021+
4022+
// fresh install
4023+
const tree1 = await buildIdeal(dir)
4024+
4025+
// The ideal tree should resolve "abbrev" at the root to 1.1.1.
4026+
t.equal(
4027+
tree1.children.get('abbrev').package.version,
4028+
'1.1.1',
4029+
'first install: root "abbrev" is forced to version 1.1.1'
4030+
)
4031+
// The workspace "onepackage" should not have its own nested "abbrev".
4032+
const onepackageNode1 = tree1.children.get('onepackage').target
4033+
t.notOk(
4034+
onepackageNode1.children.has('abbrev'),
4035+
'first install: workspace does not install "abbrev" separately'
4036+
)
4037+
4038+
// Write out the package-lock.json to disk to mimic a real install.
4039+
await tree1.meta.save()
4040+
4041+
// Simulate re-running install (which reads the package-lock).
4042+
const tree2 = await buildIdeal(dir)
4043+
4044+
// tree2 should NOT have its own abbrev dependency.
4045+
const onepackageNode2 = tree2.children.get('onepackage').target
4046+
t.notOk(
4047+
onepackageNode2.children.has('abbrev'),
4048+
'workspace should NOT have nested "abbrev" after subsequent install'
4049+
)
4050+
4051+
// The root "abbrev" should still be 1.1.1.
4052+
t.equal(
4053+
tree2.children.get('abbrev').package.version,
4054+
'1.1.1',
4055+
'second install: root "abbrev" is still forced to version 1.1.1')
4056+
4057+
// Overrides should NOT persist unnecessarily
4058+
t.notOk(
4059+
onepackageNode2.overrides && onepackageNode2.overrides.has('abbrev'),
4060+
'workspace node should not unnecessarily retain overrides after subsequent install'
4061+
)
4062+
})
39874063
})
39884064

39894065
t.test('store files with a custom indenting', async t => {

0 commit comments

Comments
 (0)