Skip to content

fix: out-of-memory on peers resolution#8457

Merged
zkochan merged 7 commits into
mainfrom
peer-dedupe2
Aug 24, 2024
Merged

fix: out-of-memory on peers resolution#8457
zkochan merged 7 commits into
mainfrom
peer-dedupe2

Conversation

@zkochan

@zkochan zkochan commented Aug 23, 2024

Copy link
Copy Markdown
Member

This change could be considered disrupting but we don't have a choice, the current resolution algorithm causes out of memory errors without this change.

When a peer dependency is a prod dependency somewhere in the dependency graph, pnpm will not attempt to resolve the peers of that peer dependency differently in the subgraph.

For example, we have react-dom in the peer deps of the form and button packages. card has react-dom and react as regular dependencies and card is a dependency of form.

These are the direct dependencies of our example project:

form
react@16
react-dom@16

These are the dependencies of card:

button
react@17
react-dom@16

When resolving peers, pnpm will not re-resolve react-dom for card, even though card shadows react@16 from the root with react@17. So, all 3 packages (form, card, and button) will use react-dom@16 that uses react@16. form will use react@16, while card and button will use react@17.

Before this optimization react-dom@16 was duplicated for the card, so that card and button would use a react-dom@16 instance that uses react@17.

Before the change:

form
-> react-dom@16(react@16)
-> react@16
card
-> react-dom@16(react@17)
-> react@17
button
-> react-dom@16(react@17)
-> react@17

After the change

form
-> react-dom@16(react@16)
-> react@16
card
-> react-dom@16(react@16)
-> react@17
button
-> react-dom@16(react@16)
-> react@17

This might change the lockfile in some projects but I don't think it will break anything.

@zkochan zkochan mentioned this pull request Aug 23, 2024
4 tasks
@zkochan zkochan marked this pull request as ready for review August 23, 2024 14:43
@zkochan zkochan requested a review from a team August 23, 2024 14:43
@gluxon

gluxon commented Sep 8, 2024

Copy link
Copy Markdown
Member

Outside of memory usage, the previous behavior was better — would you agree @zkochan?

The way pnpm ensures a peer dependency is respected through an entire dependency subgraph is one of my favorite features in pnpm because it allows incremental migration to a new major version of a dependency without accidentally leaking incompatible versions between those subgraphs.

Would you be open to me trying to reduce the memory usage of the previous algorithm? I'm not sure if I'll succeed, but if so, would you be open to bringing the original behavior back in pnpm v10?

@dmichon-msft

Copy link
Copy Markdown
Contributor

I can definitively say that this would break a lot of things. My team very much depends on the strictness of nested dependencies getting their peer from the intervening version. Granted, we do our best to mitigate by using .pnpmfile.cjs to enforce that any dependencies that we care about aligning get converted to peerDependencies.

@zkochan

zkochan commented Sep 10, 2024

Copy link
Copy Markdown
Member Author

You can add a setting to return the previous behaviour, I guess. But I would love to see examples of projects where this change actually broke anything first. The change was tested on a wide range of big monorepos and looks like so far so good.

The way pnpm ensures a peer dependency is respected through an entire dependency subgraph is one of my favorite features in pnpm because it allows incremental migration to a new major version of a dependency without accidentally leaking incompatible versions between those subgraphs.

Right, but this change doesn't change anything in how peers are isolated in different projects of the workspace. So, you can still migrate projects in the workspace gradually.

I can definitively say that this would break a lot of things.

If it does break something, please create an issue.


We do try to resolve peers correctly but I feel like peers are overused by the ecosystem and as a result they stopped being singletons. This is a big problem and we need to find a way out from this peer dependency hell. Maybe by providing some more debug info or commands for analyzing peers.

@andersk

andersk commented May 30, 2026

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants