-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[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
Comments
Possibly related: #1478 |
Probably duplicate of #4688 |
This behavior is correct
As you can see from the output of There are a few ways this can be rectified by the React Spectrum maintainers:
|
@clemyan Thanks so much for detailed response, it’s starting to make sense… Regarding this:
I had the same question. The rationale provided for that was:
Edit: and about this,
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 I plan to:
|
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. |
@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… |
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. |
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'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? |
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:
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 |
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. |
Self-service
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 inreact-spectrum-utils
would cause the bug, and removing it would make it go away.Environment
Additional context
No response
The text was updated successfully, but these errors were encountered: