fix: prefer existing tree nodes for peerOptional deps (#9249) by everett1992 · Pull Request #9283 · npm/cli

everett1992 · 2026-04-26T20:56:44Z

When a peerOptional edge conflicts, search descendants for a satisfying node before fetching from the registry. This prevents extraneous packages from blocking hoisting of required deps.

This fixes 1/2 or 1/3 of #9249. Before this change a clean install would resolve nm/jest-util@30 when resolving the conflict at nm/jest-util between ts-jest's jest-util@^29||^30, and expect's ^28, which had been placed at root. #nodeFromEdge would create a brand new node, matching greatest ^30. A subequent install would mark nm/jest-util@30 as extraneous and prune it.
This tree is valid, but ts-jest's peerOptional jest-util is unsatisfied, while compatible jest-util are installed and duplicated.

This change reduces duplication and can prevent peerOptionals from actively installing.

Now during initial installs npm will prefer hoisting a dependency to de-dupe a peerOptional conflict over creating a new extraneous edge.
It doesn't solve the problem if there's no compatible version in the sub-tree. npm will still use #nodeFromEdge and install an extraneous edge.
It doesn't fix installs from lockfiles generated before this fix. I think this is okay, because the trees are techincally valid, just not optimal.

I think a better solution to all three issues would be:

During problemEdge conflict resolution, npm would hoist nm/jest-util@28 under expect, without replacing it with anything. ts-jest's peerOptional jest-util would be unsatisfied. This creates the same tree as npm's second installs that prune extraneous.
Check for any dependencies that can be hosited. This can run during the initial install on problemEdge conflict resoultion, and in pruneIdealTree on any nodes that are removed.

I think this solves all three issues. I didn't implement it because I couldn't find a way to resolve the conflict by leaving a hole in the tree..

When a peerOptional edge conflicts, search descendants for a satisfying node before fetching from the registry. This prevents extraneous packages from blocking hoisting of required deps.

owlstronaut · 2026-04-28T20:26:47Z

+      if (!dep && edge.type === 'peerOptional') {
+        dep = this.#findHoistableNode(edge.from.resolveParent || edge.from, edge)
+      }


Suggested change

if (!dep && edge.type === 'peerOptional') {

dep = this.#findHoistableNode(edge.from.resolveParent || edge.from, edge)

}

// 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(edge.from.resolveParent || edge.from, edge)

}

I think if we add this check, we get to keep the dedupe behavior for normal installs while still letting npm update, npm audit fix, and explicit installs do their thing.

This is only in problemEdges tho, wouldn't there be other chances earlier to apply requested updates?

Aside: I noticed there is a test that peerOptionals are installed when explicitly requested, and that still passes npm install a-peer-optional.

Tho I noticed that even before this change the test would fail on re-install, or when package-lock or node_modules was already present. That seems bad, npm's only respecting the explciit request when it's not building from a lock.

Oh, I see those count as problems and land here.

owlstronaut · 2026-04-28T20:27:28Z

+      // 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.


Suggested change

// 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.

// 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.

we try to break only at natural boundaries for screen reader accessibility

everett1992 · 2026-04-28T21:50:38Z

@owlstronaut Do you have ideas for alternative fixes? I really want a solution that fixes existing lockfiles, that have the old peerOptional conflict resolution, is an extraneous dependency that will be pruned.

