From 53ef290f34acca1d1de67bc08e85accdcb77cd17 Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Sun, 24 May 2026 09:33:05 +0900 Subject: [PATCH] fix(arborist): do not materialize self-link for undeclared workspaces In install-strategy=linked, every workspace not listed in root's deps got a self-symlink at /node_modules/ as a side effect of parking its IsolatedLink off-root. Keep the Link in the tree so install scripts and --workspace filters still work, but mark it as tree-only so the reifier skips materialization and the orphan sweep removes any stale self-link left over from an older npm version. Fixes #9398 --- .../arborist/lib/arborist/isolated-reifier.js | 5 +- workspaces/arborist/lib/arborist/reify.js | 10 ++++ workspaces/arborist/test/isolated-mode.js | 51 +++++++++++++++++-- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/workspaces/arborist/lib/arborist/isolated-reifier.js b/workspaces/arborist/lib/arborist/isolated-reifier.js index d0a49c3650636..5f0041e74857e 100644 --- a/workspaces/arborist/lib/arborist/isolated-reifier.js +++ b/workspaces/arborist/lib/arborist/isolated-reifier.js @@ -293,7 +293,8 @@ module.exports = cls => class IsolatedReifier extends cls { root.inventory.set(workspace.location, workspace) root.workspaces.set(wsName, workspace.path) - // Create workspace Link. For root declared deps, link at root node_modules/. For undeclared deps, link at the workspace's own node_modules/ (self-link). + // Declared workspaces are symlinked at root node_modules/. + // Undeclared workspaces get a tree-only Link kept for diff/filter participation but not materialized on disk. const isDeclared = this.#rootDeclaredDeps.has(wsName) const wsLink = new IsolatedLink({ location: isDeclared ? join('node_modules', wsName) : join(c.localLocation, 'node_modules', wsName), @@ -306,7 +307,7 @@ module.exports = cls => class IsolatedReifier extends cls { target: workspace, }) if (!isDeclared) { - workspace.children.set(wsName, wsLink) + wsLink.isUndeclaredWorkspaceLink = true } root.children.set(wsName, wsLink) root.inventory.set(wsLink.location, wsLink) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 0d3a36af6902c..55786fbae4176 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -723,6 +723,12 @@ module.exports = cls => class Reifier extends cls { } // node.isLink + + // Tree-only Link: present in the tree for diff/filter participation, never materialized on disk. + if (node.isUndeclaredWorkspaceLink) { + return + } + await rm(node.path, { recursive: true, force: true }) // symlink @@ -1341,6 +1347,10 @@ module.exports = cls => class Reifier extends cls { if (!child.isLink) { continue } + // Tree-only Links never exist on disk; skipping them lets the sweep remove any stale self-link left by an older npm version. + if (child.isUndeclaredWorkspaceLink) { + continue + } const nmIdx = loc.lastIndexOf(NM_PREFIX) if (nmIdx === -1 || loc.includes(STORE_MARKER)) { continue diff --git a/workspaces/arborist/test/isolated-mode.js b/workspaces/arborist/test/isolated-mode.js index 86bb2f0ea23ed..2be8561806b3f 100644 --- a/workspaces/arborist/test/isolated-mode.js +++ b/workspaces/arborist/test/isolated-mode.js @@ -42,7 +42,8 @@ const rule1 = { apply: (t, dir, resolvedGraph, alreadyAsserted) => { const graph = parseGraph(resolvedGraph) const allPackages = getAllPackages(withRequireChain(graph)) - allPackages.filter(p => p.chain.length !== 0).forEach(p => { + // Workspaces are excluded: linked-strategy workspaces have no self-symlink, so self-referencing requires an `exports` field (matching pnpm). + allPackages.filter(p => p.chain.length !== 0 && !p.workspace).forEach(p => { const resolveChain = [...p.chain, p.name] const key = p.initialDir + ' => ' + resolveChain.join(' => ') if (alreadyAsserted.has(key)) { @@ -1672,8 +1673,8 @@ tap.test('workspace links are not affected by store resolved fix', async t => { const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) await arb2.reify({ installStrategy: 'linked' }) - // Verify workspace is still correctly linked (workspace can resolve itself via self-link) - t.ok(setupRequire(path.join(dir, 'packages', 'mypkg'))('mypkg'), 'workspace is requireable via self-link after second install') + // Verify the workspace's own deps still resolve from inside the workspace after the second install + t.ok(setupRequire(path.join(dir, 'packages', 'mypkg'))('abbrev'), 'workspace dep is requireable from inside workspace after second install') t.ok(setupRequire(dir)('abbrev'), 'registry dep is requireable after second install') // Verify the diff has unchanged nodes (store entries are correctly matched) @@ -2405,6 +2406,50 @@ tap.test('undeclared workspaces are not hoisted to root node_modules', async t = 'ws-c cannot access ws-b (no dependency declared)') }) +tap.test('undeclared workspaces do not get a self-link in their own node_modules', async t => { + // Undeclared workspaces used to be self-symlinked into their own node_modules/. + // Cross-workspace dep links remain unaffected, and stale self-links from older npm versions are swept on the next install. + const graph = { + registry: [ + { name: 'abbrev', version: '1.0.0' }, + ], + root: { + name: 'myapp', + version: '1.0.0', + dependencies: { '@scope/a': '*' }, + }, + workspaces: [ + { name: '@scope/a', version: '1.0.0', dependencies: { '@scope/test': '*', abbrev: '1.0.0' } }, + { name: '@scope/test', version: '1.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 }) + await arborist.reify({ installStrategy: 'linked' }) + + // No self-link inside the undeclared workspace's own node_modules + t.notOk(fs.lstatSync(path.join(dir, 'packages', '@scope', 'test', 'node_modules', '@scope', 'test'), { throwIfNoEntry: false }), + 'undeclared workspace has no self-link in its own node_modules') + + // Cross-workspace dep link still works: @scope/a depends on @scope/test + t.ok(fs.lstatSync(path.join(dir, 'packages', '@scope', 'a', 'node_modules', '@scope', 'test'), { throwIfNoEntry: false })?.isSymbolicLink(), + '@scope/a still has a symlink to @scope/test in its node_modules') + + // Stale self-link from an older npm version must be swept on a subsequent install + fs.mkdirSync(path.join(dir, 'packages', '@scope', 'test', 'node_modules', '@scope'), { recursive: true }) + fs.symlinkSync('../../..', path.join(dir, 'packages', '@scope', 'test', 'node_modules', '@scope', 'test')) + t.ok(fs.lstatSync(path.join(dir, 'packages', '@scope', 'test', 'node_modules', '@scope', 'test'), { throwIfNoEntry: false }), + 'stale self-link planted') + + const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb2.reify({ installStrategy: 'linked' }) + + t.notOk(fs.lstatSync(path.join(dir, 'packages', '@scope', 'test', 'node_modules', '@scope', 'test'), { throwIfNoEntry: false }), + 'stale self-link is removed by the orphan sweep on the next install') +}) + tap.test('omit dev dependencies with linked strategy', async t => { const graph = { registry: [