Skip to content

[Bug?]: Duplicate virtual packages #6724

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

Open
1 task
strogonoff opened this issue Mar 12, 2025 · 10 comments
Open
1 task

[Bug?]: Duplicate virtual packages #6724

strogonoff opened this issue Mar 12, 2025 · 10 comments
Labels
bug Something isn't working

Comments

@strogonoff
Copy link
Contributor

strogonoff commented Mar 12, 2025

Self-service

  • I'd be willing to implement a fix

Describe the bug

In some circumstances, Yarn produces two virtual entries for the same package and same version in .pnp.cjs.

This situation causes runtime bugs as packages
end up using two different JavaScript objects,
even as you’re referencing the same import.

Edit: to clarify, the bug arises in the bundle of the Web app, where esbuild (a well-behaved bundler) dutifully outputs code instantiating those distinct JavaScript objects.

To reproduce

https://github.com/riboseinc/possible-yarn-bug-repro

In this case, the bug is triggered by a transitive peer dependency on react-dom. All else equal, adding that dependency in react-spectrum-utils would cause the bug, and removing it would make it go away.

Environment

Node v22.14.0
macOS 15.3.1 ARM M1
Yarn 4.7.0

Additional context

No response

@strogonoff strogonoff added the bug Something isn't working label Mar 12, 2025
@strogonoff
Copy link
Contributor Author

Possibly related: #1478

@strogonoff
Copy link
Contributor Author

Probably duplicate of #4688

@clemyan
Copy link
Member

clemyan commented Mar 14, 2025

This behavior is correct

> yarn info -R --virtuals --dependents @react-spectrum/utils
├─ @react-spectrum/utils@npm:3.12.3
│  ├─ Instances: 2
│  ├─ Version: 3.12.3
│  │ 
│  ├─ Dependents
│  │  ├─ @react-spectrum/accordion@npm:3.0.4
--------------------- snip ---------------------
│  │  └─ @react-spectrum/well@npm:3.4.21
│  │ 
│  └─ Dependencies
│     ├─ @react-aria/i18n@npm:^3.12.7 → npm:3.12.7
│     ├─ @react-aria/ssr@npm:^3.9.7 → npm:3.9.7
│     ├─ @react-aria/utils@npm:^3.28.1 → npm:3.28.1
│     ├─ @react-types/shared@npm:^3.28.0 → npm:3.28.0
│     ├─ @swc/helpers@npm:^0.5.0 → npm:0.5.15
│     └─ clsx@npm:^2.0.0 → npm:2.1.1
│
├─ @react-spectrum/utils@npm:3.12.3 [6cdf7]
│  ├─ Version: 3.12.3
│  │ 
│  ├─ Dependents
│  │  ├─ @react-spectrum/accordion@npm:3.0.4 [eed4f]
--------------------- snip ---------------------
│  │  └─ @react-spectrum/well@npm:3.4.21 [eed4f]
│  │ 
│  └─ Peer dependencies
│     ├─ @types/react-dom@* → ✘
│     ├─ @types/react@* → ✘
│     ├─ react-dom@^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 → npm:18.3.1 [ae24f]
│     └─ react@^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 → npm:18.3.1
│
└─ @react-spectrum/utils@npm:3.12.3 [86ff3]
   ├─ Version: 3.12.3
   │ 
   ├─ Dependents
   │  ├─ @react-spectrum/button@npm:3.16.12 [eed4f]
--------------------- snip ---------------------
   │  └─ @react-spectrum/text@npm:3.5.13 [eed4f]
   │ 
   └─ Peer dependencies
      ├─ @types/react-dom@* → ✘
      ├─ @types/react@* → ✘
      ├─ react-dom@^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 → ✘
      └─ react@^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 → npm:18.3.1

As you can see from the output of yarn info -R --virtuals --dependents @react-spectrum/utils, there are 2 virtual instances of @react-spectrum/utils because one instance has its react-dom peer dependency satisfied (and thus can access it) and the other instance does not.

There are a few ways this can be rectified by the React Spectrum maintainers:

  1. Make sure all instances of @react-spectrum/utils in the tree have access to react-dom. As you can see from the above, a bunch of packages in the React Spectrum ecosystem fails to provide react-dom to @react-spectrum/utils. You can work around this from your end using packageExtensions to add peer dependencies on react-dom to the problematic packages.
  2. If having a single instance of @react-spectrum/utils across multiple packages is important, then those packages each declaring a dependency on @react-spectrum/utils is not correct. Even if the same version of @react-spectrum/utils is resolved for each of those dependencies, package managers do not guarantee a single instance of it. Instead, those packages should declare peer dependencies on @react-spectrum/utils to push the burden of providing @react-spectrum/utils to a single entry point package or the end-developer project itself. This guarantees a single instance because there would be a single package that depends on @react-spectrum/utils.
  3. Looks like @react-spectrum/utils does not use react and react-dom at all? In this case, @react-spectrum/utils can probably drop those peer dependencies entirely.

@clemyan clemyan added the waiting for feedback Will autoclose in a while unless more data are provided label Mar 14, 2025
@strogonoff
Copy link
Contributor Author

strogonoff commented Mar 14, 2025

@clemyan Thanks so much for detailed response, it’s starting to make sense…

Regarding this:

