feat: dedupe packages that have peers when `dedupe-peer-dependents` is `true` by zkochan · Pull Request #6153 · pnpm/pnpm

zkochan · 2023-02-28T18:14:00Z

A new setting is now supported: dedupe-peer-dependents.

When this setting is set to true, packages with peer dependencies will be deduplicated after peers resolution.

For instance, let's say we have a workspace with two projects and both of them have webpack in their dependencies. webpack has esbuild in its optional peer dependencies, and one of the projects has esbuild in its dependencies. In this case, pnpm will link two instances of webpack to the node_modules/.pnpm directory: one with esbuild and another one without it:

node_modules
  .pnpm
    webpack@1.0.0_esbuild@1.0.0
    webpack@1.0.0
project1
  node_modules
    webpack -> ../../node_modules/.pnpm/webpack@1.0.0/node_modules/webpack
project2
  node_modules
    webpack -> ../../node_modules/.pnpm/webpack@1.0.0_esbuild@1.0.0/node_modules/webpack
    esbuild

This makes sense because webpack is used in two projects, and one of the projects doesn't have esbuild, so the two projects cannot share the same instance of webpack. However, this is not what most developers expect, especially since in a hoisted node_modules, there would only be one instance of webpack. Therefore, you may now use the dedupe-peer-dependents setting to deduplicate webpack when it has no conflicting peer dependencies. In this case, if we set dedupe-peer-dependents to true, both projects will use the same webpack instance, which is the one that has esbuild resolved:

node_modules
  .pnpm
    webpack@1.0.0_esbuild@1.0.0
project1
  node_modules
    webpack -> ../../node_modules/.pnpm/webpack@1.0.0_esbuild@1.0.0/node_modules/webpack
project2
  node_modules
    webpack -> ../../node_modules/.pnpm/webpack@1.0.0_esbuild@1.0.0/node_modules/webpack
    esbuild

gluxon · 2023-03-03T05:46:44Z

Strongly agree this is the right direction to go. This is one of the most surprising gotchas today with pnpm.

I was about to propose doing this, but you beat me to it! Thanks. 😊

zkochan · 2023-03-03T06:05:18Z

We can make it default in pnpm v8 but I had to force full resolution when this setting is on. So some performance optimisations will have to be done probably.

zkochan · 2023-03-03T06:06:40Z

It is ready for review.

gluxon · 2023-03-03T06:04:15Z

  for (const { directNodeIdsByAlias, id } of opts.projects) {
    dependenciesByProjectId[id] = mapValues((nodeId) => pathsByNodeId[nodeId], directNodeIdsByAlias)
  }
