-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Prevent empty chunks and thoroughly improve experimentalMinChunkSize #4989
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#prevent-empty-chunks or load it into the REPL: |
Codecov Report
@@ Coverage Diff @@
## master #4989 +/- ##
==========================================
- Coverage 98.96% 98.93% -0.04%
==========================================
Files 222 222
Lines 8135 8156 +21
Branches 2239 2242 +3
==========================================
+ Hits 8051 8069 +18
- Misses 30 31 +1
- Partials 54 56 +2
|
a3dd00f
to
3e2264f
Compare
3e2264f
to
b7e90ac
Compare
This PR has been released as part of [email protected]. Note that this is a pre-release, so to test it, you need to install Rollup via |
At least in our case, it doesn't seem like it has any positive effect on the number of chunks. Here is a preview branch. |
For what it is worth, this is how we ended up "working around" the issue. https://gist.github.com/gajus/ff6099064fc094c65e82fbe8ebe71e9f#file-vite-config-ts-L22 |
I see. I would not expect it to produce fewer chunks, but I would hope it groups the chunks better so that with a big |
Concerning our issue, the beta release is indeed resolving it. We are able to remove the Looking forward for the actual release! :-) Thanks @lukastaegert! |
e368de7
to
3ac2750
Compare
This PR has been released as part of [email protected]. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
This started out as a fix for #4962, i.e. unnecessary empty chunks that were created under certain conditions. Note that these are not "facade chunks", i.e. chunks that are created because we do not want to expose unnecessary exports. Those can be avoided via the
preserveEntrySignatures
option.The problem here was caused by an optimization that should actually reduce the number of chunks, or at least the amount of data loaded by each entry. Namely, Rollup resolves re-exports to their original module to improve chunks. I.e. when A imports
foo
from B and B reexportsfoo
from C then Rollup (usually, in the absence of cycles) imports A directly from C. Thus when A and B are entries, A does not need to load the data of B to getfoo
but directly loads C.The basic chunking algorithm now looks only at dependent entries. Without the optimization, - A is loaded by A
Therefore, B and C end up in the same chunk.
After the optimization
Now, B and C end up in separate chunks.
I tried several ways to solve this problem, like skipping the optimization for the chunking, but most attempts caused other bugs and/or completely undermined the spirit of this optimization. So in the end, I decided to solve this with a post-processing step.
And we already have a post-processing step—
output.experimentalMinChunkSize
. So I decided to run an optimized version of this algorithm to always merge empty chunks into other chunks if there is a candidate that would not mess up side effects and does not load unnecessary code for any entry if merged.The advantage is that now, this provides fewer chunks even if in the original code, A directly imports from C.
In the process, I very much improved the
output.experimentalMinChunkSize
itself:Also, it fixes a bug where merging could introduce new side effects into entries via external dependencies. Now we use
moduleSideEffects
to check if an external dependency can be considered side effect free and if not, we treat them like any other side effect. before afterAdditionally, it creates fewer facade chunk as it no longer takes reexports from other chunks or external dependencies into account when determining if a facade needs to be created. before after
cc @gajus