Fix handling of virtual packages in Yarn2#11208
Conversation
`@react-native-community:netinfo:11.2.1` was used to test the handling of virtual packages which Yarn2+ creates when a package has peer dependencies. Replace it with `@csstools:css-color-parser:3.0.8` which in addition to peer dependencies has also normal dependencies to demonstrate the bug that `Yarn2` currently misses the transitive dependencies of such packages. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>
When a package has peer dependencies, Yarn2+ creates a virtual package for it that is used also in the output of `yarn info`. This virtual package only contains the peer dependencies, while only the non-virtual package contains the transitive dependencies. Fix the issue that transitive dependencies of such packages are ignored by mapping locators that point to virtual packages to locators that point to the non-virtual packages. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11208 +/- ##
=========================================
Coverage 57.47% 57.48%
- Complexity 1704 1705 +1
=========================================
Files 346 346
Lines 12855 12860 +5
Branches 1222 1223 +1
=========================================
+ Hits 7389 7393 +4
Misses 4992 4992
- Partials 474 475 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| .map { dependency -> | ||
| // Map virtual package locators to their corresponding real package locators as only their | ||
| // package infos contain the transitive dependencies. | ||
| val locator = virtualPackageRegex.find(dependency.locator)?.let { |
There was a problem hiding this comment.
This new code could be moved into a function (next to the regex) to keep this block more readable. E.g. fun getRealPackageLocator(locator: String): String
| } ?: dependency.locator | ||
|
|
||
| packageInfoForLocator.getValue(locator) | ||
| } |
There was a problem hiding this comment.
IIUC This change only works for "direct" dependencies.
A similar logic needs to be put into Yarn2DependencyHandler.dependenciesFor().
How about taking in my above comment mentioned function, and define it as extension property PackageInfo.realLocator next to the other ones in Yarn2DependencyHandler ?
Ideally, the test should be enhanced to have such virtual package as non-direct dependency too.
There was a problem hiding this comment.
...Maybe there is a chance, that the cycle issue goes away with this..
2 participants
Adapt a Yarn2 test project to demonstrate the issue described in #8058 (and #11073) and a fix for the problem.
The PR is currently kept a draft because it triggers the issue demonstrated in #11207 for some projects.