search

angular / closure-demo

MIT License
114 stars 27 forks source link

Working with closure compiler 20170930 HEAD + PR2641 with an entry point work-around and with 20170930 HEAD + PR2641 + patch as is #21

Closed gregmagolan closed 6 years ago

gregmagolan commented 7 years ago

Did some investigation into the PR2641 closure code with the HEAD and it works with this repo with an entry-point work-around or with a patch as is. The investigation results are below. This PR brings the compiler up to 20170930 HEAD + PR2641 + two patches (one of which is not needed to make this repo work but is needed to fix some AOT builds that have ngfactory files with ambiguous import paths).

PR2641

After some investigation, things are looking pretty good with closure. PR 2641 does indeed work with closure-demo and my abc-demo-build-with-aot-universal with a work-around to set the entry point so that it is is match correctly to the input.

For example in the current closure-demo conf,

--js built/**.js
--js_module_root=built

--entry_point=./built/src/bootstrap

The input built/src/bootstrap.js resolves in closure to module$$src$$bootstrap (since built is the js_module_root). But with PR2641, it resolves the entry point --entry_point=./built/src/bootstrap to module$$built$$src$$bootstrap, which doesn't match the input. Without an entry point to analyze the dependency depths, the build completes with an invalid bundle (bundle end ups up being 100 bytes or so).

The work-around in the closure conf in closure-demo to make it work with PR2641 is:

--js built/**.js
--js_module_root=built

--entry_point=src/bootstrap

The closure compiler then correctly matches up the entry point to the input and the build works. Ideally, the build would error out if the entry point does not match an input but that doesn't happen.

I prepared a dist of the PR2641 that you can the above with here: https://github.com/gregmagolan/closure-compiler/tree/PR2641.angular.dist

I also made a patch on top of PR2641 so that the compiler properly matches --entry_point=./built/src/bootstrap to the input built/src/bootstrap.js. With this patch, the compiler handles both --entry_point=./built/src/bootstrap and --entry_point=src/bootstrap correctly.

The patch is simple: https://github.com/gregmagolan/closure-compiler/commit/0a147fa1e3c37f5f203d0083acd06c6cc772592c

The dist with that patch to try is here: https://github.com/gregmagolan/closure-compiler/tree/PR2641+patch.angular.dist

PR2641 + 20170930 HEAD

Testing with PR2641 merged into the 20170930 HEAD (which should be close to the next closure release), the same applies entry point issue as with PR2641 but there is also an additional regression with ambiguous module resolution. You get compiler errors in some cases when compiling ngfactory files for AOT trying to resolve ambiguous imports in the ngfactory files. Compile errors look like this:

./closure-bin/src/browser/app/app.component.ngfactory.closure.js:4: ERROR - Failed to load module "src/browser/app/app.component.closure"
import * as i3 from 'src/browser/app/app.component.closure';
^

This doesn't happen on closure-demo as the imports end up relative in the ngfactory files in that repo, but I do see it an abc-demo-build-with-aot-universal and the angular-bazel-demo also generates ngfactory files with ambiguous imports.

An additional patch is needed with 20170930 HEAD + PR2641 to resolve this regression (this is 1 of the two original patches from 20170919 dist -- the 2nd patch is no longer needed): https://github.com/gregmagolan/closure-compiler/commit/fbddb40fd64db58cbeeb9fd73af96992caebb31d

The dist of PR261 + 20170930 HEAD to try is here: https://github.com/gregmagolan/closure-compiler/tree/20170930-HEAD+PR2641.angular.dist

The dist of PR261 + 20170930 HEAD + entry point patch to try is here: https://github.com/gregmagolan/closure-compiler/tree/20170930-HEAD+PR2641+patch.angular.dist

The dist of PR2641 + 20170930 HEAD + entry point patch + ambiguous import resolution patch to try is here: https://github.com/gregmagolan/closure-compiler/tree/20170930-HEAD+PR2641+patches.angular.dist

Summary of dists and compatibility

PR2641: https://github.com/gregmagolan/closure-compiler/tree/PR2641.angular.dist

PR2641 + entry point patch: https://github.com/gregmagolan/closure-compiler/tree/PR2641+patch.angular.dist

20170930 HEAD + PR2641: https://github.com/gregmagolan/closure-compiler/tree/20170930-HEAD+PR2641.angular.dist

20170930 HEAD + PR2641 + entry point patch: https://github.com/gregmagolan/closure-compiler/tree/20170930-HEAD+PR2641+patch.angular.dist

20170930 HEAD + PR2641 + entry point patch + ambiguous import resolution patch: https://github.com/gregmagolan/closure-compiler/tree/20170930-HEAD+PR2641+patches.angular.dist

Getting fixes upstream

If we can get the two fixes upstream then we should be good for the next closure release for Angular 5.0.0

1) ambiguous import resolution patch on HEAD: https://github.com/gregmagolan/closure-compiler/commit/fbddb40fd64db58cbeeb9fd73af96992caebb31d

2) entry point resolution patch on top of PR2641 or on HEAD after PR2641 is merged in: https://github.com/gregmagolan/closure-compiler/commit/0a147fa1e3c37f5f203d0083acd06c6cc772592c

language_out regression

Also of interest is that with the HEAD, I had to change the language out in the abc-demo-build-with-aot-universal repo from ES5_STRICT (which use to work) to just ES5. Using ES5_STRICT now has a runtime error with ADVANCED_OPTIMIZATIONS (it still works with SIMPLE_OPTIMIZATIONS)

gregmagolan commented 6 years ago

Deprecated