From 0c872efd2d66ecb7c2f65e7bc2a295a177cc1d9b Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Wed, 24 Jun 2026 20:10:02 +0530 Subject: [PATCH] fix(arborist): don't load store packages' devDependencies as required edges (#9626) 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`, `npm sbom` exited non-zero with `ESBOMPROBLEMS`, reporting the devDependencies of transitive packages (e.g. `matcha`, `tape`) as `missing: ... required by ...`. Those dev dependencies are correctly not installed (the same is true under hoisted), yet only the linked strategy treated them as missing-and-required. Hoisted produced a clean SBOM for the same dependency set. A package in the linked strategy's store lives at `node_modules/.store//node_modules/`, which makes it a structural tree top (`isTop`, no parent). `Node._loadDeps` loads devDependencies for every top node, so each store package — a transitive dependency whose devDependencies are never installed — gained required `dev` edges. `npm sbom` reads the filesystem tree via `loadActual`, queries it, and flags every non-optional missing edge, so those spurious dev edges surfaced as `ESBOMPROBLEMS`. Standalone `npm audit` was unaffected because it audits the virtual tree from the lockfile. Hoisted was unaffected because its transitive packages have a real parent, so they are not tops and never load devDependencies. `load-actual.js` now flags a node as `isInStore` when its realpath sits inside a `node_modules/.store/` directory, matching the flag the isolated reifier already sets on ideal-tree store nodes. `Node._loadDeps` excludes store nodes from the devDependency load (`isTop && !globalTop && path && !this.isInStore`), so a store package — a transitive dependency by definition — never gains required dev edges. This makes the linked actual tree's edge semantics match hoisted. Fixes #9610 Part of #9608 --- .../arborist/lib/arborist/load-actual.js | 5 +- workspaces/arborist/lib/node.js | 3 +- .../arborist/test/arborist/load-actual.js | 60 ++++++++++++++++++- 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/workspaces/arborist/lib/arborist/load-actual.js b/workspaces/arborist/lib/arborist/load-actual.js index 2356227a5baa3..56f473423ca92 100644 --- a/workspaces/arborist/lib/arborist/load-actual.js +++ b/workspaces/arborist/lib/arborist/load-actual.js @@ -1,6 +1,6 @@ // mix-in implementing the loadActual method -const { dirname, join, normalize, relative, resolve } = require('node:path') +const { dirname, join, normalize, relative, resolve, sep } = require('node:path') const PackageJson = require('@npmcli/package-json') const { readdirScoped } = require('@npmcli/fs') @@ -263,6 +263,9 @@ module.exports = cls => class ActualLoader extends cls { parent, root, loadOverrides, + // A package physically located in the linked strategy's store is a transitive dependency, not a real tree top, so it must not load its devDependencies. + // Never flag the loaded root itself, even if its own path happens to sit under a .store directory. + isInStore: path !== this.path && real.includes(`${sep}node_modules${sep}.store${sep}`), } try { diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js index 451ede1e52cbf..87b0ac41740bd 100644 --- a/workspaces/arborist/lib/node.js +++ b/workspaces/arborist/lib/node.js @@ -927,7 +927,8 @@ class Node { isTop: srcTop, path: srcPath, } = sourceReference || {} - const thisDev = isTop && !globalTop && path + // A package in the linked strategy's .store is a transitive dependency that is structurally a tree top, but its devDependencies are never installed or required, so they must not be loaded. + const thisDev = isTop && !globalTop && path && !this.isInStore const srcDev = !sourceReference || srcTop && !srcGlobalTop && srcPath if (thisDev && srcDev) { this.#loadDepType(this.package.devDependencies, 'dev', ad) diff --git a/workspaces/arborist/test/arborist/load-actual.js b/workspaces/arborist/test/arborist/load-actual.js index 94ad4e7269286..8ab9133f13d55 100644 --- a/workspaces/arborist/test/arborist/load-actual.js +++ b/workspaces/arborist/test/arborist/load-actual.js @@ -2,7 +2,7 @@ const t = require('tap') const { format } = require('tcompare') const Arborist = require('../../lib/arborist') -const { resolve } = require('node:path') +const { join, resolve } = require('node:path') const Node = require('../../lib/node.js') const Shrinkwrap = require('../../lib/shrinkwrap.js') const fs = require('node:fs') @@ -498,3 +498,61 @@ t.test('loading a workspace maintains overrides', async t => { const fooEdge = tree.edgesOut.get('foo') t.equal(tree.overrides, fooEdge.overrides, 'foo edge got the correct overrides') }) + +t.test('store nodes do not load devDependencies as required edges', async t => { + // A package in the linked store is structurally a tree top, so without the isInStore guard its devDependencies would load as required edges and surface as missing (e.g. npm sbom ESBOMPROBLEMS). + 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', + devDependencies: { 'a-dev-dep': '^1.0.0' }, + }), + }, + }, + }, + }, + }, + }) + + const tree = await loadActual(path) + const dep = tree.children.get('dep').target + t.equal(dep.isInStore, true, 'store node is flagged isInStore') + t.notOk(dep.edgesOut.get('a-dev-dep'), 'devDependency of a store node is not a required edge') +}) + +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({ + node_modules: { + '.store': { + 'root@1.0.0': { + node_modules: { + root: { + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + devDependencies: { 'a-dev-dep': '^1.0.0' }, + }), + }, + }, + }, + }, + }, + }) + const root = join(path, 'node_modules/.store/root@1.0.0/node_modules/root') + + const tree = await loadActual(root) + t.equal(tree.isInStore, false, 'loaded root is not flagged isInStore') + t.ok(tree.edgesOut.get('a-dev-dep'), 'root devDependency is a required edge') +})