Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion workspaces/arborist/lib/arborist/load-actual.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion workspaces/arborist/lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
60 changes: 59 additions & 1 deletion workspaces/arborist/test/arborist/load-actual.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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')
})
Loading