Skip to content
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

tree-shaking #6676

Closed
daniele-zurico opened this issue Jun 14, 2017 · 55 comments
Closed

tree-shaking #6676

daniele-zurico opened this issue Jun 14, 2017 · 55 comments
Assignees
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent severity3: broken

Comments

@daniele-zurico
Copy link

daniele-zurico commented Jun 14, 2017

Bug Report or Feature Request (mark with an x)

- [x ] bug report -> please search issues before submitting
- [ ] feature request

Versions.

@angular/cli: 1.1.1
node: 7.5.0
os: darwin x64

Hello all,
I'm struggling with tree shaking because it looks like that it doesn't work properly.
after running the follow command:
"bundle-report": "ng build --prod --target=production --environment=prod --aot --stats-json && webpack-bundle-analyzer dist/stats.json",

I realised that my vendor is: 2.4 MB and contain stuff that I don't use. RxJs for example take:
screen shot 2017-06-14 at 12 38 42

But in my code I use like that:
screen shot 2017-06-14 at 12 31 30

I'm doing something wrong or something wrong is in the tree shaking?

@Brocco Brocco added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent severity3: broken labels Jun 14, 2017
@bboehm86
Copy link

The very same goes for Angular Material. Once upon a time.. it worked.. I just got the Components that I imported but now it's the whole shabang lurking around the vendor.ts.. sadly I do not rly know when it stoped working.
(If I remember correctly it was back then when we still got the ngfactorys and styles of the Components in our chunk files)

Demo App to try out: https://github.com/bboehm86/mat-chunk-test

material_bundle

@dojchek
Copy link

dojchek commented Jun 17, 2017

Can confirm the same for Lodash. Can't tell if it worked on earlier versions, since I used to use full import on lodash.
With optimized imports:

import { first } from 'lodash';
import { mapValues } from 'lodash';
// ...

I still get the whole lodash lib loaded in..

screenshot_5

