[v2.0.0] Avoid empty imports from side-effect-free chunks

[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 #3359
Resolves #3109
Resolves #3057

Description

Originally the goal with this PR was to just suppress empty imports from chunks if moduleSideEffects is false. However as it turned out, it was possible to extend this beyond the moduleSideEffects flag by using the existing tree-shaking mechanics to infer this flag for modules for which it was not explicitly set to false; moreover, Rollup will also generate fewer chunks in many situations.

To do this right, I implemented this on module level, not on chunk level. This means that after tree-shaking, each "empty" dependency (i.e. import where no bindings are actually imported) of a module is tested for side-effects and

• is removed if moduleSideEffects is false for this dependency
• is recursively replaced by its side-effectful dependencies if moduleSideEffects is true but no side-effects can be found inside the module
• is retained otherwise

This happens BEFORE chunks are generated, which means that after this PR, more efficient chunks will be generated! As an example, consider the following:

// input
// main1.js
import dep from './dep.js';
dep();

// main2.js
import dep from './dep.js';
console.log('main2');

// dep.js
export default function onlyUsedByOne() {
	console.log('Hello');
}

If you run this in the REPL, you will get three chunks with an empty import from the main2 chunk to the shared chunk–even though the shared chunk does not have side-effects: https://rollupjs.org/repl/?version=1.31.0&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMGRlcCUyMGZyb20lMjAnLiUyRmRlcC5qcyclM0IlNUNuZGVwKCklM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJtYWluMi5qcyUyMiUyQyUyMmNvZGUlMjIlM0ElMjJpbXBvcnQlMjBkZXAlMjBmcm9tJTIwJy4lMkZkZXAuanMnJTNCJTVDbmNvbnNvbGUubG9nKCdtYWluMicpJTNCJTIyJTJDJTIyaXNFbnRyeSUyMiUzQXRydWUlN0QlMkMlN0IlMjJuYW1lJTIyJTNBJTIyZGVwLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmV4cG9ydCUyMGRlZmF1bHQlMjBmdW5jdGlvbiUyMG9ubHlVc2VkQnlPbmUoKSUyMCU3QiU1Q24lNUN0Y29uc29sZS5sb2coJ0hlbGxvJyklM0IlNUNuJTdEJTIyJTJDJTIyaXNFbnRyeSUyMiUzQWZhbHNlJTdEJTVEJTJDJTIyb3B0aW9ucyUyMiUzQSU3QiUyMmZvcm1hdCUyMiUzQSUyMmVzbSUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE

With this PR, you will instead just get the following two chunks:

// output
// main1.js
function onlyUsedByOne() {
	console.log('Hello');
}
onlyUsedByOne();

// main2.js
console.log('main2');

You can check this out here: https://rollupjs.org/repl/?circleci=9330&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMGRlcCUyMGZyb20lMjAnLiUyRmRlcC5qcyclM0IlNUNuZGVwKCklM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJtYWluMi5qcyUyMiUyQyUyMmNvZGUlMjIlM0ElMjJpbXBvcnQlMjBkZXAlMjBmcm9tJTIwJy4lMkZkZXAuanMnJTNCJTVDbmNvbnNvbGUubG9nKCdtYWluMicpJTNCJTIyJTJDJTIyaXNFbnRyeSUyMiUzQXRydWUlN0QlMkMlN0IlMjJuYW1lJTIyJTNBJTIyZGVwLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmV4cG9ydCUyMGRlZmF1bHQlMjBmdW5jdGlvbiUyMG9ubHlVc2VkQnlPbmUoKSUyMCU3QiU1Q24lNUN0Y29uc29sZS5sb2coJ0hlbGxvJyklM0IlNUNuJTdEJTIyJTJDJTIyaXNFbnRyeSUyMiUzQWZhbHNlJTdEJTVEJTJDJTIyb3B0aW9ucyUyMiUzQSU3QiUyMmZvcm1hdCUyMiUzQSUyMmVzbSUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE

This means you will not only get fewer chunks but the second chunk will also avoid loading quite a bit of maybe-not-dead-but-definitely-not-needed code.

Now assume that dep.js itself has a side-effectful dependency, for instance an external import:

// input
// dep.js
import 'external-side-effect';
export default function onlyUsedByOne() {
	console.log('Hello');
}

Then you will STILL get two chunks, but the side-effect import will be hoisted to both chunks:

// output
// main1.js
import 'external-side-effect';
function onlyUsedByOne() {
	console.log('Hello');
}
onlyUsedByOne();

// main2.js
import 'external-side-effect';
console.log('main2');

I think this is a great improvement for everyone and the first step to actually fixing many ordering issues we can currently have with code-splitting without creating endless amounts of chunks as side-effect-free modules can be moved around rather safely (not done in this PR yet, though).

I have marked this as "BREAKING" because with this change, side-effect-free modules will no longer appear as part of any chunk in the generated bundle object. Previously, they would always appear to be assigned to some chunk. I am not sure if there are tools that analyze tree-shaking quality that depend on all modules being mentioned in the output.

[codecov]

Codecov Report

Merging #3369 into release-2.0.0 will increase coverage by 0.15%.
The diff coverage is 100%.

@@                Coverage Diff                @@
##           release-2.0.0    #3369      +/-   ##
=================================================
+ Coverage          94.07%   94.23%   +0.15%     
=================================================
  Files                174      173       -1     
  Lines               5977     5950      -27     
  Branches            1768     1757      -11     
=================================================
- Hits                5623     5607      -16     
+ Misses               191      190       -1     
+ Partials             163      153      -10

Impacted Files	Coverage Δ
src/utils/deconflictChunk.ts	100% <ø> (ø)	⬆️
src/Graph.ts	97.35% <100%> (-0.17%)	⬇️
src/utils/executionOrder.ts	100% <100%> (+2.17%)	⬆️
src/Module.ts	97.93% <100%> (+0.39%)	⬆️
src/utils/chunkAssignment.ts	100% <100%> (ø)
src/ast/nodes/Program.ts	100% <100%> (ø)	⬆️
src/Chunk.ts	95.07% <100%> (+1.09%)	⬆️
src/chunk-optimization.ts	57.35% <0%> (+1.47%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfdc86b...b335238. Read the comment docs.

[LarsDenBakker]

This is a huge deal for apps with a lot of dynamic imports. Would it be possible the provide this behind a flag in 1.x? Or is 2.x already close to shipping?

[tivac]

https://rollupjs.org/repl/?circleci=9330&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMiUyRiUyRiUyMG1haW4xLmpzJTVDbmltcG9ydCUyMGRlcCUyMGZyb20lMjAnLiUyRmRlcC5qcyclM0IlNUNuZGVwKCklM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJtYWluMi5qcyUyMiUyQyUyMmNvZGUlMjIlM0ElMjIlMkYlMkYlMjBtYWluMi5qcyU1Q25pbXBvcnQlMjBkZXAlMjBmcm9tJTIwJy4lMkZkZXAuanMnJTNCJTVDbmNvbnNvbGUubG9nKCdtYWluMicpJTNCJTVDbiUyMiUyQyUyMmlzRW50cnklMjIlM0F0cnVlJTdEJTJDJTdCJTIybmFtZSUyMiUzQSUyMmRlcC5qcyUyMiUyQyUyMmNvZGUlMjIlM0ElMjIlMkYlMkYlMjBkZXAuanMlNUNuZXhwb3J0JTIwZGVmYXVsdCUyMGZ1bmN0aW9uJTIwb25seVVzZWRCeU9uZSgpJTIwJTdCJTVDbiU1Q3Rjb25zb2xlLmxvZygnSGVsbG8nKSUzQiU1Q24lN0QlMjIlN0QlNUQlMkMlMjJvcHRpb25zJTIyJTNBJTdCJTIyZm9ybWF0JTIyJTNBJTIyY2pzJTIyJTJDJTIybmFtZSUyMiUzQSUyMm15QnVuZGxlJTIyJTJDJTIyYW1kJTIyJTNBJTdCJTIyaWQlMjIlM0ElMjIlMjIlN0QlMkMlMjJnbG9iYWxzJTIyJTNBJTdCJTdEJTdEJTJDJTIyZXhhbXBsZSUyMiUzQW51bGwlN0Q=

Well that's cool as hell. 👍🏻👍🏻👍🏻

[lukastaegert]

@LarsDenBakker

Or is 2.x already close to shipping?

Actually really close https://github.com/rollup/rollup/projects/2, the only remaining thing would be aligning on and implementing #3370 . And possibly making a call on #3343: experimentalOptimizeChunks is buggy and makes refactoring and future innovations hard, I think I really want to get rid of it.

Well that's cool as hell. 👍🏻👍🏻👍🏻

I spent some inspirational time today thinking that we finally should have a bot and be able to try out PRs in the REPL 😜

[rollup-bot]

Thank you for your contribution! ❤️

You can try out this pull request locally via

npm install rollup/rollup#gh-3359-chunk-module-side-effects

or load it into the REPL:
https://rollupjs.org/repl/?circleci=9337

[tivac]

I spent some inspirational time today thinking that we finally should have a bot and be able to try out PRs in the REPL 😜

Being able to easily try out big refactors by poking at them in the REPL or even installing them into existing projects is hugely helpful in terms of being able to provide real feedback. Thank you!

[lukeed]

Awesome work! 👏 It seems like this would also open up #2566 for a solution, right?

[lukastaegert]

There is not really a connection, but with this change, fixing #2566 would also benefit code-splitting.