From 5ab259a87711c1e42374f759a57467f128081c59 Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Thu, 25 Jun 2026 15:03:09 +0530 Subject: [PATCH] fix(arborist): surface undeclared workspaces to npm ls under the linked strategy --- lib/commands/ls.js | 35 ++---- test/lib/commands/ls.js | 1 + test/lib/commands/query.js | 21 ++++ .../arborist/lib/arborist/load-actual.js | 61 ++++++--- .../arborist/test/arborist/load-actual.js | 119 ++++++++++++++++++ 5 files changed, 195 insertions(+), 42 deletions(-) diff --git a/lib/commands/ls.js b/lib/commands/ls.js index e222331d09e3e..ac0f4817efec8 100644 --- a/lib/commands/ls.js +++ b/lib/commands/ls.js @@ -135,7 +135,7 @@ class LS extends ArboristWorkspaceCmd { omit, }) : () => true) .filter(installStrategy === 'linked' - ? filterLinkedStrategyEdges({ node, currentDepth }) + ? filterLinkedStrategyEdges({ currentDepth }) : () => true) .map(mapEdgesToNodes({ seenPaths })) .concat(appendExtraneousChildren({ node, seenPaths })) @@ -442,32 +442,15 @@ const getJsonOutputItem = (node, { global, long }) => { return augmentItemWithIncludeMetadata(node, item) } -// In linked strategy, two types of edges produce false UNMET DEPENDENCYs: -// 1. Workspace edges for undeclared workspaces: the lockfile records edges from root to ALL workspaces, but only declared workspaces are hoisted to root/node_modules in linked mode. Undeclared ones are intentionally absent. -// 2. Dev edges on non-root packages: store package link targets have no parent in the node tree, so they are treated as "top" nodes and their devDependencies are loaded as edges. Those devDeps are never installed. -const filterLinkedStrategyEdges = ({ node, currentDepth }) => { - const declaredDeps = new Set(Object.keys(Object.assign({}, - node.target.package.dependencies, - node.target.package.devDependencies, - node.target.package.optionalDependencies, - node.target.package.peerDependencies - ))) - - return (edge) => { - // Skip workspace edges for undeclared workspaces at root level - if (currentDepth === 0 && edge.type === 'workspace' && edge.missing) { - if (!declaredDeps.has(edge.name)) { - return false - } - } - - // Skip dev edges for non-root packages (store packages) - if (currentDepth > 0 && edge.dev) { - return false - } - - return true +// In linked strategy, dev edges on non-root packages produce false UNMET DEPENDENCYs: store package link targets have no parent in the node tree, so they are treated as "top" nodes and their devDependencies are loaded as edges. Those devDeps are never installed. +// Undeclared workspaces no longer need filtering here: loadActual synthesizes their root links so their edges resolve instead of reporting missing. +const filterLinkedStrategyEdges = ({ currentDepth }) => (edge) => { + // Skip dev edges for non-root packages (store packages) + if (currentDepth > 0 && edge.dev) { + return false } + + return true } const filterByEdgesTypes = ({ link, omit }) => (edge) => { diff --git a/test/lib/commands/ls.js b/test/lib/commands/ls.js index cf99c3b889a6f..9f97d33f8d9ca 100644 --- a/test/lib/commands/ls.js +++ b/test/lib/commands/ls.js @@ -5425,6 +5425,7 @@ t.test('ls --install-strategy=linked', async t => { const output = cleanCwd(result()) t.notMatch(output, /UNMET DEPENDENCY/, 'should not report undeclared workspace as UNMET DEPENDENCY') t.match(output, /workspace-a/, 'should list declared workspace') + t.match(output, /workspace-b/, 'should list undeclared workspace (npm/cli#9618)') }) t.test('should not report devDeps of store packages as UNMET DEPENDENCY', async t => { diff --git a/test/lib/commands/query.js b/test/lib/commands/query.js index de3acd0ff8b98..cb9230c93fbcb 100644 --- a/test/lib/commands/query.js +++ b/test/lib/commands/query.js @@ -489,3 +489,24 @@ t.test('missing', async t => { await npm.exec('query', [':missing']) t.matchSnapshot(joinedOutput(), 'should return missing node') }) + +t.test('linked strategy surfaces undeclared workspaces', async t => { + // npm/cli#9618: undeclared workspaces are not symlinked into root node_modules under linked, but must remain visible to `npm query`. + const { npm, joinedOutput } = await loadMockNpm(t, { + config: { 'install-strategy': 'linked' }, + prefixDir: { + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + workspaces: ['packages/*'], + }), + packages: { + a: { 'package.json': JSON.stringify({ name: 'a', version: '1.0.0' }) }, + b: { 'package.json': JSON.stringify({ name: 'b', version: '1.0.0' }) }, + }, + }, + }) + await npm.exec('query', [':root > *']) + const names = JSON.parse(joinedOutput()).map(n => n.name).sort() + t.same(names, ['a', 'b'], 'both undeclared workspaces are listed') +}) diff --git a/workspaces/arborist/lib/arborist/load-actual.js b/workspaces/arborist/lib/arborist/load-actual.js index 17a05939a332a..2e39afb762cff 100644 --- a/workspaces/arborist/lib/arborist/load-actual.js +++ b/workspaces/arborist/lib/arborist/load-actual.js @@ -93,8 +93,6 @@ module.exports = cls => class ActualLoader extends cls { transplantFilter = () => true, ignoreMissing = false, forceActual = false, - // always present: the public loadActual merges this.options, which sets installStrategy - installStrategy, } = options this.#filter = filter this.#transplantFilter = transplantFilter @@ -145,6 +143,7 @@ module.exports = cls => class ActualLoader extends cls { root: this.#actualTree, }) await this[_setWorkspaces](this.#actualTree) + this.#linkActualWorkspaces() this.#transplant(root) return this.#actualTree @@ -175,22 +174,10 @@ module.exports = cls => class ActualLoader extends cls { } } await Promise.all(promises) - - // Linked undeclared workspaces aren't symlinked into root node_modules, so their edges resolve to null and flags never propagate. - // Synthesize the links from the loaded targets. Gated to linked; under hoisted a null workspace edge is a real missing link. - if (installStrategy === 'linked') { - for (const [name, path] of this.#actualTree.workspaces.entries()) { - const edge = this.#actualTree.edgesOut.get(name) - // skip workspaces already linked into root node_modules (declared deps) - if (edge.to) { - continue - } - const target = this.#cache.get(path) - new Link({ parent: this.#actualTree, name, realpath: path, target, pkg: target.package }) - } - } } + this.#linkActualWorkspaces() + // .npm-extension runs before packageExtensions, matching the ideal-tree resolution order await this.#applyNpmExtension() this.#applyPackageExtensions() @@ -235,6 +222,48 @@ module.exports = cls => class ActualLoader extends cls { return this.#actualTree } + // Under the linked (isolated) strategy, workspaces the root does not depend on are not symlinked into the root node_modules, so neither the on-disk scan nor the hidden lockfile gives the root's workspace edges a node_modules/ link to resolve to. + // Synthesize those links from the authoritative workspaces map so npm ls and npm query surface the workspaces, matching hoisted and the logical package-lock. + #linkActualWorkspaces () { + const root = this.#actualTree + if (this.options.installStrategy !== 'linked' || !root.workspaces) { + return + } + // Declared workspaces ARE symlinked at root node_modules under linked, so a missing link there is a real problem that must surface as UNMET. + // Only undeclared workspaces are intentionally absent from root node_modules and need a synthesized link so their root edges resolve. + const pkg = root.package + const declared = new Set(Object.keys(Object.assign({}, + pkg.dependencies, + pkg.devDependencies, + pkg.optionalDependencies, + pkg.peerDependencies + ))) + // index loaded workspace targets by both path and realpath, so a workspace reached through a symlinked dir still matches + const byLoc = new Map() + for (const node of root.fsChildren) { + byLoc.set(node.path, node) + byLoc.set(node.realpath, node) + } + for (const [name, path] of root.workspaces.entries()) { + // declared workspaces, and any name already a root child, resolve their edge without a synthesized link + if (declared.has(name) || root.children.has(name)) { + continue + } + const target = byLoc.get(path) + if (target) { + // eslint-disable-next-line no-new + new Link({ + name, + path: resolve(root.path, 'node_modules', name), + realpath: target.realpath, + target, + parent: root, + root, + }) + } + } + } + #transplant (root) { if (!root || root === this.#actualTree) { return diff --git a/workspaces/arborist/test/arborist/load-actual.js b/workspaces/arborist/test/arborist/load-actual.js index 88511292dd8c2..556d376afe59f 100644 --- a/workspaces/arborist/test/arborist/load-actual.js +++ b/workspaces/arborist/test/arborist/load-actual.js @@ -346,6 +346,125 @@ t.test('linked strategy propagates dep flags into undeclared workspaces', async t.equal(tools.dev, false, 'tools workspace is prod, not overwritten by the dev app link') }) +t.test('linked strategy surfaces undeclared workspaces', async t => { + // Under linked, undeclared workspaces are not symlinked into root node_modules, so loadActual must synthesize their root links for their edges to resolve (npm/cli#9618). + const fixture = { + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + workspaces: ['packages/*'], + // declared workspace is symlinked at root; undeclared one is not + dependencies: { a: '*' }, + }), + node_modules: { + a: t.fixture('symlink', '../packages/a'), + }, + packages: { + a: { 'package.json': JSON.stringify({ name: 'a', version: '1.2.3' }) }, + b: { 'package.json': JSON.stringify({ name: 'b', version: '1.2.3' }) }, + }, + } + + const assertVisible = (t, tree) => { + const aLink = tree.children.get('a') + const bLink = tree.children.get('b') + t.ok(aLink, 'declared workspace a present (from disk symlink)') + t.ok(bLink, 'undeclared workspace b synthesized') + t.equal(tree.edgesOut.get('b').to, bLink, 'workspace edge b resolves to its link') + t.notOk(tree.edgesOut.get('b').missing, 'workspace edge b is not missing') + t.equal(bLink.target.version, '1.2.3', 'b target loaded') + } + + t.test('filesystem scan', async t => { + const path = t.testdir(fixture) + assertVisible(t, await loadActual(path, { installStrategy: 'linked' })) + }) + + t.test('hidden lockfile', async t => { + const path = t.testdir({ + ...fixture, + node_modules: { + a: t.fixture('symlink', '../packages/a'), + '.package-lock.json': JSON.stringify({ + name: 'root', + lockfileVersion: 3, + requires: true, + packages: { + 'node_modules/a': { resolved: 'packages/a', link: true }, + 'packages/a': { version: '1.2.3' }, + 'packages/b': { version: '1.2.3' }, + }, + }), + }, + }) + const hidden = resolve(path, 'node_modules/.package-lock.json') + const then = Date.now() + 10000 + fs.utimesSync(hidden, new Date(then), new Date(then)) + assertVisible(t, await loadActual(path, { installStrategy: 'linked' })) + }) + + t.test('no workspaces is a no-op', async t => { + const path = t.testdir({ + 'package.json': JSON.stringify({ name: 'root', version: '1.0.0' }), + }) + const tree = await loadActual(path, { installStrategy: 'linked' }) + t.notOk(tree.workspaces, 'no workspaces set') + t.equal(tree.children.size, 0, 'no links synthesized') + }) + + t.test('does not clobber an existing root child', async t => { + // an undeclared workspace that already has a root symlink keeps that child instead of a synthesized link + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + workspaces: ['packages/*'], + }), + node_modules: { + b: t.fixture('symlink', '../packages/b'), + }, + packages: { + a: { 'package.json': JSON.stringify({ name: 'a', version: '1.2.3' }) }, + b: { 'package.json': JSON.stringify({ name: 'b', version: '1.2.3' }) }, + }, + }) + const tree = await loadActual(path, { installStrategy: 'linked' }) + t.equal(tree.children.get('b').realpath, resolve(path, 'packages/b'), 'existing b child preserved') + t.ok(tree.children.get('a'), 'undeclared a still synthesized') + }) + + t.test('skips a workspace with no loaded target', async t => { + // hidden lockfile omits packages/b, so it is in the workspaces map but never loaded; it must be skipped, not crash + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + workspaces: ['packages/*'], + }), + node_modules: { + '.package-lock.json': JSON.stringify({ + name: 'root', + lockfileVersion: 3, + requires: true, + packages: { + 'packages/a': { version: '1.2.3' }, + }, + }), + }, + packages: { + a: { 'package.json': JSON.stringify({ name: 'a', version: '1.2.3' }) }, + b: { 'package.json': JSON.stringify({ name: 'b', version: '1.2.3' }) }, + }, + }) + const hidden = resolve(path, 'node_modules/.package-lock.json') + const then = Date.now() + 10000 + fs.utimesSync(hidden, new Date(then), new Date(then)) + const tree = await loadActual(path, { installStrategy: 'linked' }) + t.ok(tree.children.get('a'), 'loaded workspace a synthesized') + t.notOk(tree.children.get('b'), 'unloaded workspace b skipped') + }) +}) + t.test('load workspaces when loading from hidden lockfile', async t => { const path = t.testdir({ 'package.json': JSON.stringify({