Use Closure Compiler with offline template compiler

alexeagle commented 8 years ago

Angular's new offline compiler was demo'ed at ng-conf day 2 keynote by @robwormald. It generates tree-shakable ES6 code. Four steps are then required:

run a tree-shaker to remove ES6 modules not reachable from the entry point, in our demo we showed rollup
run a bundler to reduce down the module imports into a declarations in a single file; we showed system.js
down-level to ES5 - we showed this with TypeScript
minify to shorten symbol names, we used uglify.

This produced a 49k JS file, but requires a lot of configuration.

Google's Closure Compiler (https://developers.google.com/closure/compiler/) produces very small JS. It does all four steps required above, so the configuration should be a lot simpler. We also suspect we can get a smaller binary size for ng2-hello-world, around 36k.

Wiring this up requires:

[x] add tsickle's closure annotation helper to `ngc
[x] modify Angular's ES6 distribution to be closure-compiler compatible
[x] ?? modify ES6 distro of our dependencies (rxjs, zone.js) to be closure-compiler compatible
[x] figure out a minimal build (maybe a shell script) that successfully compiles the application together with the framework and its dependencies
[x] document how we did it so others can repro
[x] bundle externs with Angular to allow Protractor testing (the BrowserNodeGlobal type)

evmar commented 8 years ago

The part I think I most need your guidance on is where/how this fits into the larger angular build process. Do we want to (1) build angular itself as a minified external bundle? or (2) compile angular+user code together into a single bundle? In the latter case we'll need to integrate into gulp or whatever.

alexeagle commented 8 years ago

We do distribute Angular bundles, as ES5 with UMD module loader built-in. However, it would be impossible to pick this back apart again to tree-shake it, so our plan has been #2. I'm not sure we should commit to building plugins for the several popular userland build tools; others in the community will pick that up. I think a simpler "npm run build && npm run closure-compiler" with shell scripts is sufficient at this point.

On Mon, May 9, 2016 at 11:26 AM Evan Martin notifications@github.com wrote:

The part I think I most need your guidance on is where/how this fits into the larger angular build process. Do we want to (1) build angular itself as a minified external bundle? or (2) compile angular+user code together into a single bundle? In the latter case we'll need to integrate into gulp or whatever.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/angular/angular/issues/8550#issuecomment-217947352

alexeagle commented 8 years ago

FYI @jeffbcross maybe this can make your PWA demo for I/O

jeffbcross commented 8 years ago

Yeah that would be awesome if we could have this working by the end of this week.

alexeagle commented 8 years ago

Some notes from the hacks I've had to make so far, in the hopes that others can reproduce:

Closure Compiler doesn't handle all ES6 module syntax, and discourages using it. So our strategy so far has been to use goog.module syntax. That's not a --module option to tsc, so we use tsickle to re-write it. That means angular and its dependencies have to be emitted as "Closure ES6".

Need to build closure compiler from HEAD, see https://github.com/google/closure-compiler/blob/master/README.md#building-it-yourself

To build angular into closure ES6, I have local edits in the ngc tool to run tsickle's closure annotation and convertCommonJsToGoogModule. See https://github.com/alexeagle/angular/tree/closure_hack2 Build angular the usual way with ./build.sh and then make the packages installable with for pkg in $(find dist/packages-dist -name package.json); do sed -i .bak 's/\$\$ANGULAR_VERSION\$\$/2.0.0-rc.2-snap/g' $pkg; done

To build rxjs into closure ES6, I have local edits in https://github.com/alexeagle/RxJS/tree/closure - then run npm run build_es6 to get the right files in dist/es6. Then npm run generate_packages to copy in a package.json.

We need a modification to tsickle to workaround https://github.com/ReactiveX/rxjs/issues/1703 - see https://github.com/angular/tsickle/tree/closure

Now I make an example app. Snapshot at https://github.com/alexeagle/closure-compiler-angular-bundling In the vendor directory I've already installed the locally built closure compiler.jar, angular2 packages, rxjs, and tsickle, with something like npm install ../angular/dist/packages-dist/{common,compiler_cli,compiler,core,platform-browser,platform-server} npm install ../rxjs/dist/es6

So you can just npm install and npm run build to reproduce the closure bundle :)

dpsthree commented 8 years ago

