-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix: corrects peer dependency flag propagation #8579
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.
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
Conversation
90d9c5e
to
bcd771f
Compare
@owlstronaut I've confirmed that this fixes my simple repro here ... |
The fix also works in our internal repos whose installs failed under |
Thank you so much for reviewing this @jenseng it really helps when folks with domain knowledge do this. |
6623b75
to
cb986c5
Compare
"integrity": "sha512-UlLAnTPrFdNGoFtbSXwcGFQBtQZJCNjaN6hQNP3UPvuNXT1i82N26KL3dZeIpNalWywr9IuQuncaAfUaS1g6sQ==", | ||
"dev": true, | ||
"license": "MIT", | ||
"peer": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wanted to double check some of these and the very first one is good. This is being correctly flagged as a peer, from npm explain @babel/core@7.28.0
:
peer @babel/core@"^7.0.0" from @babel/helper-module-transforms@7.27.3
Summary
Fixes peer dependency flag propagation in npm's dependency resolution system by correcting how
"peer": true
flags are calculated and applied.Problem
Peer dependency flags were inconsistently and incorrectly calculated, leading to incorrect or missing
"peer": true
flags in the ideal tree, which could cause dependency resolution issues.#8431 revealed a number of bugs, the worst of which appears to be that many packages in an ideal tree were marked peer when they shouldn't have been. If they were also optional, they were being removed by this pruning. This is my attempt to make a forward-fix instead of revert the aforementioned correct but also (through no fault of its own) disruptive PR #8431 .
This doesn't solve the problem of legitimate peerOptionals being uninstallable even with
npm i <peer-optional-package>
. It both makes sense for that to be pruned, but also for people that do it to have it either install or warn them. Right now it silently moves along. We could allow it to not be pruned that 1 time by usingexplicitRequests
, but would subsequently be pruned on further installs.Related:
#8464
#8431
#8489