diff --git a/workspaces/arborist/lib/arborist/isolated-reifier.js b/workspaces/arborist/lib/arborist/isolated-reifier.js index 8da64ce965b7e..08255313bcb20 100644 --- a/workspaces/arborist/lib/arborist/isolated-reifier.js +++ b/workspaces/arborist/lib/arborist/isolated-reifier.js @@ -267,7 +267,7 @@ module.exports = cls => class IsolatedReifier extends cls { } const root = { - fsChildren: [], + fsChildren: new Set(), integrity: null, inventory: new Map(), isLink: false, @@ -286,20 +286,24 @@ module.exports = cls => class IsolatedReifier extends cls { meta: { loadedFromDisk: false }, global: false, isProjectRoot: true, - children: [], + children: new Map(), + workspaces: new Map(), + tops: new Set(), + linksIn: new Set(), } - // root.inventory.set('', t) - // root.meta = this.idealTree.meta - // TODO We should mock better the inventory object because it is used by audit-report.js ... maybe + root.inventory.set('', root) + // TODO inventory.query is a stub; audit-report needs 'packageName' support root.inventory.query = () => { return [] } const processed = new Set() proxiedIdealTree.workspaces.forEach(c => { + const wsName = c.packageName const workspace = { edgesIn: new Set(), edgesOut: new Map(), - children: [], + children: new Map(), + fsChildren: new Set(), hasInstallScript: c.hasInstallScript, binPaths: [], package: c.package, @@ -307,9 +311,42 @@ module.exports = cls => class IsolatedReifier extends cls { path: c.localPath, realpath: c.localPath, resolved: c.resolved, + isLink: false, + isRoot: false, + name: wsName, + linksIn: new Set(), } - root.fsChildren.push(workspace) + workspace.target = workspace + root.fsChildren.add(workspace) root.inventory.set(workspace.location, workspace) + + // Create workspace Link entry in children for _diffTrees lookup + const wsLink = { + name: wsName, + location: join('node_modules', wsName), + path: join(root.path, 'node_modules', wsName), + realpath: workspace.path, + isLink: true, + target: workspace, + children: new Map(), + fsChildren: new Set(), + edgesIn: new Set(), + edgesOut: new Map(), + binPaths: [], + root, + parent: root, + isRoot: false, + isProjectRoot: false, + isTop: false, + global: false, + globalTop: false, + package: workspace.package, + linksIn: new Set(), + } + root.children.set(wsLink.name, wsLink) + root.inventory.set(wsLink.location, wsLink) + root.workspaces.set(wsName, workspace.path) + workspace.linksIn.add(wsLink) }) const generateChild = (node, location, pkg, inStore) => { const newChild = { @@ -321,11 +358,11 @@ module.exports = cls => class IsolatedReifier extends cls { name: node.packageName || node.name, optional: node.optional, top: { path: proxiedIdealTree.root.localPath }, - children: [], + children: new Map(), edgesIn: new Set(), edgesOut: new Map(), binPaths: [], - fsChildren: [], + fsChildren: new Set(), /* istanbul ignore next -- emulate Node */ getBundler () { return null @@ -343,7 +380,7 @@ module.exports = cls => class IsolatedReifier extends cls { package: pkg, } newChild.target = newChild - root.children.push(newChild) + root.children.set(newChild.location, newChild) root.inventory.set(newChild.location, newChild) } proxiedIdealTree.external.forEach(c => { @@ -379,10 +416,10 @@ module.exports = cls => class IsolatedReifier extends cls { let from, nmFolder if (externalEdge) { const fromLocation = join('node_modules', '.store', key, 'node_modules', node.packageName) - from = root.children.find(c => c.location === fromLocation) + from = root.children.get(fromLocation) nmFolder = join('node_modules', '.store', key, 'node_modules') } else { - from = node.isProjectRoot ? root : root.fsChildren.find(c => c.location === node.localLocation) + from = node.isProjectRoot ? root : root.inventory.get(node.localLocation) nmFolder = join(node.localLocation, 'node_modules') } /* istanbul ignore next - strict-peer-deps can exclude nodes from the tree */ @@ -401,9 +438,9 @@ module.exports = cls => class IsolatedReifier extends cls { let target if (external) { const toLocation = join('node_modules', '.store', toKey, 'node_modules', dep.packageName) - target = root.children.find(c => c.location === toLocation) + target = root.children.get(toLocation) } else { - target = root.fsChildren.find(c => c.location === dep.localLocation) + target = root.inventory.get(dep.localLocation) } // TODO: we should no-op is an edge has already been created with the same fromKey and toKey /* istanbul ignore next - strict-peer-deps can exclude nodes from the tree */ @@ -430,8 +467,8 @@ module.exports = cls => class IsolatedReifier extends cls { name: toKey, resolved: dep.resolved, top: { path: dep.root.localPath }, - children: [], - fsChildren: [], + children: new Map(), + fsChildren: new Set(), isLink: true, isStoreLink: true, isRoot: false, @@ -444,7 +481,7 @@ module.exports = cls => class IsolatedReifier extends cls { const newEdge2 = { optional: false, from: link, to: target } link.edgesOut.set(dep.name, newEdge2) target.edgesIn.add(newEdge2) - root.children.push(link) + root.children.set(link.location, link) } for (const dep of node.localDependencies) { diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 5f376e94a4cec..dc82a51da54b3 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -146,7 +146,7 @@ module.exports = cls => class Reifier extends cls { // was not changed, delete anything in the ideal and not actual. // Then we move the entire idealTree over to this.actualTree, and // save the hidden lockfile. - if (this.diff && this.diff.filterSet.size) { + if (this.diff && this.diff.filterSet.size && !linked) { const reroot = new Set() const { filterSet } = this.diff @@ -422,7 +422,7 @@ module.exports = cls => class Reifier extends cls { if (includeWorkspaces) { // add all ws nodes to filterNodes for (const ws of this.options.workspaces) { - const ideal = this.idealTree.children.get && this.idealTree.children.get(ws) + const ideal = this.idealTree.children.get(ws) if (ideal) { filterNodes.push(ideal) } diff --git a/workspaces/arborist/lib/diff.js b/workspaces/arborist/lib/diff.js index 465657cc62422..5a20749ad135d 100644 --- a/workspaces/arborist/lib/diff.js +++ b/workspaces/arborist/lib/diff.js @@ -71,6 +71,7 @@ class Diff { tree: filterNode, visit: node => filterSet.add(node), getChildren: node => { + const orig = node node = node.target const loc = node.location const idealNode = ideal.inventory.get(loc) @@ -87,7 +88,12 @@ class Diff { } } - return ideals.concat(actuals) + const result = ideals.concat(actuals) + // Include link targets so store entries end up in filterSet + if (orig.isLink) { + result.push(node) + } + return result }, }) } diff --git a/workspaces/arborist/test/isolated-mode.js b/workspaces/arborist/test/isolated-mode.js index cd8434e638966..3c2b14bbb6e68 100644 --- a/workspaces/arborist/test/isolated-mode.js +++ b/workspaces/arborist/test/isolated-mode.js @@ -1601,6 +1601,62 @@ tap.test('postinstall scripts run once for store packages', async t => { t.equal(count, 1, 'postinstall ran exactly once') }) +tap.test('workspace-filtered install with linked strategy', async t => { + // Two workspaces sharing the same dependency must not crash when installing with --workspace + --install-strategy=linked. + const graph = { + registry: [ + { name: 'abbrev', version: '2.0.0' }, + ], + root: { + name: 'myroot', version: '1.0.0', + }, + workspaces: [ + { name: 'ws-a', version: '1.0.0', dependencies: { abbrev: '2.0.0' } }, + { name: 'ws-b', version: '1.0.0', dependencies: { abbrev: '2.0.0' } }, + ], + } + + const { dir, registry } = await getRepo(graph) + + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + + // Full install first + await arborist.reify({ installStrategy: 'linked' }) + + // Verify store entry exists + const storeDir = path.join(dir, 'node_modules', '.store') + const storeEntries = fs.readdirSync(storeDir) + t.ok(storeEntries.some(e => e.startsWith('abbrev@')), 'store has abbrev entry after full install') + + // Workspace-filtered install must not crash + const arborist2 = new Arborist({ + path: dir, + registry, + packumentCache: new Map(), + cache, + workspaces: ['ws-a'], + }) + await arborist2.reify({ + installStrategy: 'linked', + workspaces: ['ws-a'], + }) + + // Verify workspace filtering was actually applied (not silently skipped) + t.ok(arborist2.diff.filterSet.size > 0, 'workspace filter was applied to diff') + + // Store entries still intact + const storeEntries2 = fs.readdirSync(storeDir) + t.ok(storeEntries2.some(e => e.startsWith('abbrev@')), 'store entries preserved after ws install') + + // Workspace symlinks preserved + const wsALink = fs.readlinkSync(path.join(dir, 'packages', 'ws-a', 'node_modules', 'abbrev')) + t.ok(wsALink.includes('.store'), 'workspace a abbrev symlink points to store') + + const wsBLink = fs.readlinkSync(path.join(dir, 'packages', 'ws-b', 'node_modules', 'abbrev')) + t.ok(wsBLink.includes('.store'), 'workspace b abbrev symlink preserved') +}) + tap.test('bins are installed', async t => { // Input of arborist const graph = {