So I would need to have a Java JRE installed to bundle a hello world Angular2 app?

alexeagle commented 8 years ago

Closure Compiler is just one option for the tree-shake/bundle/Es6-to-5/minify pipeline. If you choose the closure compiler option, you take a JRE dependency for now. But there is also a JS version in the works: https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/gwt/client/GwtRunner.java

On Fri, May 13, 2016 at 9:44 AM dpsthree notifications@github.com wrote:

So I would need to have a Java JRE installed to bundle a hello world Angular2 app?

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/angular/angular/issues/8550#issuecomment-219096815

alexeagle commented 8 years ago

I wrote some notes on the hacks required to make it work. https://docs.google.com/document/d/17m1GwzXraKgbCkCmH1JnY9IZzPy4cqlpCFVhvlZnOys/edit

jjudd commented 7 years ago

We reproduced @alexeagle's work and with a more up to date version of Angular. The entire build process is automatic, so you can follow everything from source to end result. You can see it in action at https://github.com/lucidsoftware/closure-typescript-example. Instructions for running the example can be found in the README for the example repo. There is a Docker container for the project, so it should be possible for pretty much anyone to try it out.

The above example utilizes Angular 2, clutz, and tsickle. Closure JS is used in Angular 2 TS, which is compiled to Closure compatible JS.

The changes we made to the dependencies to get this to work are largely the same as Alex's. There are a few changes we added and a few changes that are no longer necessary. Regardless, the basic idea is the same. We use a modified ngc or tsickle on Angular 2 and its dependencies to produce Closure compatible versions of those dependencies.

Closure Compiler

Building from head is no longer necessary. Just use one of the versions from June or July.

Angular

https://github.com/lucidsoftware/angular/tree/closure-bundle

The bulk of the work in the Angular 2 source is modifying ngc to produce Closure compilable js by adding a few tsickle steps to tsc-wrapped. There are a few hoops that we jump through in order to use the modified source as input to the TypeScript compiler in one of the tsickle passes. Otherwise, this is pretty straight forward.

We also change certain Angular modules to be compiled to Closure compatible JS by putting an angularCompilerOptions block in the tsconfig.json like so

"angularCompilerOptions": {
  "googleClosureOutput": true
}

Then when you run build.sh those modules have the output you want.

A test utillity file that uses selenium-webdriver is modified because selenium-webdriver is missing and we didn't want to make it closure compatible.

RxJS

https://github.com/lucidsoftware/rxjs/tree/closure-hack

The build process of RxJS is modified to build with the Makefile from our project. About half the build process is in JS and the other half in Make. It's pretty janky.

The only other changes besides the build target are a few small things to make RxJS compile with TypeScript and Closure.

symbol-observable (RxJS dependency)

https://github.com/lucidsoftware/symbol-observable/tree/closure

RxJS now depends on a some code that used to be part of the RxJS source and is now its own module. We modify that module (which is JS) to be compatible with the Closure compiler. There are only two files, so I did this manually.

tsickle

https://github.com/lucidsoftware/tsickle/tree/ignore-type-comments

We modify Tsickle in two ways:

Add a --ignoreTypesInComments option. This prevents tsickle from throwing an error when it encounters jsdoc in comments. This is needed to compile RxJS which has a bunch of jsdoc comments.
Modify the pathToModuleName for the CLI to be the same as the pathToModuleName we use in ngc. That way modules are named the same when run through tsickle or ngc. This should be committed upstream because the current pathToModuleName sometimes produces invalid goog.module names.

JamesHenry commented 7 years ago

run a tree-shaker to remove ES6 modules not reachable from the entry point, in our demo we showed rollup

run a bundler to reduce down the module imports into a declarations in a single file; we showed system.js

down-level to ES5 - we showed this with TypeScript

minify to shorten symbol names, we used uglify.

Whilst it perhaps cannot match the final output size of the Closure Compiler, webpack 2 (using typescript loader and standard optimize plugins) gets you all of the these steps.

I would argue that the benefit of devs already using (dare I say requiring) a tool like webpack in their stack, and therefore not having to add more (hacked) dependencies, is more important than shaving off extra bytes in the final payload.

Would love to hear your thoughts!

alexeagle commented 7 years ago

@jjudd thank you so much for posting your repo and these notes, this is really helpful.

