From 44ed242c9a459942257ec1c3ef1a7f1a14a5aab4 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Fri, 14 Aug 2026 15:12:10 +0000 Subject: [PATCH 1/2] fix(arborist): do not fetch packuments for uninstallable optional peer deps --- .../arborist/lib/arborist/build-ideal-tree.js | 5 + .../arborist/build-ideal-tree.js.test.cjs | 114 +++++++++--------- .../test/arborist/build-ideal-tree.js | 31 ++++- 3 files changed, 92 insertions(+), 58 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 6d9ecd1ea555d..5d98a994b434c 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -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') { + continue + } // easy, just put the thing there await this.#nodeFromEdge(edge, node.parent, null, required) continue diff --git a/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs b/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs index e7dc3416004b8..e1ded74cdf83a 100644 --- a/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs @@ -16755,11 +16755,11 @@ 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", }, @@ -16767,47 +16767,24 @@ ArboristNode { "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", }, @@ -16815,36 +16792,57 @@ ArboristNode { "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": "", @@ -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", }, }, @@ -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 { diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 6b1da789deddf..1ff1cffe1404c 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4885,6 +4885,35 @@ 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('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. @@ -4930,7 +4959,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({ From bba1def7b58d379789af7e3b0bae56511278a19e Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Fri, 14 Aug 2026 17:41:43 +0000 Subject: [PATCH 2/2] test: cover peerOptional resolution via other dependents --- .../test/arborist/build-ideal-tree.js | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 1ff1cffe1404c..3d5257148a94c 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -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( + [...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 => { @@ -4914,6 +4924,46 @@ t.test('does not fetch packuments for peerOptional deps that will not be install 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.