Looks like @react-spectrum/utils does not use react and react-dom at all? In this case, @react-spectrum/utils can probably drop those peer dependencies entirely.

I had the same question. The rationale provided for that was:

It was introduced because it depends on a package below, @react-aria/utils which specifies it as a peer. That package does make use of react-dom.

Edit: and about this,

If having a single instance of @react-spectrum/utils across multiple packages is important

It might be true, because it declares a React context, which is supposed to be a singleton, but I’m not sure whether the bug arises because of multiple instances of @react-spectrum/utils in particular or of react-dom deeper down the line…

I plan to:

  • Expand the repro repository with a functional Web app illustrating a bug in Spectrum-based GUI due to the duplicates

    (On a real-life project I seem to have definitively connected presence and absence of the duplicate virtual entry of the utils package in .pnp.cjs to the bug, but I’d like to be doubly sure…)

    Done.

  • Attempt to implement the changes you described in my local Spectrum build and see if it helps

  • Follow up on a ticket I filed in Spectrum

@dobesv
Copy link

dobesv commented Mar 17, 2025

Yarn's handling of peer dependencies is unfortunately confusing and error-prone. For packages that are intended to be a singleton package, as is common for react components that depend on packages like react, react-dom, styled-components, and so on, it's far too easy to end up with unexpected multiple instances and things break as a result.

This is a huge disadvance of yarn, I hope a solution can be found.

Ideally it should be possible disable the virtual package feature for most packages and replicate whatever npm is doing, and then selectively yarn's virtual package system for packages that have peer dependencies, can safely have multiple instances, and actually run into problems with having just a single instance.

@strogonoff
Copy link
Contributor Author

@dobesv I sort of agree, having encountered this pain as a end developer.

However, I can also see how with more functional packages this behavior may be preferable, where every package is just given whatever it depends on, and multiple instances can help reveal bugs/smelly patterns in packages. I think having that by default is fine, and multiple instances should not trigger an error in most cases, but also a built-in workaround option that de-dupes virtuals without ditching PnP & zero installs would be lovely.

If I understand this all correctly…

@arcanis
Copy link
Member

arcanis commented Mar 17, 2025

Ideally it should be possible disable the virtual package feature for most packages and replicate whatever npm is doing

npm doesn't prevent ghost dependencies, and probably doesn't even pass our testsuites. While it's fine to take a look at what they do in some situations, Yarn's niche is to provide sound, stable, and predictable installs. Even if virtuals can be unwieldy, their behaviour matches those requirements.

That said they are unwieldy, and Yarn should do better to surface and/or block when dependencies are added in a way that causes the amount of virtuals to jump. But that's a UI issue, not a technical one.

@dobesv
Copy link

dobesv commented Mar 17, 2025

That said they are unwieldy, and Yarn should do better to surface and/or block when dependencies are added in a way that causes the amount of virtuals to jump. But that's a UI issue, not a technical one.

Well, yes that's what I'm advocating for here - to have a simple way to block multiple virtual instances. It's probably too late to make this the default.

I can also see how with more functional packages this behavior may be preferable, where every package is just given whatever it depends on, and multiple instances can help reveal bugs/smelly patterns in packages

I'm not sure if "functional" packages would make use of peer dependencies, though? From my understanding the whole point of peer dependencies is that you specifically want to avoid multiple instances of those dependencies, so you have the dependent package supply the version. Otherwise you could just use a normal dependency and let it be merged with the other ones in a compatible range, right?

@strogonoff
Copy link
Contributor Author

strogonoff commented Mar 20, 2025

From my understanding the whole point of peer dependencies is that you specifically want to avoid multiple instances of those dependencies, so you have the dependent package supply the version.

That was my impression, too. Where it seems to create problems is if the dependent package is not the “end project” but is also a dependency, like in React Spectrum. It makes it possible for an inverse problem to arise: package manager creates multiple instances of the package that specified peer deps, one copy per peer dep satisfaction combination by packages up the chain, if I am not mistaken:

  • Package A needs B and C as peers
  • Packages X and Y depend on A
  • Packages X and Y depend on B and C in different ways (e.g., X depends on B and Y depends on C)
  • Your project depends on Z, which depends on X and Y
  • => There are now two copies of package A, one for X & gets B, one for Y & gets C.
  • => This is complicated by deeper trees.

Technically it seems to make sense? The bundled code will run package A as specified in both cases (or will it though? as in, would A version for Y be unable to import B? not sure).

If so it seems reasonable that dependencies should be fixed. If I understand this correctly, then this ticket is not a bug. I’d probably close it once I manage to reproduce suggested steps above (probably via packageExtensions, since the fix in Spectrum pull request doesn’t seem to work for me).

@dobesv
Copy link

dobesv commented Mar 20, 2025

Yes, this is technically not a bug in the sense that it is the intended and documented behavior of yarn. At least from my understanding of it.

You make a good point that it's not enough to use peer dependencies when you want to have a single shared instance of a package. Packages that depend on a package that is using peer dependencies also have to have it as a peer dependency as well, or they have to supply a matching set of dependencies to resolve the peer dependencies the same way as any other package that has that dependency. It's confusing and error prone and has cost me dozens of hours of pain dealing with mismatched dependencies in our monorepo.

@clemyan clemyan removed the waiting for feedback Will autoclose in a while unless more data are provided label Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants