-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
[feature] hint hoister algorithm #2306
Comments
Shouldn't Yarn be able to solve this by hoisting the "most popular" version of a package? How does it determine the version to hoist at the moment? |
That's tricky tho, because what defines "most popular" - semver-latest? "latest" dist-tag on npm? most downloads reported on the registry? most packages requiring it in the current tree? |
Sorry, I should have clarified. This is what I meant - relative popularity. If 10 dependencies/packages depend on core-js@2 and only one package depends on core-js@1, then core-js@2 should be the one that's hoisted. |
Makes sense - what would be the deterministic tiebreaker? |
You mean if, for example, 5 packages depend on core-js@1 and 5 packages also depend on core-js@2? In that case, I think we should hoist the newest version. |
Just a heads-up we plan to disable Create React App Yarn integration because of this. |
+1 for this! I just tried
Over half a gig worth of dependencies was quite a surprise. That said, it still looks like |
Isn't this fixed by #2676? |
Yes Isn't fixed by #2676 |
#2676 gives an acceptable hoisting structure for most cases based on how often a dependency would be repeated if installed without hoisting. I'd say this is now a low priority until we see some real life examples where it would help. |
Do you want to request a feature or report a bug?
feature
What is the current behavior?
Yarn hoists dependencies consistently but sometimes unpredictable.
A very common version of a package (core-js@2) can fail to hoist on top of node_modules tree if a less common package (core-js@1) got hoisted there first.
What is the expected behavior?
The text was updated successfully, but these errors were encountered: