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

Add closure-compiler #2

Open
alexeagle opened this issue Feb 2, 2017 · 6 comments
Open

Add closure-compiler #2

alexeagle opened this issue Feb 2, 2017 · 6 comments

Comments

@alexeagle
Copy link

https://github.com/alexeagle/closure-compiler-angular-bundling is now pretty mature. One of us should add such an app to your benchmark so we can compare.

@marclaval
Copy link
Owner

Very interesting, I'll give it a try

@alexeagle
Copy link
Author

alexeagle commented Feb 3, 2017

Note also, we have a nearly identical setup in an Angular integration test: https://github.com/angular/angular/tree/master/integration/hello_world__closure

The difference is the alexeagle/ one has zone.js as a separate <script> while the angular/ one bundles it with the app. Not a big size difference.

@marclaval
Copy link
Owner

Done, and the results are quite impressive!
The application is much smaller and faster this way, compared to Rollup or Webpack:

  • the bundle generated is more than 40% smaller (I've kept zone.js and core-js in a separate file)
  • the initial display time is reduced by 40% too

I'll keep the issue opened to share more details about what I did and get some feedback about it.

@marclaval
Copy link
Owner

Here is what I did:

Known issues:

  • The script doesn't work on Windows because there are too many arguments for the CLI. In theory, it could be addressed by using patterns.

Questions:

  • Are the changes made in the application bugs or features?
  • Is the special build of @ng-bootstrap/ng-bootstrap the right to do it? Can it be made easier?
  • Can all this be further improved?
  • Is it possible to do lazy loading?

@alexeagle
Copy link
Author

Thank you for integrating this and gathering the results!

Build script: alexeagle/closure-compiler-angular-bundling#11

ng-bootstrap build: tsickle should have done the @angular/core -> @angular/core/index rewrites for you. See https://github.com/angular/angular/blob/master/tools/%40angular/tsc-wrapped/src/main.ts#L85 - maybe you have an older version of tsc-wrapped that doesn't have that change?

import 'rxjs/add/operator/map'; bug is google/closure-compiler#2247 another workaround is to add a trivial export const aoeuaoeu=1 to the map.js file

the 100-component-line issue is really interesting, any idea which tool is doing the wrong thing? we should file a bug for it.

rxjs dep: I'm attending their team meeting on Monday to figure out how they can publish the distro we need. In the meantime, a nicer way to do this could be: add rxjs as a git submodule and build it in an npm postinstall

size improvements: Misko is looking at how to trim @angular/core more (the decorator reflection code is still included, for instance, even though that was all code-generated and no longer needed at runtime)

ergonomic improvements: there should be better clues to help avoid things like renaming bugs. Can we discuss more what things would be the most useful?

lazy loading: absolutely, Splittable does this with closure compiler, I've started to look at how to wire these together.

@marclaval
Copy link
Owner

I've managed to add the lazy loading with closure compiler case through #4
The demo is available here: https://mlaval.github.io/optimize-angular-app/lazyloading-closure/#/

For documentation purpose, here the steps involved in the build process:

  1. Use ngc to compile all the sources and generate all the factories
  2. Use ngc again to transpile all the TS code to ES6 with annotations for closure compiler
  3. For each of the 4 entry points of the app, run closure compiler to generate a bundle, using all sources (app, angular, rxjs, ng-bootstrap) and the entry point
  4. Use the 4 manifest files generated to find out which files need to go in each bundle
  5. Run closure compiler with the right module definition to generate the 4 final bundles

On top of that, I've reused the System.import polyfill from https://github.com/cramforce/splittable
It means that a bit of code has to be added to each lazy loaded bundle.

I'm not sure if it is the best that can be done, but it works and the result is coherent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants