tree shaking regression with angular 9

[saithis]

🐞 Bug report

Command (mark with an x)

• [ ] new
• [x] build
• [ ] serve
• [ ] test
• [ ] e2e
• [ ] generate
• [ ] add
• [ ] update
• [ ] lint
• [ ] xi18n
• [ ] run
• [ ] config
• [ ] help
• [ ] version
• [ ] doc

Is this a regression?

Yes, the previous version in which this bug was not present was: @angular/cli 8.3.25 with angular 8.2.14 ### Description

We have multiple components that are all imported via a single barrel file, but not all apps use all components. One of the components uses ng2-charts, but it is only used in one app.

With angular 8 the component and its dependencies (chart.js, etc.) get tree shaked away.

With angular 9 the component with all its dependencies is included in all apps.

🔬 Minimal Reproduction

checkout https://github.com/saithis/ng9-treeshaking-repo npm install npm run build npm run stats

if you want to compare to the angular 8 behaviour, checkout commit 5144d883a682aa3fea1776fd3d1f5aa4888a433f

🌍 Your Environment

Angular CLI: 9.0.1
Node: 13.8.0
OS: win32 x64

Angular: 9.0.0
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                            Version
------------------------------------------------------------
@angular-devkit/architect          0.900.1
@angular-devkit/build-angular      0.900.1
@angular-devkit/build-ng-packagr   0.900.1
@angular-devkit/build-optimizer    0.900.1
@angular-devkit/build-webpack      0.900.1
@angular-devkit/core               9.0.1
@angular-devkit/schematics         9.0.1
@angular/cli                       9.0.1
@ngtools/webpack                   9.0.1
@schematics/angular                9.0.1
@schematics/update                 0.900.1
ng-packagr                         9.0.0
rxjs                               6.5.4
typescript                         3.7.5
webpack                            4.41.2

Anything else relevant?

[saithis]

I just found out that creating a package.json beside the barrel file with "sideEffects": false fixes the issue. From reading the info in https://angular.io/guide/ivy-compatibility#payload-size-debugging I thought this would only work for real libraries that are compiled separatelly of the app. But it seems that this also works for any normal folder. Even if it is just a subfolder of the app with a barrel file.

But I also noticed that the app is broken if the package.json with "sideEffects": false is alongside the apps main.ts file. Then the generated main bundle is almost empty and the app doesn't work.

Why did the tree shaking work by default with ng8 in this case, but needs the sideEffects: false with ng9? Is this expected behaviour? If yes, then this issue can be closed.

[clydin]

main.ts has side effects (the angular bootstrap call) so marking that file as side effect free will break the application. However, it should typically be safe to put the side effects package.json in the src/app directory of an application (instead of src). An audit of the application code to ensure there are no unintended side effect calls would be recommended though. In general, it is recommended to write side effect free code whenever possible as it allows for more complete optimization of the resulting code.

This is indeed expected behavior in version 9. As to the difference, tree-shaking is actually behaving the same between 8 and 9. The difference is in the compilation. Previously, the AOT compiler would bypass the barrel files (or any intermediate file) and deep import the component/module/service. While this typically worked since most application code is side effect free (even though it wasn't marked as such), it was technically incorrect as there may well have been code in those intermediate files that needed to be executed.

[saithis]

Ok, thank you. Maybe it would be helpful to others to mention this in the angular upgrade guide or the breaking changes of angular 9. I was quite suprised to see a main bundle increase of 200 kb after the upgrade and it took me a few hours to find the answer.

[filipesilva]

https://github.com/angular/angular-cli/issues/16799#issuecomment-580912090 (the link in https://angular.io/guide/ivy-compatibility#payload-size-debugging) explains why this changed. It was a bug in the previous compiler that just so happened to also make smaller bundles in these cases. But it wasn't correct. If you had side effects they would be lost.

I think the debugging section is the right place in the docs for this type of information though. @StephenFluin do you think this should also be mentioned on the update guide?

[saithis]

Then maybe the ivy-compatibility page could be mentioned in the "After the Update" section of the upgrade guide. Something like "ivy is now the default, read the ivy-compatibility page for more info". Currently the only mention of ivy is, that entryComponents aren't neccessary anymore. Nothing about any problems that could arise because of ivy.

[saithis]

Maybe it helps to make to upgrade easier for others, so here is my upgrade proccess and why it took me so long to find the solution:

1. I read the breaking changes section of the angular changelog and the angular-cli changelog, the linked ivy blogpost and the Ivy Minor Changes google doc. I didn't really read the bugfixes and features, as there are so many and I didn't think I would need it for the upgrade.
2. I followed the angular upgrade guide (8 -> 9 advanced)
3. I saw the huge increase in main bundle filesize, but didn't suspect it to be an ivy change. Also I hadn't seen any mention of this sideEffects change and hadn't encountered the link to the ivy compatibility page.
4. I googled the issue and searched the issues tracker of angular and angular cli (somethow missed #16799 at first)
5. opened this issue
6. found #16799 after some more searching and found the fix

I think somewhere in step 1 or 2 I should have encountered this info, as it is rather important.

Edit: If you agree that this should be more prominent and point me in the right direction where to add it, then I can make a pull request. Otherwise this can be closed.

[filipesilva]

Then maybe the ivy-compatibility page could be mentioned in the "After the Update" section of the upgrade guide.

I think this is the right approach, yeah. Especially reading through your process. The information existed in https://angular.io/guide/ivy-compatibility, but it wasn't very clear you should have been on the lookout for these things.

Would be interested in adding an "After the update" step in https://github.com/StephenFluin/angular-update-guide/blob/master/src/app/recommendations.ts?

Looking over the update app I think the "After the update" messages for 9 are the ones with possibleIn: 900, necessaryAsOf: 1000,. Level refers to the app complexity. Not really sure what step is for.

I don't think this side effects thing is very relevant for basic apps, but it is relevant for medium complexity apps. So you'd add something like

{ possibleIn: 900, necessaryAsOf: 1000, level: 2, step: 'debug', action: 'MESSAGE GOES HERE' },

[saithis]

I'll try it :)

[saithis]

@filipesilva Thanks for the help :)

[filipesilva]

Nice, thanks for the pr!

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}