Do you have a minified app size you can share? Perhaps using Brotli compression so we have an apples-to-apples comparison with the hello world closure-ng2 example (which was 26.2k)

@JamesHenry we agree that webpack 2 is the way forward for most developers. Closure compiler is an expert tool and I doubt we can wrap it up in a package that 'just works' for developers who aren't already familiar with debugging the obscure ways its ADVANCED_OPTIMIZATIONS can bust your app.

jjudd commented 7 years ago

@alexeagle We're currently working on getting this to work with ADVANCED_OPTIMIZATIONS. When we get that working I'll go ahead and post some benchmarks.

evmar commented 7 years ago

Converted the tsickle issue 1 above into a bug report there.

robwormald commented 7 years ago

closure compiler is now available on npm in pure javascript : https://www.npmjs.com/package/google-closure-compiler-js

jjudd commented 7 years ago

Wanted to provide an update:

We have the example repo and the beta Lucidchart editor working with advanced optimizations. We ran a short test using our version with simple optimizations, and our version using advanced optimizations is going out today or tomorrow.

The version of Angular used in the example repo has been upgraded to RC5 (HEAD as of Tuesday Aug 16).

The example repo and its accompanying Docker container have been updated in case anyone wants to try it out. Here's a link to the example repo https://github.com/lucidsoftware/closure-typescript-example

Bundle sizes

The final bundle size for the example project using advanced compilation are listed below. Note: the example repo includes code going from js -> ts using clutz and then ts -> js using tsickle. It is a bit bigger than just a simple hello world app.

Uncompressed: 112K
Brotli quality 10: 29K
Gzip: 34K

Notes on Getting This to Work

Here are the notes on what it took to get advanced optimizations working.In case I forgot something, I'm going to provide the links to all our forks, which have all of our commits.

Example repo

Externs files for Zone, Reflect, and Jasmine were included in the build process, so those function calls were not renamed.

