From 5667d0b96f344bc55b708c7e2f2c9e90f1286787 Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Thu, 25 Jun 2026 23:08:29 +0530 Subject: [PATCH] fix(arborist): load transitive optional deps into linked actual tree (#9654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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`, an installed transitive optional dependency was missing from the actual tree built by `loadActual` when scanning the filesystem (`forceActual: true`), the path `npm sbom` and `npm query` use. On disk the dep is correct — extracted in `.store` and symlinked as a store sibling of its consumer — but `npm sbom` omitted it (e.g. `chokidar` → `fsevents`: 14 components vs the hoisted strategy's 15; `esbuild` → `@esbuild/darwin-arm64` likewise). In `#findMissingEdges()`, the skip condition treated an edge as already resolved when `!edge.missing`. An unresolved optional edge has no target yet reports `missing === false` (`Edge.error` returns `null` for an optional edge with no target), so it was skipped and the on-disk store sibling was never walked or loaded. This changes the check to walk any edge whose target is unresolved, including optional ones. The walk only loads a package that actually exists in an ancestor `node_modules`, so genuinely-uninstalled optionals (impossible platform) stay absent, and behavior is unchanged for required, missing, dummy, and hoisted-ancestor edges. The linked SBOM now matches the hoisted strategy. ## References Fixes #9627 (cherry picked from commit 6a5bf269408b1d2822e5c4d5c126e2aff850c3c2) --- .../arborist/lib/arborist/load-actual.js | 7 ++-- .../arborist/test/arborist/load-actual.js | 37 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/workspaces/arborist/lib/arborist/load-actual.js b/workspaces/arborist/lib/arborist/load-actual.js index 56f473423ca92..7746495b683bb 100644 --- a/workspaces/arborist/lib/arborist/load-actual.js +++ b/workspaces/arborist/lib/arborist/load-actual.js @@ -371,9 +371,10 @@ module.exports = cls => class ActualLoader extends cls { const depPromises = [] for (const [name, edge] of node.edgesOut.entries()) { - const notMissing = !edge.missing && - !(edge.to && (edge.to.dummy || edge.to.parent !== node)) - if (notMissing) { + // An unresolved optional edge reports missing === false, so check the target directly. + // Otherwise an installed optional dep that lives only as a store sibling is never loaded. + const resolved = edge.to && !edge.to.dummy && edge.to.parent === node + if (resolved) { continue } diff --git a/workspaces/arborist/test/arborist/load-actual.js b/workspaces/arborist/test/arborist/load-actual.js index 8ab9133f13d55..c933d4f5fadf1 100644 --- a/workspaces/arborist/test/arborist/load-actual.js +++ b/workspaces/arborist/test/arborist/load-actual.js @@ -531,6 +531,43 @@ t.test('store nodes do not load devDependencies as required edges', async t => { t.notOk(dep.edgesOut.get('a-dev-dep'), 'devDependency of a store node is not a required edge') }) +t.test('loads an installed transitive optional dep from the linked store', async t => { + // A transitive optional dep lives as a store sibling, and its edge reports missing === false despite having no target. + // #findMissingEdges must still walk it, or npm sbom/query omit the installed dep. + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + dependencies: { dep: '1.0.0' }, + }), + node_modules: { + dep: t.fixture('symlink', '.store/dep@1.0.0/node_modules/dep'), + '.store': { + 'dep@1.0.0': { + node_modules: { + dep: { + 'package.json': JSON.stringify({ + name: 'dep', + version: '1.0.0', + optionalDependencies: { opt: '^1.0.0' }, + }), + }, + // the optional dep is installed as a store sibling of its consumer + opt: { 'package.json': JSON.stringify({ name: 'opt', version: '1.0.0' }) }, + }, + }, + }, + }, + }) + + const tree = await loadActual(path) + const dep = tree.children.get('dep').target + const edge = dep.edgesOut.get('opt') + t.ok(edge && !edge.error, 'optional edge resolves') + t.equal(edge.to?.name, 'opt', 'edge resolves to the installed package') + t.ok([...tree.inventory.values()].some(n => n.name === 'opt'), 'opt is present in the inventory') +}) + t.test('a project located under a .store path still loads its own devDependencies', async t => { // The loaded root must never be treated as a store node, even when its own path happens to sit under a node_modules/.store directory. const path = t.testdir({