Skip to content

Commit a30d855

Browse files
fix(arborist): don't load store packages' devDependencies as required edges (#9633)
Backport of #9626 to `release/v11`. Under `install-strategy=linked`, `npm sbom` exited with `ESBOMPROBLEMS`, reporting transitive packages' devDependencies as missing/required. A store package lives at `node_modules/.store/<key>/node_modules/<pkg>`, which makes it a structural tree top, so `Node._loadDeps` loaded its devDependencies as required edges. `loadActual` now flags such nodes `isInStore` and `_loadDeps` skips devDependencies for them, matching the hoisted strategy. Cherry-picked cleanly except for a test-file context conflict: the unrelated `applies root packageExtensions to a linked actual tree` test (not part of this PR and not present on `release/v11`) was dropped from the resolution; only this fix's two regression tests are included. ## References Backports #9626
1 parent 887ca97 commit a30d855

3 files changed

Lines changed: 65 additions & 3 deletions

File tree

‎workspaces/arborist/lib/arborist/load-actual.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// mix-in implementing the loadActual method
22

3-
const { dirname, join, normalize, relative, resolve } = require('node:path')
3+
const { dirname, join, normalize, relative, resolve, sep } = require('node:path')
44

55
const PackageJson = require('@npmcli/package-json')
66
const { readdirScoped } = require('@npmcli/fs')
@@ -263,6 +263,9 @@ module.exports = cls => class ActualLoader extends cls {
263263
parent,
264264
root,
265265
loadOverrides,
266+
// 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.
267+
// Never flag the loaded root itself, even if its own path happens to sit under a .store directory.
268+
isInStore: path !== this.path && real.includes(`${sep}node_modules${sep}.store${sep}`),
266269
}
267270

268271
try {

‎workspaces/arborist/lib/node.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -927,7 +927,8 @@ class Node {
927927
isTop: srcTop,
928928
path: srcPath,
929929
} = sourceReference || {}
930-
const thisDev = isTop && !globalTop && path
930+
// 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.
931+
const thisDev = isTop && !globalTop && path && !this.isInStore
931932
const srcDev = !sourceReference || srcTop && !srcGlobalTop && srcPath
932933
if (thisDev && srcDev) {
933934
this.#loadDepType(this.package.devDependencies, 'dev', ad)

‎workspaces/arborist/test/arborist/load-actual.js‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ const t = require('tap')
22
const { format } = require('tcompare')
33
const Arborist = require('../../lib/arborist')
44

5-
const { resolve } = require('node:path')
5+
const { join, resolve } = require('node:path')
66
const Node = require('../../lib/node.js')
77
const Shrinkwrap = require('../../lib/shrinkwrap.js')
88
const fs = require('node:fs')
@@ -498,3 +498,61 @@ t.test('loading a workspace maintains overrides', async t => {
498498
const fooEdge = tree.edgesOut.get('foo')
499499
t.equal(tree.overrides, fooEdge.overrides, 'foo edge got the correct overrides')
500500
})
501+
502+
t.test('store nodes do not load devDependencies as required edges', async t => {
503+
// 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).
504+
const path = t.testdir({
505+
'package.json': JSON.stringify({
506+
name: 'root',
507+
version: '1.0.0',
508+
dependencies: { dep: '1.0.0' },
509+
}),
510+
node_modules: {
511+
dep: t.fixture('symlink', '.store/dep@1.0.0/node_modules/dep'),
512+
'.store': {
513+
'dep@1.0.0': {
514+
node_modules: {
515+
dep: {
516+
'package.json': JSON.stringify({
517+
name: 'dep',
518+
version: '1.0.0',
519+
devDependencies: { 'a-dev-dep': '^1.0.0' },
520+
}),
521+
},
522+
},
523+
},
524+
},
525+
},
526+
})
527+
528+
const tree = await loadActual(path)
529+
const dep = tree.children.get('dep').target
530+
t.equal(dep.isInStore, true, 'store node is flagged isInStore')
531+
t.notOk(dep.edgesOut.get('a-dev-dep'), 'devDependency of a store node is not a required edge')
532+
})
533+
534+
t.test('a project located under a .store path still loads its own devDependencies', async t => {
535+
// 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.
536+
const path = t.testdir({
537+
node_modules: {
538+
'.store': {
539+
'root@1.0.0': {
540+
node_modules: {
541+
root: {
542+
'package.json': JSON.stringify({
543+
name: 'root',
544+
version: '1.0.0',
545+
devDependencies: { 'a-dev-dep': '^1.0.0' },
546+
}),
547+
},
548+
},
549+
},
550+
},
551+
},
552+
})
553+
const root = join(path, 'node_modules/.store/root@1.0.0/node_modules/root')
554+
555+
const tree = await loadActual(root)
556+
t.equal(tree.isInStore, false, 'loaded root is not flagged isInStore')
557+
t.ok(tree.edgesOut.get('a-dev-dep'), 'root devDependency is a required edge')
558+
})

0 commit comments

Comments
 (0)