optimize-angular-app icon indicating copy to clipboard operation
optimize-angular-app copied to clipboard

Add closure-compiler

Open alexeagle opened this issue 7 years ago • 6 comments

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.

alexeagle avatar Feb 02 '17 18:02 alexeagle

Very interesting, I'll give it a try

marclaval avatar Feb 03 '17 14:02 marclaval

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.

alexeagle avatar Feb 03 '17 15:02 alexeagle

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 avatar Feb 08 '17 16:02 marclaval

Here is what I did:

  • upgrade the project to Angular 4.0.0-beta.6
  • dependencies:
    • @angular/*: took the builds from github, e.g. https://github.com/angular/common-builds/tree/4.0.0-beta.6-es2015
    • rxjs: build taken from https://github.com/alexeagle/closure-compiler-angular-bundling
    • zone.js: externs file taken from https://github.com/alexeagle/closure-compiler-angular-bundling
    • @ng-bootstrap/ng-bootstrap: created a special build at https://github.com/mlaval/ng-bootstrap-tsickle-build , see the details there
  • application:
    • in app/search/search.ts: changed the way the map operator from rxjs was imported and used
    • in app/search/search.ts: had to use string to access properties of a JSON object, to avoid renaming by the compiler
    • in app/welcome/welcome.ts: having 100 components on one line was generating an error at runtime. Tried different setup, got different errors. Ended up with one component per line...
  • finally, adapted the build_closure.sh script from https://github.com/alexeagle/closure-compiler-angular-bundling

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?

marclaval avatar Feb 08 '17 16:02 marclaval

Thank you for integrating this and gathering the results!

Build script: https://github.com/alexeagle/closure-compiler-angular-bundling/issues/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 https://github.com/google/closure-compiler/issues/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.

alexeagle avatar Feb 08 '17 17:02 alexeagle

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.

marclaval avatar Mar 06 '17 15:03 marclaval