-
Notifications
You must be signed in to change notification settings - Fork 20
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
Fix bug with recursive bundles depending on items from original modules. #1002
Conversation
As described here the problem is more complex than I usually thought if one wants to minimize the amount of bundled items. So this PR implements a conservative approach (that is much more simple than what we did before): when there is a cycle at the module level, it bundles together in a new module everything the content of all the modules in the cycle. #1005 will track whether this is practical enough and if we should find an algorithm to minimize bundles. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Looks like tests snapshot are not up to date? 🤔 |
Yes, noticed that too. I am looking into it. |
7b8f2ec
to
e0f0bdd
Compare
Fixes a bug found while investigating #995 . More generally this fixes many problems in the bundling of cyclic dependencies