Prevent empty chunks and thoroughly improve experimentalMinChunkSize

[lukastaegert]

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers:

• resolves Treeshaking multiple entry modules with cross-referencing creates extra chunks #4962

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 reexports foo 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 get foo but directly loads C.

The basic chunking algorithm now looks only at dependent entries. Without the optimization, - A is loaded by A

• B is loaded by A and B
• C is loaded by A and B

Therefore, B and C end up in the same chunk.

After the optimization

• A is loaded by A
• B is loaded by B
• C is loaded by A and B

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:

• It should now be faster as we are now using BigInt Set operations in several places to avoid loops
• Instead of some vague heuristics, we now calculate how much unnecessary code will be loaded by any entry after a merge and pick the best merge candidate chunk based on that metric.
• The size calculation has been very much improved as we do not take the size of the full original code but instead just sum up the sizes of the included top-level statements in each module. This is still (usually) far off the final numbers as minification is not applied, but it now ignores imports, exports, and other tree-shaken code as well as white-space and comments between (but not within) top-level statements.

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 after

Additionally, 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

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
rollup	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 16, 2023 7:31pm

[github-actions]

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:
https://rollup-gcxmzufgt-rollup-js.vercel.app/repl/?pr=4989

[codecov]

Codecov Report

Merging #4989 (e368de7) into master (5b29267) will decrease coverage by 0.04%.
The diff coverage is 99.16%.

❗ Current head e368de7 differs from pull request most recent head 3ac2750. Consider uploading reports for the commit 3ac2750 to get more accurate results

@@            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     

Impacted Files	Coverage Δ
src/utils/chunkAssignment.ts	98.76% <99.09%> (-1.24%)	⬇️
src/Chunk.ts	99.65% <100.00%> (+<0.01%)	⬆️
src/Module.ts	100.00% <100.00%> (ø)
src/utils/options/normalizeOutputOptions.ts	100.00% <100.00%> (ø)

[rollup-bot]

This PR has been released as part of rollup@3.22.0-0. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.22.0-0 or npm install rollup@beta. It will likely become part of a regular release later.

[gajus]

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.

https://web-app-cba11ac8.contra.dev/itsjennylee

[gajus]

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

[lukastaegert]

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 minChunkSize, entries do not load "everything".
I am thinking of a follow-up feature, though, that would allow you to optimize for specific entry points so that it focuses on reducing the number of chunks just for those entry points, leaving any other entry points alone. As I understand SEO is the main concern, my understanding is that this might actually help.

[tmsns]

Concerning our issue, the beta release is indeed resolving it. We are able to remove the manualChunks function we created as a workaround for the issue. So for us, this PR is looking good!

Looking forward for the actual release! :-)

Thanks @lukastaegert!

[rollup-bot]

This PR has been released as part of rollup@3.22.0. You can test it via npm install rollup.