@angular/cli: 1.1.0
node: 7.4.0
os: win32 x64
@angular/*: 4.1.3

@galuszkak
Copy link

@daniele-zurico @dojchek @bboehm86 I think your issue is duplicate of this:
#2901

The Angular CLI team said it's webpack issue therefore it should be reported there and track there. (webpack/webpack#2899).

The sad thing is that issue in webpack is open for 10 months now and doesn't seem to go anywhere.

These led me to believe that maybe CLI should be discourage for use in production, as it clearly doesn't do it's job and making really big bundles. I'm also struggling because of that issue as it's impacting performance a lot. If you are using Angular lazy loading ( loadChildren in router) it's even worse because chunks are really big (1 mb each piece of really small modules), that you get 9 Mb bundle of 5 mb vendor and 4 mb of modules (so sum is 9). Without lazy loading it's little better because you have 5-6mb depending on code base, but still quite not what I expect as I'm using really only small part of RxJS and angular material.

@Delagen
Copy link

Delagen commented Jul 4, 2017

I think it's problem with webpack config.
I use own process as ngc=>webpack so result is normal
image
image

@galuszkak
Copy link

@Delagen as far I remember problem was with UglifyJS. Is that included in your webpack config?

See relevant threads:
webpack/webpack#2867 webpack
mishoo/UglifyJS#1261 UglifyJS2

@Delagen
Copy link

Delagen commented Jul 5, 2017

@galuszkak This diagrams for minified code with uglify, tried to use Closure, but no success. Uglify works

@Delagen
Copy link

Delagen commented Jul 5, 2017

I think in webpack config does not added 'module' field to resolve section, this is why modules resolved via main field and es6 modules does not used

@Delagen
Copy link

Delagen commented Jul 6, 2017

Ps may be it left for performance? With real tree shaking my project build up to 20!!! minutes with webpack. May be with rollup will be faster

@filipesilva
Copy link
Contributor

Webpack defaults to resolving module first and only main after that, so we don't specify it ourselves.

So I'm trying to test this, and to do so I did npm i source-map-explorer and added this npm script:

"sme": "ng build --prod --sm --output-hashing=none && source-map-explorer ./dist/vendor.bundle.js ./dist/vendor.bundle.js.map"

I use source-map-explorer because webpack-bundle-analyzer seems to not be working with the latest webpack. Vendor chunking is disabled so everything shows in the same bundle for analysis.

Running this script on a brand new project, and zooming in to rxjs and then operator I see four: mergeAll, merge, multicast, share. Total size 2.28kb.

image

Then on main.ts I add import 'rxjs/add/operator/switchMap';, run the script again.

Now there are five operators, the same as before plus switchMap, total size 3.75kb.

image

This to me seems like the tree shaking available in webpack is working correctly - only the operator I asked for was added.

I cannot speak for Material, but they seem to have an issue opened for how their packaging format is amenable to tree shaking: angular/components#4137.

For the other example provided here (rxjs, lodash), I'd say that your app might not be using the other operators but a third party library is, and so they get included.

@Delagen
Copy link

Delagen commented Jul 17, 2017

@filipesilva RXJS fine
image
but @angular/* not
image

@filipesilva
Copy link
Contributor

@Delagen those are the correct files for @angular/*. If you open their package.json you will see "module": "./@angular/core.es5.js",.

@Delagen
Copy link

Delagen commented Jul 17, 2017

@filipesilva I know
But for my pipeline source-map show like this
image

@filipesilva
Copy link
Contributor

What is your pipeline?

@Delagen
Copy link

Delagen commented Jul 17, 2017

@filipesilva I manually build my application using ngc compiler, after it normal webpack build with uglify

@daniele-zurico
Copy link
Author

@filipesilva what you mean with that: For the other example provided here (rxjs, lodash), I'd say that your app might not be using the other operators but a third party library
The rxjs is the default one as you can see from my screenshot.

@galuszkak
Copy link

I will still argue that this issue is dupe of: #2901

and @filipesilva said it's related to and it's watching it: webpack/webpack#2899

But it's about year and that issue is still there. Webpack people are saying that this is mostly issue with UglifyJS, so I'm quite confused because I'm experiencing this issue also for some time.

@filipesilva
Copy link
Contributor

@Delagen then that massively depends on your webpack configuration. Internally the CLI uses ngc as well. I can see that you're tracing sourcemaps for Angular libs, whereas the CLI is not, but other than that I have no idea what you are doing in your config. Are you getting smaller builds using your pipeline?

@daniele-zurico I mean that, if your third party libraries use RxJs or Lodash, it will of course be included in the final build even if your app does not use it directly.

@daniele-zurico
Copy link
Author

@filipesilva also if lodash is not used at all in my code?
To be more specific the third party library has a module that use lodash but that module is not imported in my code. Lodash will end up in my build?
If it's I don't understand what mean tree shacking

@trotyl
Copy link
Contributor

trotyl commented Jul 17, 2017

@daniele-zurico lodash is using CommonJS rather than ES Module, it's not shakable theoretically. You may be talking about some other kind of DCE instead of tree-shaking.

@Delagen
Copy link

Delagen commented Jul 17, 2017

@filipesilva I use generic config with uglify. It seems some plugin disable tree shaking at all.
My config I think allow it, and build time increase up to 20 min, but size is smaller. Looks at angular package sizes in my screenshots.

@filipesilva
Copy link
Contributor

@Delagen it really depends on a lot of things. If you could provide me with your config I can run some benchmarks and see if something could be used in the in CLI that we aren't using already.

@bossysmile11
Copy link

bossysmile11 commented Jul 17, 2017 via email

@filipesilva
Copy link
Contributor

@bossysmile11 I cannot unsubscribe you myself, but you can click on this button on the side of the issue in github:
image

@Delagen
Copy link

Delagen commented Jul 17, 2017

@filipesilva I cannot provide config, cause it generated with private library at runtime. Simply used loaderoptions plugin with minimize and disable debug options, uglifyjs. Used awesome-typescript-loader as typescript with module Es2015 setting, with ts 2.4.1 ESNext. I try to make demo repo for this.

@filipesilva
Copy link
Contributor

Thank you for taking the time to do it @Delagen 👍

@Delagen
Copy link

Delagen commented Jul 17, 2017

@filipesilva Yes. It's causes that you ignored source-maps in node_modules, so users saw as error.

{ enforce: 'pre', test: /\.js$/, loader: 'source-map-loader', exclude: [nodeModules] },

Created sample project. Difference is about 1K

@daniele-zurico
Copy link
Author

Sorry guys but at that point I got lost....starting from my original issue...do you think that everything is working as expected? I need to do something? or is not doing tree shacking properly?

@filipesilva
Copy link
Contributor

Tree shaking, as far as webpack is capable of doing, is working correctly from what I can tell.

#6365 implements scope hoisting, which should help as well.

#6520 adds a couple of opt-in experimental optimizations.

@daniele-zurico
Copy link
Author

daniele-zurico commented Jul 18, 2017

is that available in 1.2.1? or we need to wait the next release?
and last question is that command correct and optimised for build production:
ng build --target=production --environment=prod --aot

@filipesilva
Copy link
Contributor

filipesilva commented Jul 18, 2017

Scope hoisting is available in 1.3.0-beta.1. Both should be available in 1.3.0 final.

@daniele-zurico
Copy link
Author

and what about my actual command to build in production mode?

@filipesilva
Copy link
Contributor

I'm not sure what you mean, can you rephrase that?

@daniele-zurico
Copy link
Author

To build in production mode i'm using this command: ng build --target=production --environment=prod --aot
It's the best way or I'm doing something wrong or can I add same extra parameter?

@filipesilva
Copy link
Contributor

Just doing ng build --prod does all those things, and a couple more (https://github.com/angular/angular-cli/wiki/build#bundling--tree-shaking). It is the best way imho.

@daniele-zurico
Copy link
Author

daniele-zurico commented Jul 18, 2017

This is what I don't understand (look on the vendor):
ng build --prod
screen shot 2017-07-18 at 15 59 23

ng serve --prod
screen shot 2017-07-18 at 16 01 27

@filipesilva
Copy link
Contributor

ng serve --prod also includes live reload functionality from webpack-dev-server, which increases the size of the vendor bundle. It also has gzip enabled while serving.

@daniele-zurico
Copy link
Author

but is the opposite ... I was expecting that but:
ng serve (vendor) --> 239Kb
ng build (vendor) -->790Kb

@filipesilva
Copy link
Contributor

The greyed out (bigger) number is the content size (after being unzipped), while the black number is the size of the response (what was transferred) itself. Due to gzip, the response is smaller than the content.

In ng build the vendor has a content size of 789KB, while on ng serve the content size is 911KB. But since ng serve uses gzip, it was delivered compressed at 239KB.

You used some server to server the result of ng build, and that server doesn't seem to have gzip enabled. That is why most requests don't have a smaller response size.

@ianchi
Copy link

ianchi commented Jul 18, 2017

@filipesilva the optimizer you referenced seems a great feature.
Right now I'm not sure I understand the status of tree-shaking in Angular.
I'm using Angular Material, and it brings in a lot of components even if a don't use them anywhere.
So I don't understand your comment:

Tree shaking, as far as webpack is capable of doing, is working correctly from what I can tell.

Is this then not a problem of webpack but of the angular compiler, that leaves function calls not marked as pure when creating the factories?
For instance, this ends up in my bundled code (comming from index.ngfactory.js):

var MdNativeDateModuleNgFactory = createNgModuleFactory(MdNativeDateModule, (([])), function (_l) {
    return moduleDef([moduleProvideDef(512, ComponentFactoryResolver, CodegenComponentFactoryResolver, [[8, (([]))], [3, ComponentFactoryResolver], NgModuleRef]), moduleProvideDef(4608, DateAdapter, NativeDateAdapter, (([]))), moduleProvideDef(512, NativeDateModule, NativeDateModule, (([]))), moduleProvideDef(512, MdNativeDateModule, MdNativeDateModule, (([]))), moduleProvideDef(256, MD_DATE_FORMATS, MD_NATIVE_DATE_FORMATS, (([])))]);
});

I never used MdNativeDateModule, but how could Webpack/Uglify possibly know if createNgModuleFactory call has side effects or not, so to be able to remove it? If it is not removed, then MdNativeDateModule is kept, which in turns brings in a lot more unused stuff.

Will this be addressed with the optimizer you mentioned?

@filipesilva
Copy link
Contributor

@ianchi for information on Angular Material tree shaking, check out angular/components#4137.

@ianchi
Copy link

ianchi commented Jul 24, 2017

@filipesilva I've just tried the new RC of the cli, which includes the optimizer.
My gziped app reduced from around 250kb to 185kb, that's about 25% reduction!
Great work!!

@filipesilva
Copy link
Contributor

@ianchi that's great news! Let me know if you experience any problems with it.

@ianchi
Copy link

ianchi commented Jul 24, 2017

I need to do more testing, but so far, it's not working if you eject the project:
Webpack throws the following error:

    new PurifyPlugin(),
        ^

ReferenceError: PurifyPlugin is not defined
    at Object.<anonymous> (webpack.config.js:453:9)

@filipesilva
Copy link
Contributor

Yeah that seems to be missing. You can get it from import { PurifyPlugin } from '@angular-devkit/build-optimizer';.

@ianchi
Copy link

ianchi commented Jul 25, 2017

Tree shaking is working much better with this plugin.
Most of the unnecessary ngFactories are gone. So far I only spotted some suspicious code with styles imports that could have been removed, but perhaps it's a Material issue.

var 
styles_MdCalendar = [ ".mat-calendar{display:block}.mat-calendar-header{padding:8px 8px 0 8px}.mat-calendar-content{padding:0 8px 8px 8px;outline:0}.mat-calendar-controls{display:flex;padding:5% calc(100% / 14 - 22px) 5% calc(100% / 14 - 22px)}.mat-calendar-spacer{flex:1 1 auto}.mat-calendar-period-button{min-width:0}.mat-calendar-arrow{display:inline-block;width:0;height:0;border-left:5px solid transparent;border-right:5px solid transparent;border-top-width:5px;border-top-style:solid;margin:0 0 0 5px;vertical-align:middle}.mat-calendar-arrow.mat-calendar-invert{transform:rotate(180deg)}[dir=rtl] .mat-calendar-arrow{margin:0 5px 0 0}.mat-calendar-next-button,.mat-calendar-previous-button{position:relative}.mat-calendar-next-button::after,.mat-calendar-previous-button::after{content:'';position:absolute;top:0;left:0;bottom:0;right:0;margin:15.5px;border:0 solid currentColor;border-top-width:2px}[dir=rtl] .mat-calendar-next-button,[dir=rtl] .mat-calendar-previous-button{transform:rotate(180deg)}.mat-calendar-previous-button::after{border-left-width:2px;transform:translateX(2px) rotate(-45deg)}.mat-calendar-next-button::after{border-right-width:2px;transform:translateX(-2px) rotate(45deg)}.mat-calendar-table{border-spacing:0;border-collapse:collapse;width:100%}.mat-calendar-table-header th{text-align:center;padding:0 0 8px 0}.mat-calendar-table-header-divider{position:relative;height:1px}.mat-calendar-table-header-divider::after{content:'';position:absolute;top:0;left:-8px;right:-8px;height:1px}" ], 
styles_MdCalendarBody = (index_ngfactory___WEBPACK_IMPORTED_MODULE_0__angular_core__._2({
            encapsulation: 2,
            styles: styles_MdCalendar,
            data: {}
        }), [ ".mat-calendar-body{min-width:224px}.mat-calendar-body-label{padding:7.14286% 0 7.14286% 7.14286%;height:0;line-height:0;transform:translateX(-6px);text-align:left}.mat-calendar-body-cell{position:relative;width:14.28571%;height:0;line-height:0;padding:7.14286% 0;text-align:center;outline:0;cursor:pointer}.mat-calendar-body-disabled{cursor:default}.mat-calendar-body-cell-content{position:absolute;top:5%;left:5%;display:flex;align-items:center;justify-content:center;box-sizing:border-box;width:90%;height:90%;border-width:1px;border-style:solid;border-radius:50%}[dir=rtl] .mat-calendar-body-label{padding:0 7.14286% 0 0;transform:translateX(6px);text-align:right}" ])

I'm not sure if I get it quite right, but it seems that the definition of styles_MdCalendar is finished inside the definition of the next style styles_MdCalendarBody as a kind of side effect with the comma operator (invoking the createRendererType2 function).
This goes on with different styles, forming a chain where each definition references the previous component, and so cannot be eliminated. I only use one of the last components of the chain, but this seems to pull all the chain to the top, with many unnecessary definitions (which contains very long strings).

I don't know where do Decorators get transformed to this chained construction. I guess the Angular Compiler?

Another question. Is there any option in the CLI to add html minification of the component's templates? Perhaps in the ejected project I can add the loader to the html chain with raw-loader or is it handled internally in the ngtools/webpack-loader?. It would be nice to have it directly in the CLI.
This can also shrink the bundle a bit, depending on how 'unoptimized' where the templates in the first place.

Finally, I wanted to also check using source map explorer, but the generated source map (with -sm option) only shows "*" for all the vendor sources. Any hint to make it work, so it also follows the source files in node_modules?
I'm using this command:

ng build --prod --vendor-chunk=false --build-optimizer=true -sm

stats.json also seems to be somehow broken, as I cannot explore it with any tool.

@filipesilva
Copy link
Contributor

@ianchi there is no support for html minification in the CLI no. index.html has it, but it's the only one.

There's a bug with sourcemaps with --bo right now, being tracked in #7093.

Unsure what's wrong with stats.json, but it seems related to scope hoisting as well.

As for the Material styles you related, I cannot answer but will ping the appropriate people (@IgorMinar @kara).

@filipesilva
Copy link
Contributor

Also, thanks for looking at it so attentively, this is all great feedback!

@ianchi
Copy link

ianchi commented Jul 25, 2017

Perhaps the problem with styles is that the call to createRendererType2 is not annotated as PURE.
This is from the same app, but just disabling the Uglify plugin:

var styles_MdCalendar = ['.mat-calendar{display:block}.mat-calendar-header{padding:8px 8px 0 8px}.mat-calendar-content{padding:0 8px 8px 8px;outline:0}.mat-calendar-controls{display:flex;padding:5% calc(100% / 14 - 22px) 5% calc(100% / 14 - 22px)}.mat-calendar-spacer{flex:1 1 auto}.mat-calendar-period-button{min-width:0}.mat-calendar-arrow{display:inline-block;width:0;height:0;border-left:5px solid transparent;border-right:5px solid transparent;border-top-width:5px;border-top-style:solid;margin:0 0 0 5px;vertical-align:middle}.mat-calendar-arrow.mat-calendar-invert{transform:rotate(180deg)}[dir=rtl] .mat-calendar-arrow{margin:0 5px 0 0}.mat-calendar-next-button,.mat-calendar-previous-button{position:relative}.mat-calendar-next-button::after,.mat-calendar-previous-button::after{content:\'\';position:absolute;top:0;left:0;bottom:0;right:0;margin:15.5px;border:0 solid currentColor;border-top-width:2px}[dir=rtl] .mat-calendar-next-button,[dir=rtl] .mat-calendar-previous-button{transform:rotate(180deg)}.mat-calendar-previous-button::after{border-left-width:2px;transform:translateX(2px) rotate(-45deg)}.mat-calendar-next-button::after{border-right-width:2px;transform:translateX(-2px) rotate(45deg)}.mat-calendar-table{border-spacing:0;border-collapse:collapse;width:100%}.mat-calendar-table-header th{text-align:center;padding:0 0 8px 0}.mat-calendar-table-header-divider{position:relative;height:1px}.mat-calendar-table-header-divider::after{content:\'\';position:absolute;top:0;left:-8px;right:-8px;height:1px}'];
var RenderType_MdCalendar = index_ngfactory___WEBPACK_IMPORTED_MODULE_0__angular_core__["_2" /* ɵcrt */]({ encapsulation: 2, styles: styles_MdCalendar,
    data: {} });