+  if (opts.dedupePeerDependents) {


It looks like this "patches" the resolution after it's occurred.

That works, but it may be worth discussing an alternative approach that behaves similarly to resolve-peers-from-workspace-root, but instead passes in the entire depGraph rather than only workspace root dependencies. Would that work as well?

I'm wondering if there's a way we an avoid recomputing the dep graph again, and have it deduped already when peersOfChildren is called.

I assume you mean to move the dedupe logic to this file: https://github.com/pnpm/pnpm/blob/main/pkg-manager/resolve-dependencies/src/resolveDependencies.ts

I was thinking about it. It was my first idea. However, at that point we don't know yet that the packages are duplicated (we know maybe the ones that were duplicated during previous install). The packages with peer dependencies are duplicated only after peers resolution, which is the next stage. So we can only deduplicate them after peers resolution.

My first idea was to try to tweak the code that does auto installation of peer dependencies and prefer versions that are already present in the lockfile or are already used to resolve peers. But that was a lot harder to implement and less reliable I believe.

I assume you mean to move the dedupe logic to this file: https://github.com/pnpm/pnpm/blob/main/pkg-manager/resolve-dependencies/src/resolveDependencies.ts

Roughly. To elaborate more, I was thinking of adding the full dep graph to the pkgsByName object above, in addition to rootPkgsByName.

I was thinking about it. It was my first idea. However, at that point we don't know yet that the packages are duplicated (we know maybe the ones that were duplicated during previous install). The packages with peer dependencies are duplicated only after peers resolution, which is the next stage. So we can only deduplicate them after peers resolution.

That's true. Adding more available peers to select from during peers resolution would change the result without knowing whether that change was necessary to dedupe. I find something nice about being able to intuit "If it's in the lockfile, it can be a peer.", but I could also see that being undesirable for complicated cases.

Don't have strong opinions here. Your final solution is good. 👍

Thanks for discussing!

gluxon · 2023-03-03T06:12:24Z

We can make it default in pnpm v8

This option plus auto-install-peers will make pnpm v8 a huge user experience improvement.

but I had to force full resolution when this setting is on.

Ah, is this why the approach dedupes after the initial peers resolution rather than during peers resolution?

So some performance optimisations will have to be done probably.

This version should work and I definitely wouldn't let perfect get in the way!

zkochan · 2023-03-03T06:22:11Z

but I had to force full resolution when this setting is on.

Ah, is this why the approach dedupes after the initial peers resolution rather than during peers resolution?

The issue is that currently we don't dedupe, so when we add a new dependency, we should only analyse a subgraph. And we resolve peer dependencies for the subgraph. We only pass a subgraph to the peers resolver. But now that we dedupe, we always need to pass the whole graph to peers resolver and we will also re-resolve all the peers every time (which might be slow). But I don't see other alternatives. I was thinking about different approaches for 2 or 3 days 😅

gluxon · 2023-03-03T06:47:49Z

I ran into that same problem researching this independently. My draft approach didn't handle --filter'ed installs. 😬

For what it's worth, I think the performance penalty of looking at the entire graph is well worth it.

There is a possible performance improvement given an index mapping dependencies to other dependencies it can be a peer of. That allows pnpm to avoid reanalyzing the entire graph, but that could be complex to implement for small performance gains.

zkochan · 2023-03-03T11:17:18Z

🚢 7.29.0-1

simonihmig · 2023-03-03T11:30:11Z

Is this supposed to fix #6152? (if so we can close the issue I guess)

zkochan · 2023-03-03T11:54:27Z

@simonihmig yes

patroza · 2023-03-03T12:04:47Z

what means conflicting peer dependencies?
Different version ranges, or having multiple peer dependencies of which different ones are installed across different repo packages?

zkochan · 2023-03-03T12:07:38Z

Not different version ranges. The version ranges will be the same. But they are resolved by different versions of the peer.

Like

webpack@1.0.0_esbuild@1.0.0

and

webpack@1.0.0_esbuild@1.1.0_react@18.0.0

In this case we cannot deduplicate webpack because it uses different versions of esbuild.

zanminkian · 2023-03-03T12:17:03Z

Great work! It's hard to explain to others that optional peerDependencies will make framework library save into duplicate copies. After turning on this flag, developer don't need to concern about the peerDependencies. They only need to ensure that the library versions are the same.
Am I correct? @zkochan

zkochan · 2023-03-03T12:18:11Z

Sounds correct.

zkochan · 2023-03-03T12:30:27Z

As @gluxon mentioned, this change basically means that the --filter option will not work with the install command unless the lockfile is up-to-date. I guess it should be fine as most of the people use the --filter command to optimise CI build and in CI the lockfile is up-to-date. Also, people that use the --filter option probably want their projects to be isolated, so it makes sense for them to disable dedupe-peer-dependents.

I need to make another PR that will force resolution on all projects when adding a new dependency to a workspace project. Currently, dedupe will break if you run pnpm add <pkg> in one of the workspace projects.

ref #6153

zanminkian · 2023-03-03T13:11:36Z

It's a little bit complex to understand. Most of developers only want to make sure that, there is only one copy of each dependencies in one application. That means, one application process have only one react instance. Because having more than one react instance in one application will introduce some bug.

I wrote a tool to check.

I hope my tool can help developers like me.

shellscape · 2023-03-03T13:22:11Z

Please make a note of this change in the migration guide for v8

unional · 2023-03-04T00:39:54Z

Have a question about the naming between dependents vs dependencies. Should it be dependencies or there are precedents of using dependents in pnpm?

zkochan · 2023-03-04T00:41:45Z

it should be dependents because the packages that have peer dependencies are duplicated. So, dependents of peer dependencies.

zkochan changed the title ~~fix: automatically install peer dependencies if they are already pres…~~ fix: automatically install peer dependencies if they are already present inside node_modules Feb 28, 2023

zkochan force-pushed the peers-dedupe branch from f2ca25f to c918036 Compare March 1, 2023 23:08

zkochan changed the title ~~fix: automatically install peer dependencies if they are already present inside node_modules~~ fix: deduplicated packages that have peer dependencies Mar 2, 2023

zkochan changed the title ~~fix: deduplicated packages that have peer dependencies~~ fix: deduplicate packages that have peer dependencies Mar 2, 2023

zkochan changed the title ~~fix: deduplicate packages that have peer dependencies~~ feat: deduplicate packages that have peer dependencies Mar 2, 2023

zkochan force-pushed the peers-dedupe branch 2 times, most recently from 5a7511d to 4addac1 Compare March 3, 2023 04:28

zkochan marked this pull request as ready for review March 3, 2023 05:37

zkochan requested a review from gluxon as a code owner March 3, 2023 05:37

fix: dedupe pkgs with peers

326f792

zkochan force-pushed the peers-dedupe branch from 3996c22 to 326f792 Compare March 3, 2023 05:37

docs: add changesets

6e23568

zkochan requested a review from a team March 3, 2023 06:06

zkochan changed the title ~~feat: deduplicate packages that have peer dependencies~~ feat: deduplicate packages that have peer dependencies when dedupe-peer-dependents is true Mar 3, 2023

gluxon approved these changes Mar 3, 2023

View reviewed changes

zkochan changed the title ~~feat: deduplicate packages that have peer dependencies when dedupe-peer-dependents is true~~ feat: dedupe packages that have peers when dedupe-peer-dependents is true Mar 3, 2023

gluxon mentioned this pull request Mar 3, 2023

Non-deterministic install #5106

Closed

zkochan merged commit 5c31fa8 into main Mar 3, 2023

zkochan deleted the peers-dedupe branch March 3, 2023 06:31

zkochan added this to the v7.29 milestone Mar 3, 2023

This comment was marked as outdated.

Sign in to view

simonihmig mentioned this pull request Mar 3, 2023

Add a document about pnpm monorepo error embroider-build/embroider#1349

Merged

This was referenced Mar 3, 2023

Deduplicate packages with peer dependencies #6152

Closed

pnpm install installs multiple copies of the same dependency at the same version #5585

Closed

pnpm install with workspaces causes duplicate modules #2443

Closed

zkochan added a commit that referenced this pull request Mar 3, 2023

docs: add explanation about conflicting peer dependencies

b7ae11c

ref #6153

simonihmig mentioned this pull request Mar 3, 2023

Module not found: Error: Can't resolve '@ember/test-helpers' embroider-build/embroider#1338

Open

zkochan mentioned this pull request Mar 3, 2023

dedupe-peer-dependents sometimes doesn't #6171

Closed

zkochan mentioned this pull request Mar 5, 2023

feat: dedupe peer dependents teambit/bit#7122

Merged

This was referenced Mar 12, 2023

[Draft] Peer Dependency Sharing gluxon/pnpm-rfcs#1

Closed

[Draft] Peer Dependency Sharing pnpm/rfcs#4

Closed

njmh mentioned this pull request May 4, 2023

Issue with styled component using pnpm 7 version #5879

Open

gentoo90 mentioned this pull request Oct 7, 2023

Dependency deduplication seems not to work #7105

Open

4 tasks

astegmaier mentioned this pull request Jun 24, 2026

dedupePeerDependents doesn't deduplicate when there's a cyclic relationship between packages #11834

Open

4 tasks

Uh oh!

Uh oh!

Conversation

zkochan commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gluxon commented Mar 3, 2023

Uh oh!

zkochan commented Mar 3, 2023

Uh oh!

zkochan commented Mar 3, 2023

Uh oh!

gluxon Mar 3, 2023

Choose a reason for hiding this comment

Uh oh!

zkochan Mar 3, 2023

Choose a reason for hiding this comment

Uh oh!

gluxon Mar 3, 2023

Choose a reason for hiding this comment

Uh oh!

gluxon commented Mar 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zkochan commented Mar 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gluxon commented Mar 3, 2023

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

zkochan commented Mar 3, 2023

Uh oh!

simonihmig commented Mar 3, 2023

Uh oh!

zkochan commented Mar 3, 2023

Uh oh!

patroza commented Mar 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zkochan commented Mar 3, 2023

Uh oh!

zanminkian commented Mar 3, 2023

Uh oh!

zkochan commented Mar 3, 2023

Uh oh!

zkochan commented Mar 3, 2023

Uh oh!

zanminkian commented Mar 3, 2023

Uh oh!

shellscape commented Mar 3, 2023

Uh oh!

unional commented Mar 4, 2023

Uh oh!

zkochan commented Mar 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

zkochan commented Feb 28, 2023 •

edited

Loading

gluxon commented Mar 3, 2023 •

edited

Loading

zkochan commented Mar 3, 2023 •

edited

Loading

patroza commented Mar 3, 2023 •

edited

Loading