-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[node-modules] Support reducing trees requiring multiple hoisting passes to a terminal result #2394
Conversation
574c5cb
to
d629c22
Compare
Maybe yarn could do a single pass by default, and try again with multiple if there is a linking error? |
Yeah, I'm still investigating why these multiple passes are really needed, maybe all these multiple passes can be really done in a single pass.... |
974aea3
to
ce866cc
Compare
What's the problem this PR addresses?
Fixes issue:
babel/babel#12583 (comment)
The issue happens because the dependency graph was not fully hoisted, which resulted in an unlucky situation when the place was temporary occupied in a single workspace by conflicting versions of the same dependency.
How did you fix it?
The simplified example of a dependency graph requiring multiple hoisting passes is below:
We try to hoist everything we can to the
.
node first, we cannot hoist anythingThen we try to hoist everything we can to
A
(C@Z
has a priority, because its more popular), we haveAnd now we can hoist
C@Z
fromA
, but we need another pass to do it, because we need to try to hoist to the root again, and the final result will be:I have fixed the issue by adding support of detecting the case when multiple rounds of hoisting are needed. The algorithm stops when detection yields no results, thus the minimum number of required rounds are performed. I have also added to a self-check the validation whether result is actually terminal, by doing another hoisting round and checking whether any hoisting was performed during this additional round.
Checklist