In constrast, ngFactories are annotated and thus unused ones get removed:

var MdCalendarNgFactory = /*@__PURE__*/index_ngfactory___WEBPACK_IMPORTED_MODULE_0__angular_core__["_0" /* ɵccf */]('md-calendar', material_es5_MdCalendar, View_MdCalendar_Host_0, { startAt: 'startAt', startView: 'startView',
    selected: 'selected', minDate: 'minDate', maxDate: 'maxDate', dateFilter: 'dateFilter' }, { selectedChange: 'selectedChange' }, []);

The comma operator construct, must be some kind of Uglify optimization.

Going back to HTML minification, are there plans to add it to components' templates? Is there a way I can add it in the ejected webpack? I suspect this must be done inside the ngtools/webpack loader, in between the replacement to require the html template file and the compiling of the string.

@filipesilva
Copy link
Contributor

We fiddled with html processing for images a while ago, and couldn't reach a secure approach so left them as is.

On an ejected config you can probably edit the loader for html (currently raw-loader https://github.com/angular/angular-cli/blob/master/packages/@angular/cli/models/webpack-configs/common.ts#L108).

Thinking about it a bit more, I'm not 100% sure that will work with --aot... but then again --aot shouldn't care, since it will make the factories anyway? @hansl would know.

@daniele-zurico
Copy link
Author

daniele-zurico commented Sep 11, 2017

is possible to eject as well with --bo command? v1.4.0
The option '--bo' is not registered with the eject command. Run ng eject --help for a list of supported options.

@trotyl
Copy link
Contributor

trotyl commented Sep 11, 2017

@daniele-zurico There are no --bo command at all, only --build-optimizer or --buildOptimizer, see #7231 for more information.

@raysuelzer
Copy link

Why was this closed? All of the above issues still occur.

@Axure
Copy link

Axure commented Apr 15, 2018

@raysuelzer probably it means "WON'T FIX".

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent severity3: broken
Projects
None yet
Development

No branches or pull requests