To work around a Closure Compiler bug where static methods get lost, we have a really hacky sed command that we run on the built Angular js files, replacing all lines which include a static keyword with the line preceded by /* @nocollapse /. Here's a link to the Closure bug: https://github.com/google/closure-compiler/issues/1776

Angular

The biggest change we made to Angular is making it use the Tsickle annotated output during compilation. We thought we did this last time, but we had a bug where the annotated output was being thrown away :D

There were a few places that Angular refers to functions as strings. These functions were referred to in other places using dot notation. Accordingly, some references were renamed and others were not. We change Angular to use dot notation everywhere for those functions, so that they are always renamed.

We also updated Angular/Tsickle to not annotate .d.ts files.

We made Angular play nicely with ES6, so it works in uncompiled mode. We change some places where .apply was used on a constructor to use the new keyword.

Tsickle

We fixed a bug in tsickle where the CLI did not always include all the source files during annotation. The list of filenames from the TypeScript Program should have been used, instead we were using a different list of filenames.

Links to all our forks of projects to make this work

Angular: https://github.com/lucidsoftware/angular/tree/closure-bundle Tsickle: https://github.com/lucidsoftware/tsickle/tree/closure-bundle RxJS: https://github.com/lucidsoftware/rxjs/tree/closure-hack symbol-observable: https://github.com/lucidsoftware/symbol-observable/tree/closure Clutz: https://github.com/lucidsoftware/clutz

alexeagle commented 7 years ago

So we have a couple examples of this working, plus we use JSCompiler internally at Google so we have many apps doing this (though using the Bazel build system and some not-yet-released Bazel rules).

@robwormald @mprobst should we try to package this up in a more easily reproducible way for external users? I don't think we can claim success and close this issue yet.

thelgevold commented 7 years ago

The JS closure compiler looks really cool. However, I have seen issues where the process runs out of memory when targeting medium sized AOT projects via Rollup. Have you encountered this in your testing?

The Java version does not seem to suffer from this problem though.

https://github.com/google/closure-compiler-js/issues/23

alexeagle commented 7 years ago

I've heard that too. We have had to call node with --max-old-space-size=4096 to get some tools to work in the past.

On Tue, Sep 20, 2016 at 11:27 AM Torgeir Helgevold notifications@github.com wrote:

The JS closure compiler looks really cool. However, I have seen issues where the process runs out of memory with targeting medium sized AOT projects. Have you encountered this in your testing?

The Java version does not seem to suffer from this problem though.

google/closure-compiler-js#23 https://github.com/google/closure-compiler-js/issues/23

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub https://github.com/angular/angular/issues/8550#issuecomment-248389501, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5I_FDlMdHvHO2YCvypXju9wR8onzuks5qsCWmgaJpZM4IaZIi .

thelgevold commented 7 years ago

I tried max-old-space-size with 8000 and 14000 on a Mac with 16GB of ram, but I still ran out of memory in my sample project. Java version works though.

alexeagle commented 7 years ago

oh, that seems bad :) could you file a bug on https://github.com/google/closure-compiler/issues

On Tue, Sep 20, 2016 at 11:52 AM Torgeir Helgevold notifications@github.com wrote:

I tried max-old-space-size with 8000 and 14000 on a Mac with 16GB of ram, but I still ran out of memory in my sample project. Java version works though.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/angular/angular/issues/8550#issuecomment-248396986, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5I25uOXJ-Pt8a9WzQQUO1zRx4j1YUks5qsCthgaJpZM4IaZIi .

thelgevold commented 7 years ago

Yup I have an active issue there already https://github.com/google/closure-compiler-js/issues/23

qdouble commented 7 years ago

@thelgevold that may be related to Typescript bug...try building with Typescript nightly build... (unfortunately, ngc doesn't support TS 2.1, so you'd have to go back and forward between that and 2.0.2).

thelgevold commented 7 years ago

@qdouble Hm interesting.. Are you saying it might be related to the JavaScript produced by TS 2.0.2? Because by the time my code enters into rollup/closure compiler land, the code is already in JS format.

qdouble commented 7 years ago

@thelgevold ah, with there is a bug that can stop your code from building if you have a lot of form controls and stuff with 2.0.2...if your files are already JS before you encounter the bug, then it may be something else...but I suppose you could try to use Typescript nightly to turn it into JS and see if it makes any difference.

u12206050 commented 7 years ago

Hi Guys, all of this sounds very interesting and we are looking into this to improve the performance of our website. What is the current status of the custom angular builds you are making. Will you be making a build for Angular v 2.0.0 ?

alexeagle commented 7 years ago

I will try to have some update on this for ngEurope...

On Fri, Oct 7, 2016, 1:48 AM Gerard A Lamusse notifications@github.com wrote:

Hi Guys, all of this sounds very interesting and we are looking into this to improve the performance of our website. What is the current status of the custom angular builds you are making. Will you be making a build for Angular v 2.0.0 ?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/angular/angular/issues/8550#issuecomment-252186017, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5I1TZZEGiuBu8zJe2BtYEzilr-VPHks5qxgdTgaJpZM4IaZIi .

jjudd commented 7 years ago

Hi @u12206050, I took a crack getting this upgraded to Angular 2.0.1 a couple weekends ago. I got it about 2/3 of the way there. The example works with simple optimizations, but not with advanced optimizations.

I'm no longer actively working on this, but another team here at Lucid I spoke with today wants to take a crack at it soon. If I get another free weekend, I'd like to finish moving it to 2.0.1. So whoever gets to it first.

In the meantime, if you would like a bit more info on this, you can check out the blog post we wrote on it at https://www.lucidchart.com/techblog/2016/09/26/improving-angular-2-load-times/. I hope that helps.

If you run into any issues and want to bounce some questions off of me, please feel free. It is likely going to be a bit of an adventure hooking it up to your build system, though. Best of luck! :)

steve8708 commented 7 years ago

any update from ngeurope by any chance?

after realizing that webpack2 tree shaking does not tree shake reexports, and angular requires importing from reexports, this is quite a bummer

my obvious thought was oh, tsickle! thought I'd pipe ngc -> tsickle -> webpack -> gcc advanced (instead of just ngc -> webpack -> gcc simple) only to realize tsickle converts import/require to goog.require, so there is no easy webpack (or any other non-google bundler, really) integration here currently

angular core + forms + router + platform-browser + common is taking up ~30% of our main bundle with ngc + webpack2. that's ~100kb minified and gzipped, ouch

I'd love to see tsickle working with webpack, as clearly even the angular cli team think webpack is a great choice. I'm open to any suggestions on this one. Otherwise I'll take a deeper look at @jjudd's work, but I'd hate to waste the time if an official solution is just on the horizon (perhaps with @ngtools/webpack?)

lypanov commented 7 years ago

https://docs.google.com/presentation/d/1SaHtM1_mpBZuN74wxAJSPQRB0sPbWRSJPQZsxx4_BpE/preview

Their Twitter feed seems to have some content.

Get Outlook for Androidhttps://aka.ms/ghei36

On Fri, Oct 28, 2016 at 12:52 AM +0200, "Steve Sewell" notifications@github.com<mailto:notifications@github.com> wrote:

any update from ngeurope by any chance?

after realizing that webpack tree shaking does not tree shake reexports, and angular requires importing from the reexports, this is quite a bummer

my obvious thought was oh, tsickle! thought I'd pipe ngc -> tsickle -> webpack (instead of just ngc -> webpack) only to realize tsickle converts import/require to goog.require, so there is no easy webpack integration here currently

angular core + forms + router + platform-browser + common is taking up ~30% of our main bundle with webpack2. that's ~100kb minified and gzipped, ouch

open to any suggestions on this one. otherwise I'll take a deeper look at @jjuddhttps://github.com/jjudd's work, but I'd hate to waste the time if an official solution is on the horizon

You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/angular/angular/issues/8550#issuecomment-256791543, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAABN_sAdZ1RDtPIk1Zt4QJr7oUdg723ks5q4SqbgaJpZM4IaZIi.

alexeagle commented 7 years ago

Update: we had some serious discussion at ngEurope, with @robwormald and @hansl and others. @pkozlowski-opensource is skeptical that the external community would embrace a new toolchain, rather than improving the existing one. So I think the first step is that angular-cli will work to improve webpack to do a better job.

At the same time, I would still like to have a canonical seed project that demonstrates using closure compiler. @steve8708 I'm not sure what you mean about goog.require breaking a bundler, since closure compiler is the bundler. No goog.require statements appear in its output. @jjudd is anyone at Lucidchart interested to collaborate to unroll our hacks and make it possible for the external community to repro the 26k ng2 app?

steve8708 commented 7 years ago

@alexeagle currently we use webpack for a lot of things - bundling, code splitting, preprocessing, linting, hashing, gzipping, and more, similar to what the angular CLI and a lot of the popular open source angular + webpack starters do

I'd really love to keep webpack for everything it does (esp. bc we can leverage a lot of community plugins to make some complex tasks simple and configurable) rather than reimplement everything we currently do (say with gcc + gulp).

I'd love to just be able to just be able to swap tsickle for tsc to add the jsdoc annotations during transpilation needed for advanced mode minification (we currently use a webpack gcc plugin in simple mode for minification)

Really appreciate your update though, and very eager to track the progress the angular-cli team makes with their webpack toolchain!

alexeagle commented 7 years ago

@steve8708 which tools do you want to be able to operate on the closure-annotated goog-module'd ES6 code before you hand it to your gcc plugin?

jjudd commented 7 years ago

@alexeagle Thanks for the update. I'll talk with people here and see what interest there is.

As for an example, is the example repo we built for this what you were thinking of? https://github.com/lucidsoftware/closure-typescript-example

Meligy commented 7 years ago

This sounds like a great thing for the CLI. It wraps Webpack and bunch of other plugins existing and new. It would be awesome if the CLI could use Closure compiler under the hood instead of uglifier. It's abstracted away from the user either way.

alexeagle commented 7 years ago

@jjudd yeah that's the best we have right now, but it's pinned to a particular (old) version of ng2 and its dependencies, right? We also need to explain how to use externs so you can have non-closure-compatible dependencies.

jjudd commented 7 years ago

True. We're actively working on moving to the latest version of Angular right now. It should hopefully be done very soon. I'll post an update when we get that done.

steve8708 commented 7 years ago

@alexeagle well frankly webpack is built around commonjs/amd/es6 modules, so a lot of features don't work properly (if at all) if modules aren't in this format. some quick ones that come to mind

url loader. We import assets in typescript (or html) as require('./asset.png') and simply by doing that webpack handles the copying, hashing, compressing, and loading of the images where needed
automatic code splitting (e.g. using an angular2 router loader like this one) and any other System.import throughout your code for easy lazy loading of any content
various other loaders - for example we use the import loader for angular2 material as material currently has window references to find typescript helpers like __extends so we must rewrite that to global (example). Even things like this really only work most elegantly (or, less hackily) with webpack when you have it handling your bundling as it is intended to
various other plugins - for example the html plugin will produce our base index.html after running the full build/bundle since from doing that it is aware of what entry javascript files have been created (including their full paths and hashes) and produces our root html template with the correct script urls with hashes, etc appropriately

frankly I've implemented this same (or similar) stuff plenty of times with anything from make/jakefiles, grunt, gulp, etc but I've never had nearly such a seamless and solid build tooling experience as using webpack (esp. for the amount of code you must write and maintain for your build), so I'd love to see tsickle allowing for more common (in open source land) module formats that can integrate with tools like webpack

tl;dr, webpack just depends on using commonjs, amd, or es imports, and they have quite an awesome system of loaders and plugins that can do a lot of great things as long as webpack can understand your modules in any of the common javascript formats, which unfortunately does not include any goog.* modules

lmk if there is anything else I can help to answer here or even code contributions I can make to try and use tsickle with other tools such as webpack. I'm very eager to get this going however possible!

and thanks!

alexeagle commented 7 years ago

@steve8708 the reason we currently hand goog.module syntax to closure compiler is that it has bugs/missing features for ES6 modules. Last time I checked they had no support for export * for example. The team has indicated they are not interested in supporting ES6 modules since they don't work natively in most browsers.

If you need to take TS code through these steps before bundling/treeshaking/minifying with closure compiler, we'd need to fix all the ES6 module support in closure compiler. I don't think anyone owns this right now.

steve8708 commented 7 years ago

Good to know, thanks @alexeagle

I just found some docs suggesting gcc supports commonjs modules as well. I'll test it out a bit and find out if maybe it's more implemented or less buggy than es6 modules currently.

If so I'll poke around and see if maybe it's possible to make the goog module conversion optional in tsickle and if there is a way to get the benefits of gcc advanced minification with jsdoc type annotations pulled from typescript but with commonjs or es6 modules. I think that would certainly be nice for myself and the rest of the community!

Thanks again for the info, it's very helpful!

alexeagle commented 7 years ago

Discussed some more here in-person ... if the only bug with closure compiler is export *, we could easily make a tsickle option to flatten that into export {symbol1, symbol2, ...} in the post-processing step. We could also add the JSDoc annotations (and include these in our ES6 distributions).

steve8708 commented 7 years ago

@alexeagle that'd be amazing to get the jsdoc annotations in the ES6 distributions!

From some testing today I found that tsickle actually already does convert export * to export { a, b, c } format for es6 modules, so that is great news!

Even better, I cloned tsickle locally and commented out the conversion to goog modules and was able to get tsickle + gcc advanced working perfectly with our webpack build just as hoped!

This got our main bundle down a full 15% smaller, and that's without anything in @angular/* being annotated yet. This is looking very exciting and promising

On top of that, from the investigation I did today I see that when you transpile with es6 modules none are replace with goog.require, only commonjs require calls are. This is really good news as it means projects can use tsickle as-is already today with tools that support esmodules like rollup and webpack2 as long as they replace all commonjs require calls with es6 imports.

Fortunately ts 2.0 made this much easier with declare module '*', eliminating the need to use require for non-typescript code

I'm really eager to see how much more we can save when @angular/* es distributions have type annotations from tsickle as well! Especially while webpack2 still doesn't tree shake reexports the savings from removing lots of bundled but unused angular core/common/etc code here could be really big!

thelgevold commented 7 years ago

Decided to give this a try as well.

Cloned latest tsickle, but still running into a few issues:

For me there are two categories of errors: 1) export * compilation errors from closure - Same error described by others above...

node_modules/@angular/common/src/location.js:9 (JSC_CANNOT_CONVERT_YET)
ES6 transpilation of ''Wildcard export'' is not yet implemented.
export * from './location/location_strategy';

Will the solution be to publish Angular source without export * here ?

2) closure keywords in the angular source code comments causing compilation errors

node_modules/@angular/core/src/metadata/ng_module.js:15 (JSC_PARSE_ERROR)
Parse error. illegal use of unknown JSDoc tag "stable"; ignoring it
 * @stable

I am doing my bundling using rollup with the closure compiler plugin. I am using rxjs-es instead of rxjs since I ran into a few issues with commonJS conversions.

I am curious about the plan for tsickle vs ngc. From what I can tell you now have to run them in tandem ngc -> tsickle. Are there plans to combine them to reduce this to one step?

I am very interested in seeing how far this can go. I guess it really comes down to the structure of the code, but there could be potential for huge code reductions. Did a non angular POC with closure here: http://www.syntaxsuccess.com/viewarticle/tree-shaking-in-javascript

From what I can tell, Closure has an enormous potential when it comes to code optimization. My sample reduced several classes to a one liner, but still unclear how well this will translate to Angular applications. Hopeful though!

alexeagle commented 7 years ago

Now that PR is in so you can use

    "@angular/common": "angular/common-builds",
    "@angular/compiler": "angular/compiler-builds",
    "@angular/compiler-cli": "angular/compiler-cli-builds",
    "@angular/core": "angular/core-builds",
    "@angular/platform-browser": "angular/platform-browser-builds",
    "@angular/platform-server": "angular/platform-server-builds",
    "@angular/tsc-wrapped": "angular/tsc-wrapped-builds",

and there is no export * and all the code has JSDoc for closure.

@thelgevold @steve8708 want to try again? I'll investigate https://github.com/cramforce/splittable with @robwormald

steve8708 commented 7 years ago

@alexeagle awesome!

I'm trying this out now and when using the -builds versions I get this error from ngc:

Error: Metadata version mismatch for module [path to a project .ts file], found version 1, expected 2

I'm not fully sure the meaning of this message, do you have any thoughts on how to resolve it?

alexeagle commented 7 years ago

@tbosch made a change subsequent to mine. He updated the metadata collector, and now Angular expects to read version 2 metadata. However, when version 1 metadata is encountered it should upgrade-in-place. Looks like a bug, I'll let Tobias confirm...

alexeagle commented 7 years ago

this is the commit: https://github.com/angular/angular/commit/dddbb1c1cb08115f339e34487774c85a3c6493f3

alexeagle commented 7 years ago

one thing to try, do you have any generated .metadata.json files in your project? try cleaning them and retry?

thelgevold commented 7 years ago

@alexeagle I have been playing with this a little bit, but I am running into compilation issues with named imports like @angular/core

I think I have read somewhere that closure doesn't currently handle named imports. If I go through and convert the imports to absolute paths it seems to fix the compilation errors. There are too many errors to fix them all manually though.

Converting references like @angular/core to ../../core/index in angular/application code seems to work....

Here is a sample error:

node_modules/@angular/common/src/pipes/slice_pipe.js:8: ERROR - required "module$$angular$core" namespace never provided
import { Pipe } from '@angular/core';

Gave your repo a try as well and it seems like the same thing is happening there too.

I wonder if something could be added to closure to resolve a named import like @angular/core to the corresponding physical path.

alexeagle commented 7 years ago

@thelgevold I found that as well. The problem is that node_modules/@angular/core/index.js is registered as module$$angular$core$index. Since ESModule semantics are undefined, there is no spec saying that "foo/index.js" may be imported with "foo". Since CommonJS allows this, rollup and systemjs support it, but it's unspecified. Therefore the response from the closure team has been that it's a module loader problem, and they don't support this. https://github.com/google/closure-compiler/issues/1710 (and also a chat with @MatrixFrog) I suspect that changing import {} from '@angular/core' to from '@angular/core/index' solves this as well, though it's not a useful workaround since user code would have to do this too.

I imagine that converting the module syntax to commonjs is another approach, though adding a tool to do this makes the build that much harder for anyone to reliably reproduce.

steve8708 commented 7 years ago

Yeah, unfortunately that's the same issue I'm hung up on now and not sure if there really is a straightforward solution :(

We may have to give up on tsickle again since even if for our own imports we add /index to the end of paths, imports within libraries (e.g. @angular/platform-browser importing from @angular/core, etc) will also not have the correct paths for gcc support

I'll give it more thought but in the meantime it sounds like gcc advanced may not be a possibility for the general user anytime soon unless anyone else has any other ideas

angular / angular