Skip to content

fix(arborist): don't fetch packuments for uninstallable optional peer deps #9877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
martinrrm merged 4 commits into from
Aug 20, 2026
Merged

fix(arborist): don't fetch packuments for uninstallable optional peer deps #9877

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
44ed242
fix(arborist): do not fetch packuments for uninstallable optional pee…
danielroe Aug 14, 2026
bba1def
test: cover peerOptional resolution via other dependents
danielroe Aug 14, 2026
8b108c0
fix: check real tree before skipping
danielroe Aug 18, 2026
9dd70e8
test: cover peer optional dep that's satsfied
danielroe Aug 18, 2026
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: 5 additions & 0 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1607,6 +1607,11 @@ This is a one-time fix-up, please be patient...

if (!edge.to) {
if (!parentEdge) {
// a missing peerOptional is valid and will never be placed from
// here, so don't waste a packument fetch resolving it
if (edge.type === 'peerOptional') {
Comment thread
martinrrm marked this conversation as resolved.
continue
}
// easy, just put the thing there
await this.#nodeFromEdge(edge, node.parent, null, required)
continue
Expand Down
114 changes: 57 additions & 57 deletions workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16755,96 +16755,94 @@ ArboristNode {
exports[`test/arborist/build-ideal-tree.js TAP detect conflicts in transitive peerOptional deps nest when peerOptional conflicts > must match snapshot 1`] = `
ArboristNode {
"children": Map {
"@isaacs/test-conflicted-optional-peer-dep-has-peer" => ArboristNode {
"@isaacs/test-conflicted-optional-peer-dep-has-peer-optional" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer",
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer",
"from": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional",
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"spec": "1",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/test-conflicted-optional-peer-dep-peer" => EdgeOut {
"name": "@isaacs/test-conflicted-optional-peer-dep-peer",
"spec": "1",
"spec": "2",
"to": "node_modules/@isaacs/test-conflicted-optional-peer-dep-peer",
"type": "peer",
"type": "peerOptional",
},
},
"location": "node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer",
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer",
"path": "{CWD}/test/fixtures/test-conflicted-optional-peer-dep/nest-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer",
"resolved": "https://registry.npmjs.org/@isaacs/test-conflicted-optional-peer-dep-has-peer/-/test-conflicted-optional-peer-dep-has-peer-1.0.0.tgz",
"location": "node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"path": "{CWD}/test/fixtures/test-conflicted-optional-peer-dep/nest-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"resolved": "https://registry.npmjs.org/@isaacs/test-conflicted-optional-peer-dep-has-peer-optional/-/test-conflicted-optional-peer-dep-has-peer-optional-1.0.0.tgz",
"version": "1.0.0",
},
"@isaacs/test-conflicted-optional-peer-dep-meta-peer" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "",
"name": "@isaacs/test-conflicted-optional-peer-dep-meta-peer",
"spec": "1",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/test-conflicted-optional-peer-dep-has-peer" => EdgeOut {
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer",
"spec": "1",
"to": "node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer",
"type": "prod",
},
},
"location": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer",
"name": "@isaacs/test-conflicted-optional-peer-dep-meta-peer",
"path": "{CWD}/test/fixtures/test-conflicted-optional-peer-dep/nest-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer",
"resolved": "https://registry.npmjs.org/@isaacs/test-conflicted-optional-peer-dep-meta-peer/-/test-conflicted-optional-peer-dep-meta-peer-1.0.0.tgz",
"version": "1.0.0",
},
"@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional" => ArboristNode {
"children": Map {
"@isaacs/test-conflicted-optional-peer-dep-has-peer-optional" => ArboristNode {
"@isaacs/test-conflicted-optional-peer-dep-has-peer" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional",
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"from": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer",
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer",
"spec": "1",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/test-conflicted-optional-peer-dep-peer" => EdgeOut {
"name": "@isaacs/test-conflicted-optional-peer-dep-peer",
"spec": "2",
"to": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-peer",
"type": "peerOptional",
"spec": "1",
"to": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer/node_modules/@isaacs/test-conflicted-optional-peer-dep-peer",
"type": "peer",
},
},
"location": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"path": "{CWD}/test/fixtures/test-conflicted-optional-peer-dep/nest-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"resolved": "https://registry.npmjs.org/@isaacs/test-conflicted-optional-peer-dep-has-peer-optional/-/test-conflicted-optional-peer-dep-has-peer-optional-1.0.0.tgz",
"location": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer/node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer",
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer",
"path": "{CWD}/test/fixtures/test-conflicted-optional-peer-dep/nest-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer/node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer",
"resolved": "https://registry.npmjs.org/@isaacs/test-conflicted-optional-peer-dep-has-peer/-/test-conflicted-optional-peer-dep-has-peer-1.0.0.tgz",
"version": "1.0.0",
},
"@isaacs/test-conflicted-optional-peer-dep-peer" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"from": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer/node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer",
"name": "@isaacs/test-conflicted-optional-peer-dep-peer",
"spec": "2",
"type": "peerOptional",
"spec": "1",
"type": "peer",
},
},
"extraneous": true,
"location": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-peer",
"location": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer/node_modules/@isaacs/test-conflicted-optional-peer-dep-peer",
"name": "@isaacs/test-conflicted-optional-peer-dep-peer",
"optional": true,
"path": "{CWD}/test/fixtures/test-conflicted-optional-peer-dep/nest-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-peer",
"path": "{CWD}/test/fixtures/test-conflicted-optional-peer-dep/nest-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer/node_modules/@isaacs/test-conflicted-optional-peer-dep-peer",
"peer": true,
"resolved": "https://registry.npmjs.org/@isaacs/test-conflicted-optional-peer-dep-peer/-/test-conflicted-optional-peer-dep-peer-2.0.0.tgz",
"version": "2.0.0",
"resolved": "https://registry.npmjs.org/@isaacs/test-conflicted-optional-peer-dep-peer/-/test-conflicted-optional-peer-dep-peer-1.0.0.tgz",
"version": "1.0.0",
},
},
"edgesIn": Set {
EdgeIn {
"from": "",
"name": "@isaacs/test-conflicted-optional-peer-dep-meta-peer",
"spec": "1",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/test-conflicted-optional-peer-dep-has-peer" => EdgeOut {
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer",
"spec": "1",
"to": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer/node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer",
"type": "prod",
},
},
"location": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer",
"name": "@isaacs/test-conflicted-optional-peer-dep-meta-peer",
"path": "{CWD}/test/fixtures/test-conflicted-optional-peer-dep/nest-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer",
"resolved": "https://registry.npmjs.org/@isaacs/test-conflicted-optional-peer-dep-meta-peer/-/test-conflicted-optional-peer-dep-meta-peer-1.0.0.tgz",
"version": "1.0.0",
},
"@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "",
Expand All @@ -16857,7 +16855,7 @@ ArboristNode {
"@isaacs/test-conflicted-optional-peer-dep-has-peer-optional" => EdgeOut {
"name": "@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"spec": "1",
"to": "node_modules/@isaacs/test-conflicted-optional-peer-dep-meta-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"to": "node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"type": "prod",
},
},
Expand All @@ -16870,18 +16868,20 @@ ArboristNode {
"@isaacs/test-conflicted-optional-peer-dep-peer" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer",
"from": "node_modules/@isaacs/test-conflicted-optional-peer-dep-has-peer-optional",
"name": "@isaacs/test-conflicted-optional-peer-dep-peer",
"spec": "1",
"type": "peer",
"spec": "2",
"type": "peerOptional",
},
},
"extraneous": true,
"location": "node_modules/@isaacs/test-conflicted-optional-peer-dep-peer",
"name": "@isaacs/test-conflicted-optional-peer-dep-peer",
"optional": true,
"path": "{CWD}/test/fixtures/test-conflicted-optional-peer-dep/nest-peer-optional/node_modules/@isaacs/test-conflicted-optional-peer-dep-peer",
"peer": true,
"resolved": "https://registry.npmjs.org/@isaacs/test-conflicted-optional-peer-dep-peer/-/test-conflicted-optional-peer-dep-peer-1.0.0.tgz",
"version": "1.0.0",
"resolved": "https://registry.npmjs.org/@isaacs/test-conflicted-optional-peer-dep-peer/-/test-conflicted-optional-peer-dep-peer-2.0.0.tgz",
"version": "2.0.0",
},
},
"edgesOut": Map {
Expand Down
81 changes: 80 additions & 1 deletion workspaces/arborist/test/arborist/build-ideal-tree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2358,6 +2358,16 @@ t.test('detect conflicts in transitive peerOptional deps', async t => {
const name = '@isaacs/test-conflicted-optional-peer-dep-peer'
const peers = tree.inventory.query('name', name)
t.equal(peers.size, 2, 'installed the peer dep twice to avoid conflict')
t.strictSame(
Comment thread
martinrrm marked this conversation as resolved.
Outdated
[...peers].map(p => p.version).sort(),
['1.0.0', '2.0.0'],
'both conflicting versions are present'
)
for (const peer of peers) {
for (const edge of peer.edgesIn) {
t.ok(edge.valid, `edge from ${edge.from.name} is valid`)
}
}
})

await t.test('omit peerOptionals when not needed for conflicts', async t => {
Expand Down Expand Up @@ -4885,6 +4895,75 @@ t.test('circular peer back-off does not crash when node is detached mid-resoluti
'backs off to plugin@1.0.0 to satisfy the optional peer instead of crashing')
})

t.test('does not fetch packuments for peerOptional deps that will not be installed', async t => {
const registry = createRegistry(t, false)

const hostPack = registry.packument({
name: 'host',
version: '1.0.0',
peerDependencies: { plugin: '^1.0.0' },
peerDependenciesMeta: { plugin: { optional: true } },
})
const hostManifest = registry.manifest({ name: 'host', packuments: [hostPack] })
await registry.package({ manifest: hostManifest })

const path = t.testdir({
'package.json': JSON.stringify({
dependencies: { host: '^1.0.0' },
}),
})

const arb = newArb(path)
const tree = await arb.buildIdealTree()

t.equal(tree.children.get('host').version, '1.0.0', 'installed host')
t.equal(tree.children.get('plugin'), undefined, 'did not install the optional peer')
const edge = tree.children.get('host').edgesOut.get('plugin')
t.equal(edge.type, 'peerOptional')
t.equal(edge.to, null, 'peerOptional edge left unresolved')
t.ok(edge.valid, 'missing peerOptional edge is valid')
})

t.test('resolves peerOptional deps installed by another dependent', async t => {
const registry = createRegistry(t, false)

const hostPack = registry.packument({
name: 'host',
version: '1.0.0',
peerDependencies: { plugin: '^1.0.0' },
peerDependenciesMeta: { plugin: { optional: true } },
})
const hostManifest = registry.manifest({ name: 'host', packuments: [hostPack] })
await registry.package({ manifest: hostManifest })

const otherPack = registry.packument({
name: 'other',
version: '1.0.0',
dependencies: { plugin: '^1.0.0' },
})
const otherManifest = registry.manifest({ name: 'other', packuments: [otherPack] })
await registry.package({ manifest: otherManifest })

const pluginManifest = registry.manifest({ name: 'plugin' })
await registry.package({ manifest: pluginManifest })

const path = t.testdir({
'package.json': JSON.stringify({
dependencies: { host: '^1.0.0', other: '^1.0.0' },
}),
})

const arb = newArb(path)
const tree = await arb.buildIdealTree()

const plugin = tree.children.get('plugin')
t.ok(plugin, 'installed the plugin for the dependent that requires it')
const edge = tree.children.get('host').edgesOut.get('plugin')
t.equal(edge.type, 'peerOptional')
t.equal(edge.to, plugin, 'peerOptional edge resolved to the installed plugin')
t.ok(edge.valid, 'peerOptional edge is valid')
})

t.test('peerOptional prefers existing tree node over registry fetch (#9249)', async t => {
// Reproduction: ts-jest has peerOptional jest-util@"^29||^30".
// @types/jest@28 → expect@28 → jest-util@28 placed at root first.
Expand Down Expand Up @@ -4930,7 +5009,7 @@ t.test('peerOptional prefers existing tree node over registry fetch (#9249)', as
// Only publish 28, 29, and 30.
const jestUtilPacks = registry.packuments(['28.0.0', '29.0.0', '30.0.0'], 'jest-util')
const jestUtilManifest = registry.manifest({ name: 'jest-util', packuments: jestUtilPacks })
await registry.package({ manifest: jestUtilManifest, times: 3 })
await registry.package({ manifest: jestUtilManifest, times: 2 })

const path = t.testdir({
'package.json': JSON.stringify({
Expand Down
Loading