This is causing friction updating to npm 11 (I know, I'm behind the curve).

…, and audit fixes

owlstronaut

I don't have ideas currently about the existing lockfile issue... in the meantime i think this is still a meaningful improvement on its own

owlstronaut · 2026-04-30T18:49:38Z

+  t.ok(peerOptEdge, 'ts-jest has peerOptional edge for jest-util')
+  t.not(peerOptEdge.to?.version, undefined, 'peerOptional jest-util resolved')


Suggested change

t.ok(peerOptEdge, 'ts-jest has peerOptional edge for jest-util')

t.not(peerOptEdge.to?.version, undefined, 'peerOptional jest-util resolved')

t.equal(peerOptEdge.to?.version, '30.0.0', 'peerOptional jest-util refetched to @30, not deduped to @29')

This would pass even without the !skipExistingShortcut guard

everett1992 · 2026-04-30T20:05:24Z

I don't have ideas currently about the existing lockfile issue... in the meantime i think this is still a meaningful improvement on its own

What do you think about running a dedupe pass when nodes are deleted by idealTreePrune?
Arborist could check for children of siblings that can be hoisted and replace the node.

owlstronaut · 2026-05-04T15:41:48Z

I don't have ideas currently about the existing lockfile issue... in the meantime i think this is still a meaningful improvement on its own

What do you think about running a dedupe pass when nodes are deleted by idealTreePrune? Arborist could check for children of siblings that can be hoisted and replace the node.

That seems like a promising direction. I'm worried that'll get expensive on big trees, but I'm open to a dedupe pass given we could profile the performance appropriately

github-actions · 2026-05-04T15:43:34Z

⚠️ Backport to release/v11 failed.

This usually means the cherry-pick had conflicts. Please create a manual backport:

git fetch origin release/v11
git checkout -b backport/v11/9283 origin/release/v11
git cherry-pick -x 0629fbf736eafcb555428d96bd86a69f8e791d70
# resolve any conflicts, then:
git push origin backport/v11/9283

Error details

Command failed: git cherry-pick -x 0629fbf736eafcb555428d96bd86a69f8e791d70
error: could not apply 0629fbf73... fix: prefer existing tree nodes for peerOptional deps (#9249) (#9283)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"

When a peerOptional edge conflicts, search descendants for a satisfying node before fetching from the registry. This prevents extraneous packages from blocking hoisting of required deps. This fixes 1/2 or 1/3 of #9249. Before this change a clean install would resolve `nm/jest-util@30` when resolving the conflict at nm/jest-util between ts-jest's jest-util@^29||^30, and expect's ^28, which had been placed at root. `#nodeFromEdge` would create a brand new node, matching greatest ^30. A subequent install would mark nm/jest-util@30 as extraneous and prune it. This tree is valid, but ts-jest's peerOptional jest-util is unsatisfied, while compatible jest-util are installed and duplicated. This change reduces duplication and can prevent peerOptionals from actively installing. 1. Now during initial installs npm will prefer hoisting a dependency to de-dupe a peerOptional conflict over creating a new extraneous edge. 2. It doesn't solve the problem if there's no compatible version in the sub-tree. npm will still use `#nodeFromEdge` and install an extraneous edge. 3. It doesn't fix installs from lockfiles generated before this fix. I think this is okay, because the trees are techincally valid, just not optimal. I think a better solution to all three issues would be: * During problemEdge conflict resolution, npm would hoist nm/jest-util@28 under expect, without replacing it with anything. ts-jest's peerOptional jest-util would be unsatisfied. This creates the same tree as npm's second installs that prune extraneous. * Check for any dependencies that can be hosited. This can run during the initial install on problemEdge conflict resoultion, and in pruneIdealTree on any nodes that are removed. I think this solves all three issues. I didn't implement it because I couldn't find a way to resolve the conflict by leaving a hole in the tree.. (cherry picked from commit 0629fbf)

fix: prefer existing tree nodes for peerOptional deps (npm#9249)

ff3515b

When a peerOptional edge conflicts, search descendants for a satisfying node before fetching from the registry. This prevents extraneous packages from blocking hoisting of required deps.

everett1992 requested a review from a team as a code owner April 26, 2026 20:56

owlstronaut self-requested a review April 28, 2026 15:17

owlstronaut reviewed Apr 28, 2026

View reviewed changes

fix: skip peerOptional dedupe shortcut for explicit updates, installs…

fea8328

…, and audit fixes

everett1992 force-pushed the fix-9220-prefer-existing branch from 9970730 to 9b96929 Compare April 30, 2026 17:07

owlstronaut reviewed Apr 30, 2026

View reviewed changes

test: cover skipExistingShortcut branch for peerOptional dedupe

13270e2

everett1992 force-pushed the fix-9220-prefer-existing branch from 9b96929 to 13270e2 Compare April 30, 2026 20:02

owlstronaut self-assigned this May 1, 2026

owlstronaut added the backport:v11 label May 4, 2026

owlstronaut approved these changes May 4, 2026

View reviewed changes

owlstronaut merged commit 0629fbf into npm:latest May 4, 2026
18 checks passed

github-actions Bot mentioned this pull request May 4, 2026

chore: release 12.0.0-pre.0.0 #9229

Open

owlstronaut removed the backport:v11 label May 4, 2026

owlstronaut mentioned this pull request May 4, 2026

[release/v11] fix: prefer existing tree nodes for peerOptional deps (#9249) #9301

Merged

github-actions Bot mentioned this pull request May 1, 2026

chore: release 11.14.0 #9273

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: prefer existing tree nodes for peerOptional deps (#9249)#9283

fix: prefer existing tree nodes for peerOptional deps (#9249)#9283
owlstronaut merged 3 commits intonpm:latestfrom
everett1992:fix-9220-prefer-existing

everett1992 commented Apr 26, 2026

Uh oh!

owlstronaut Apr 28, 2026

Uh oh!

everett1992 Apr 28, 2026

Uh oh!

everett1992 Apr 28, 2026

Uh oh!

owlstronaut Apr 28, 2026

Uh oh!

everett1992 commented Apr 28, 2026

Uh oh!

owlstronaut left a comment •

edited

Loading

Uh oh!

owlstronaut Apr 30, 2026

Uh oh!

everett1992 commented Apr 30, 2026

Uh oh!

owlstronaut commented May 4, 2026

Uh oh!

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-      if (!dep && edge.type === 'peerOptional') {
-        dep = this.#findHoistableNode(edge.from.resolveParent || edge.from, edge)
-      }
+       // 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(edge.from.resolveParent || edge.from, edge)
+       }

		t.ok(peerOptEdge, 'ts-jest has peerOptional edge for jest-util')
		t.not(peerOptEdge.to?.version, undefined, 'peerOptional jest-util resolved')

	t.ok(peerOptEdge, 'ts-jest has peerOptional edge for jest-util')
	t.not(peerOptEdge.to?.version, undefined, 'peerOptional jest-util resolved')
	t.equal(peerOptEdge.to?.version, '30.0.0', 'peerOptional jest-util refetched to @30, not deduped to @29')

Conversation

everett1992 commented Apr 26, 2026

Uh oh!

owlstronaut Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

everett1992 Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

everett1992 Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

owlstronaut Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

everett1992 commented Apr 28, 2026

Uh oh!

owlstronaut left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

owlstronaut Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

everett1992 commented Apr 30, 2026

Uh oh!

owlstronaut commented May 4, 2026

Uh oh!

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

owlstronaut left a comment •

edited

Loading