From 026be939952377c85995cec6a7dfde15837ca50e Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Fri, 26 Jun 2026 01:44:07 +0530 Subject: [PATCH] fix(arborist): surface undeclared workspaces under the linked strategy (#9657) In continuation of our exploration of using `install-strategy=linked` in the [Gutenberg monorepo](https://github.com/WordPress/gutenberg/pull/75814), which powers the WordPress Block Editor. Under `install-strategy=linked`, a declared workspace that the root does not depend on is installed on disk but was invisible to `npm ls --all`, `npm ls --workspaces`, and `npm query ':root > *'`, all of which reported an empty tree. The hoisted strategy lists every workspace. Undeclared workspaces are intentionally not symlinked into the root `node_modules` under the linked strategy, so when `loadActual` rebuilds the actual tree (from the filesystem scan or the hidden lockfile, neither of which records a `node_modules/` entry) the root's `workspace` edges resolve to nothing and the workspaces drop out of the tree that `ls` and `query` traverse. `npm ls` then hid those missing edges entirely to avoid a false `UNMET DEPENDENCY`. The fix adds `loadActual` post-processing that, for the linked strategy, synthesizes an in-memory `Link` at `node_modules/` for each undeclared workspace so its root edge resolves, matching the logical layout the regular `package-lock.json` already records. This is introspection-only and writes nothing to disk. A declared workspace whose root link is genuinely missing is left alone so it still reports `UNMET`, an existing root child of the same name is never replaced, and a workspace that was not loaded is skipped. With the edges now resolving, the workspace-skip branch that `npm ls` used to suppress them is removed. Fixes #9618 --- lib/commands/ls.js | 37 ++---- 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, 196 insertions(+), 43 deletions(-) diff --git a/lib/commands/ls.js b/lib/commands/ls.js index f2ecdc5b92c0a..8b9b4f3f5d2e2 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 })) @@ -400,33 +400,16 @@ 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) - /* istanbul ignore next: store packages no longer carry dev edges, so this guard is not exercised by tests */ - 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) + /* istanbul ignore next: store packages no longer carry dev edges, so this guard is not exercised by tests */ + 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 17733bc03ef3f..899a2b8169051 100644 --- a/test/lib/commands/ls.js +++ b/test/lib/commands/ls.js @@ -5358,6 +5358,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 30a4ee9b16c6e..bf73856ec2ddc 100644 --- a/workspaces/arborist/lib/arborist/load-actual.js +++ b/workspaces/arborist/lib/arborist/load-actual.js @@ -91,8 +91,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 @@ -143,6 +141,7 @@ module.exports = cls => class ActualLoader extends cls { root: this.#actualTree, }) await this[_setWorkspaces](this.#actualTree) + this.#linkActualWorkspaces() this.#transplant(root) return this.#actualTree @@ -173,22 +172,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() + if (!ignoreMissing) { await this.#findMissingEdges() } @@ -229,6 +216,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 17770c58dd367..f699fa594f557 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({