Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 18 additions & 21 deletions workspaces/arborist/lib/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -808,28 +808,23 @@ module.exports = cls => class Reifier extends cls {
// Combined Map keyed by path (how allChildren() in diff.js keys)
const combined = new Map()

// Add actual tree's children (the top-level symlinks)
for (const child of actualTree.children.values()) {
combined.set(child.path, child)
}

// Add synthetic entries for store nodes and store links that exist on disk.
// The proxy tree is flat: all store entries (isInStore) and store links (isStoreLink) are direct children of root.
// The actual tree only has top-level links as root children, so store entries need synthetic actual entries for the diff to match them.
// Create synthetic actual entries for ALL ideal children that exist on disk.
// The isolated ideal tree is flat (all entries as root children), but loadActual() produces a nested tree where workspace deps are under fsChildren and store entries are deep link targets.
// Synthetic entries ensure the diff compares matching resolved/integrity values (e.g. workspace links have resolved=undefined in the ideal tree but resolved="file:../packages/..." in the actual tree).
for (const child of idealTree.children.values()) {
if (!combined.has(child.path) && (child.isInStore || child.isStoreLink) &&
existsSync(child.path)) {
let entry
if (child.isLink) {
entry = new IsolatedLink(child)
} else {
entry = new IsolatedNode(child)
}
if (child.isLink && combined.has(child.realpath)) {
entry.target = combined.get(child.realpath)
}
combined.set(child.path, entry)
if (combined.has(child.path) || !existsSync(child.path)) {
continue
}
let entry
if (child.isLink) {
entry = new IsolatedLink(child)
} else {
entry = new IsolatedNode(child)
}
if (child.isLink && combined.has(child.realpath)) {
entry.target = combined.get(child.realpath)
}
combined.set(child.path, entry)
}

// Proxy .get(name) to original actual tree for filterNodes compatibility
Expand All @@ -850,7 +845,9 @@ module.exports = cls => class Reifier extends cls {
wrapper.binPaths = actualTree.binPaths
wrapper.children = combined
wrapper.edgesOut = actualTree.edgesOut
wrapper.fsChildren = actualTree.fsChildren
// Use empty fsChildren so that allChildren() only picks up entries from the combined map.
// The actual fsChildren have real children with different resolved values (e.g. file:../../../node_modules/.store/... vs file:.store/...) that would overwrite our synthetic entries in allChildren().
wrapper.fsChildren = new Set()
wrapper.integrity = actualTree.integrity
wrapper.inventory = actualTree.inventory

Expand Down
37 changes: 37 additions & 0 deletions workspaces/arborist/test/isolated-mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -1863,6 +1863,43 @@ tap.test('workspace links are not affected by store resolved fix', async t => {
t.ok(arb2.diff.unchanged.length > 0, 'second install should have unchanged nodes')
})

tap.test('idempotent install with cross-workspace deps (diamond pattern)', async t => {
// Regression: when workspace-x depends on workspace-y (and both share a registry dep), the second install would report "changed N packages" because (1) workspace link resolved values didn't match between ideal and actual trees, and (2) actual fsChildren overwrote synthetic store entries in the diff proxy.
const graph = {
registry: [
{ name: 'abbrev', version: '2.0.0' },
],
root: {
name: 'myproject',
version: '1.0.0',
dependencies: { 'workspace-x': '*', 'workspace-y': '*' },
},
workspaces: [
{ name: 'workspace-x', version: '1.0.0', dependencies: { abbrev: '2.0.0', 'workspace-y': '*' } },
{ name: 'workspace-y', version: '1.0.0', dependencies: { abbrev: '2.0.0' } },
],
}
const { dir, registry } = await getRepo(graph)
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)

// First install
const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arb1.reify({ installStrategy: 'linked' })

// Second install — should detect everything is up-to-date
const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arb2.reify({ installStrategy: 'linked' })

const leaves = arb2.diff?.leaves || []
const actions = leaves.filter(l => l.action)
t.equal(actions.length, 0, 'second install should have no diff actions')
t.ok(arb2.diff.unchanged.length > 0, 'second install should have unchanged nodes')

// Verify packages are still correctly installed (abbrev is a workspace dep, not root)
t.ok(setupRequire(path.join(dir, 'packages', 'workspace-y'))('abbrev'),
'abbrev is requireable from workspace after second install')
})

tap.test('orphaned store entries are cleaned up on dependency update', async t => {
const graph = {
registry: [
Expand Down
Loading