diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index bc509ca47944c..e91e74d7c0107 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -898,8 +898,21 @@ This is a one-time fix-up, please be patient... // be forced to agree on a version of z. const required = new Set([edge.from]) const parent = edge.peer ? virtualRoot : null - const dep = vrDep && vrDep.satisfies(edge) ? vrDep - : await this.#nodeFromEdge(edge, parent, null, required) + let dep = vrDep && vrDep.satisfies(edge) ? vrDep : null + + // A peerOptional conflict can be resolved by finding an existing node in the tree that satisfies the edge, avoiding a registry fetch that may introduce an extraneous package. See npm/cli#9249. + // Skip the shortcut when the user has signaled an explicit re-fetch intent (npm update by name, explicit request, or audit fix), so we honor those signals rather than silently keeping the existing node. + const skipExistingShortcut = this[_updateNames].includes(edge.name) + || this.#explicitRequests.has(edge) + || (edge.to && this.auditReport?.isVulnerable(edge.to)) + if (!dep && edge.type === 'peerOptional' && !skipExistingShortcut) { + dep = this.#findHoistableNode( + /* istanbul ignore next - resolveParent is always set for non-root nodes */ + edge.from.resolveParent || edge.from, edge) + } + if (!dep) { + dep = await this.#nodeFromEdge(edge, parent, null, required) + } /* istanbul ignore next */ debug(() => { @@ -1029,6 +1042,24 @@ This is a one-time fix-up, please be patient... return this.#buildDepStep() } + // BFS descendants of `root` for a node satisfying `edge`. + // Prefers nodes closer to root. Skips bundled nodes. + #findHoistableNode (root, edge) { + const queue = [...root.children.values()] + while (queue.length) { + const node = queue.shift() + if (node.name === edge.name + && !node.inDepBundle + && node.satisfies(edge)) { + return node + } + for (const child of node.children.values()) { + queue.push(child) + } + } + return null + } + // loads a node from an edge, and then loads its peer deps (and their peer deps, on down the line) into a virtual root parent. async #nodeFromEdge (edge, parent_, secondEdge, required) { // create a virtual root node with the same deps as the node that is requesting this one, so that we can get all the peer deps in a context where they're likely to be resolvable. diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index d1e1895baa6ee..270040425d58a 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4482,6 +4482,140 @@ t.test('skip invalid peerOptional edges in problemEdges when save=false (#8726)' t.ok(tree.children.get('util'), 'util is in the tree') }) +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. + // jest@29 → jest-util@29 nested (root slot taken by @28). + // ts-jest re-queued, peerOptional jest-util resolves to root @28 → INVALID. + // Without fix: #nodeFromEdge fetches jest-util@30 (latest ^29||^30), blocks @29. + // With fix: #findHoistableNode finds nested @29, PlaceDep hoists it to root. + const registry = createRegistry(t, false) + + const jestPack = registry.packument({ + name: 'jest', + version: '29.0.0', + dependencies: { 'jest-util': '^29.0.0' }, + }) + const jestManifest = registry.manifest({ name: 'jest', packuments: [jestPack] }) + await registry.package({ manifest: jestManifest }) + + const tsJestPack = registry.packument({ + name: 'ts-jest', + version: '29.0.0', + peerDependencies: { jest: '^29.0.0', 'jest-util': '^29.0.0 || ^30.0.0' }, + peerDependenciesMeta: { 'jest-util': { optional: true } }, + }) + const tsJestManifest = registry.manifest({ name: 'ts-jest', packuments: [tsJestPack] }) + await registry.package({ manifest: tsJestManifest }) + + const expectPack = registry.packument({ + name: 'expect', + version: '28.0.0', + dependencies: { 'jest-util': '^28.0.0' }, + }) + const expectManifest = registry.manifest({ name: 'expect', packuments: [expectPack] }) + await registry.package({ manifest: expectManifest }) + + const atTypesPack = registry.packument({ + name: '@types/jest', + version: '28.0.0', + dependencies: { expect: '^28.0.0' }, + }) + const atTypesManifest = registry.manifest({ name: '@types/jest', packuments: [atTypesPack] }) + await registry.package({ manifest: atTypesManifest }) + + // 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 }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + dependencies: { + jest: '^29.0.0', + 'ts-jest': '^29.0.0', + '@types/jest': '^28.0.0', + }, + }), + }) + + const arb = newArb(path) + const tree = await arb.buildIdealTree() + + // jest-util@29 at root — found via #findHoistableNode, not fetched as @30 + t.equal(tree.children.get('jest-util').version, '29.0.0', + 'jest-util@29 hoisted to root from nested location') + + // ts-jest's peerOptional resolved to @29 from the tree, not @30 from registry + const tsJest = tree.children.get('ts-jest') + const peerOptEdge = tsJest.edgesOut.get('jest-util') + t.equal(peerOptEdge.to.version, '29.0.0', + 'ts-jest peerOptional jest-util resolved to @29') + + // jest-util@28 nested under expect (incompatible with root @29) + const expectNode = [...tree.inventory.query('name', 'expect')][0] + t.equal(expectNode?.children?.get('jest-util')?.version, '28.0.0', + 'jest-util@28 nested under expect') +}) + +t.test('peerOptional skips dedupe shortcut when update.names includes the dep', async t => { + // Same scenario as above, but with update: { names: ['jest-util'] }. + // skipExistingShortcut=true so #findHoistableNode is NOT called; + // #nodeFromEdge fetches from registry, getting jest-util@30 (latest matching ^29||^30). + const registry = createRegistry(t, false) + + const jestPack = registry.packument({ + name: 'jest', + version: '29.0.0', + dependencies: { 'jest-util': '^29.0.0' }, + }) + await registry.package({ manifest: registry.manifest({ name: 'jest', packuments: [jestPack] }) }) + + const tsJestPack = registry.packument({ + name: 'ts-jest', + version: '29.0.0', + peerDependencies: { jest: '^29.0.0', 'jest-util': '^29.0.0 || ^30.0.0' }, + peerDependenciesMeta: { 'jest-util': { optional: true } }, + }) + await registry.package({ manifest: registry.manifest({ name: 'ts-jest', packuments: [tsJestPack] }) }) + + const expectPack = registry.packument({ + name: 'expect', + version: '28.0.0', + dependencies: { 'jest-util': '^28.0.0' }, + }) + await registry.package({ manifest: registry.manifest({ name: 'expect', packuments: [expectPack] }) }) + + const atTypesPack = registry.packument({ + name: '@types/jest', + version: '28.0.0', + dependencies: { expect: '^28.0.0' }, + }) + await registry.package({ manifest: registry.manifest({ name: '@types/jest', packuments: [atTypesPack] }) }) + + const jestUtilPacks = registry.packuments(['28.0.0', '29.0.0', '30.0.0'], 'jest-util') + await registry.package({ manifest: registry.manifest({ name: 'jest-util', packuments: jestUtilPacks }), times: 3 }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + dependencies: { + jest: '^29.0.0', + 'ts-jest': '^29.0.0', + '@types/jest': '^28.0.0', + }, + }), + }) + + const arb = newArb(path) + const tree = await arb.buildIdealTree({ update: { names: ['jest-util'] } }) + + // With skipExistingShortcut=true, #nodeFromEdge fetches from registry + // so jest-util@30 (latest matching ^29||^30) is used instead of deduping @29 + const tsJest = tree.children.get('ts-jest') + const peerOptEdge = tsJest.edgesOut.get('jest-util') + t.equal(peerOptEdge.to?.version, '30.0.0', 'peerOptional jest-util refetched to @30, not deduped to @29') +}) + t.test('overrides with bundledDependencies', async t => { t.test('does not infinite loop with bundledDependencies and overrides', async t => { // https://github.com/npm/cli/issues/9227