angular / closure-demo

MIT License
115 stars 27 forks source link

Upgrade this repo to angular 6 #32

Closed steveblue closed 6 years ago

steveblue commented 6 years ago

I noticed the @angular packages have been updated to the latest Package Format 6.0 so I tried to update my closure.conf to mirror the changes. @alexeagle @gregmagolan is there any guidance for updating to 6.0.0? I think I figured it out.

I got the below conf to bundle with google-closure-compiler@20180319.0.0, @angular@6.0.0-rc.1 and rxjs@6.0.0-beta.4.

I used rollup to bundle rxjs and rxjs/operators into FESM and then modified rxjs and rxjs/operators package.json to point to the FESM instead of the ES2015 bundles. Perhaps rxjs should include FESM by default, I doubt this would be hard for them to implement seeing as rollup just bundled these packages in ms.

---compilation_level=ADVANCED_OPTIMIZATIONS
--language_out=ES5
--variable_renaming_report=closure/variable_renaming_report
--property_renaming_report=closure/property_renaming_report
--create_source_map=%outname%.map

--warning_level=QUIET
--dependency_mode=STRICT
--rewrite_polyfills=false
--module_resolution=NODE

--externs closure.externs.js
--externs node_modules/zone.js/dist/zone_externs.js
--externs node_modules/@angular/core/src/testability/testability.externs.js

--js node_modules/rxjs/package.json
--js node_modules/rxjs/_fesm2015/index.js

--js node_modules/rxjs/operators/package.json
--js node_modules/rxjs/_fesm2015/operators/index.js

--js node_modules/@angular/core/package.json
--js node_modules/@angular/core/fesm2015/core.js

--js node_modules/@angular/common/package.json
--js node_modules/@angular/common/fesm2015/common.js

--js node_modules/@angular/platform-browser/package.json
--js node_modules/@angular/platform-browser/fesm2015/platform-browser.js

--js node_modules/@angular/forms/package.json
--js node_modules/@angular/forms/fesm2015/forms.js

--js node_modules/@angular/http/package.json
--js node_modules/@angular/http/fesm2015/http.js

--js node_modules/@angular/common/http/package.json
--js node_modules/@angular/common/fesm2015/http.js

--js node_modules/@angular/router/package.json
--js node_modules/@angular/router/fesm2015/router.js

--js node_modules/@angular/animations/package.json
--js node_modules/@angular/animations/fesm2015/animations.js

--js node_modules/@angular/animations/browser/package.json
--js node_modules/@angular/animations/fesm2015/browser.js

--js node_modules/@angular/platform-browser/animations/package.json
--js node_modules/@angular/platform-browser/fesm2015/animations.js

--js ngfactory/**.js
--js main.js

--package_json_entry_names es2015

--entry_point=./main
alexeagle commented 6 years ago

We have some local patches to rxjs internally at google, I imagine some of those might be required to make it work with Closure Compiler - @vikerman would know?

We should indeed update this repo to demonstrate Angular 6.0 before final, thanks for the reminder. I hope we'll be able to get to it, but it's possible we will run out of time, still fixing other Bazel-related things for the release.

steveblue commented 6 years ago

^ @alexeagle modified the original issue with a working conf.

steveblue commented 6 years ago

I can probably find some time to submit a PR, if above solution is workable.

alexeagle commented 6 years ago

Would this require users to manually create the rxjs fesm and patch the package.json file? I think we'd rather figure out what is wrong with having closure import the individual files we compiled from rxjs, so this setup remains the same as what we do internally.

steveblue commented 6 years ago

Yeah, it is very hacky at this point, I agree. At first glance the paths in the es2015 rxjs modules seemed to be what are tripping closure compiler up and going back to cjs seems like a no go with rxjs-compat.

alexeagle commented 6 years ago

it would be easier to follow if you included some snippets of the errors so we could compare against our internal fixes.

steveblue commented 6 years ago

@alexeagle ask and you shall receive. This is when referencing the _esm2015 modules.

--js node_modules/rxjs/package.json
--js node_modules/rxjs/_esm2015/index.js

--js node_modules/rxjs/operators/package.json
--js node_modules/rxjs/_esm2015/operators/index.js
...
--package_json_entry_names es2015

The amount of errors are pretty immense so here is a snippet:

node_modules/rxjs/_esm2015/index.js:57:
Originally at:
node_modules/src/index.ts:66: ERROR - Failed to load module "./internal/observable/empty"

node_modules/rxjs/_esm2015/index.js:58:
Originally at:
node_modules/src/index.ts:67: ERROR - Failed to load module "./internal/observable/never"

node_modules/rxjs/_esm2015/index.js:60:
Originally at:
node_modules/src/index.ts:73: ERROR - Failed to load module "./internal/config"

node_modules/rxjs/_esm2015/operators/index.js:2:
Originally at:
node_modules/src/operators/index.ts:3: ERROR - Failed to load module "../internal/operators/audit"

node_modules/rxjs/_esm2015/operators/index.js:3:
Originally at:
node_modules/src/operators/index.ts:4: ERROR - Failed to load module "../internal/operators/auditTime"

node_modules/rxjs/_esm2015/operators/index.js:4:
Originally at:
node_modules/src/operators/index.ts:5: ERROR - Failed to load module "../internal/operators/buffer"

node_modules/rxjs/_esm2015/operators/index.js:5:
alexeagle commented 6 years ago

in Angular we have an integration test for this: https://github.com/angular/angular/tree/master/integration/hello_world__closure given that it's green, I think the task here is just to upgrade this repo to match, then to figure out what's different between that working example and your broken one.

alexeagle commented 6 years ago

In particular note that we use google-closure-compiler@20171023.0.1 - anything later is not known to work

steveblue commented 6 years ago

When I use a similar conf it builds OK, but in the browser console there is this error when trying to execute the bundle, so this may not be revealed by your integration tests?

Using --js node_modules/rxjs/**.js and --process_common_js_modules:

zone.js:675 Unhandled Promise rejection: Cannot read property 'prototype' of undefined
  Evaluating http://localhost:4200/bundle.js
  Loading bundle ; Zone: <root> ; Task: Promise.then ; Value: Error: Cannot read property 'prototype' of undefined
  Evaluating http://localhost:4200/bundle.js
  Loading bundle

It appears Zone is throwing the Error.

zone.js:675 Unhandled Promise rejection: Cannot read property 'prototype' of undefined
  Evaluating http://localhost:4200/bundle.js
  Loading bundle ; Zone: <root> ; Task: Promise.then ; Value: Error: Cannot read property 'prototype' of undefined
  Evaluating http://localhost:4200/bundle.js
  Loading bundle
    at Object.$jscomp.inherits (bundle.js:formatted:98)
    at eval (bundle.js:formatted:12687)
    at eval (<anonymous>)
    at Re (system.js:4)
    at system.js:4
    at S (system.js:4)
    at E (system.js:4)
    at k (system.js:4)
    at system.js:4
    at ZoneDelegate.invoke (zone.js:388) TypeError: Cannot read property 'prototype' of undefined
    at Object.$jscomp.inherits (http://localhost:4200/bundle.js:6:67)
    at eval (http://localhost:4200/bundle.js:926:38)
    at eval (<anonymous>)
    at Re (http://localhost:4200/lib/systemjs/dist/system.js:4:25523)
    at http://localhost:4200/lib/systemjs/dist/system.js:4:33132
    at S (http://localhost:4200/lib/systemjs/dist/system.js:4:7538)
    at E (http://localhost:4200/lib/systemjs/dist/system.js:4:7038)
    at k (http://localhost:4200/lib/systemjs/dist/system.js:4:6037)
    at http://localhost:4200/lib/systemjs/dist/system.js:4:38977
    at ZoneDelegate.invoke (http://localhost:4200/lib/zone.js/dist/zone.js:388:26)
api.onUnhandledError @ zone.js:675
handleUnhandledRejection @ zone.js:702
_loop_1 @ zone.js:692
api.microtaskDrainDone @ zone.js:696
drainMicroTaskQueue @ zone.js:602
Promise.then (async)
scheduleMicroTask @ zone.js:578
ZoneDelegate.scheduleTask @ zone.js:410
Zone.scheduleTask @ zone.js:232
Zone.scheduleMicroTask @ zone.js:252
scheduleResolveOrReject @ zone.js:872
resolvePromise @ zone.js:818
(anonymous) @ zone.js:734
Promise.then (async)
(anonymous) @ zone.js:1072
ZoneAwarePromise @ zone.js:901
Ctor.then @ zone.js:1071
resolvePromise @ zone.js:785
(anonymous) @ zone.js:883
ZoneDelegate.invokeTask @ zone.js:421
Zone.runTask @ zone.js:188
drainMicroTaskQueue @ zone.js:595
Promise.then (async)
scheduleMicroTask @ zone.js:578
ZoneDelegate.scheduleTask @ zone.js:410
Zone.scheduleTask @ zone.js:232
Zone.scheduleMicroTask @ zone.js:252
scheduleResolveOrReject @ zone.js:872
resolvePromise @ zone.js:818
(anonymous) @ zone.js:734
Promise.then (async)
(anonymous) @ zone.js:1072
ZoneAwarePromise @ zone.js:901
Ctor.then @ zone.js:1071
N @ system.js:4
(anonymous) @ system.js:4
ZoneDelegate.invoke @ zone.js:388
Zone.run @ zone.js:138
(anonymous) @ zone.js:882
ZoneDelegate.invokeTask @ zone.js:421
Zone.runTask @ zone.js:188
drainMicroTaskQueue @ zone.js:595
Promise.then (async)
scheduleMicroTask @ zone.js:578
ZoneDelegate.scheduleTask @ zone.js:410
Zone.scheduleTask @ zone.js:232
Zone.scheduleMicroTask @ zone.js:252
scheduleResolveOrReject @ zone.js:872
resolvePromise @ zone.js:818
(anonymous) @ zone.js:734
Promise.then (async)
(anonymous) @ zone.js:1072
ZoneAwarePromise @ zone.js:901
Ctor.then @ zone.js:1071
resolvePromise @ zone.js:785
(anonymous) @ zone.js:883
ZoneDelegate.invokeTask @ zone.js:421
Zone.runTask @ zone.js:188
drainMicroTaskQueue @ zone.js:595
Promise.then (async)
scheduleMicroTask @ zone.js:578
ZoneDelegate.scheduleTask @ zone.js:410
Zone.scheduleTask @ zone.js:232
Zone.scheduleMicroTask @ zone.js:252
scheduleResolveOrReject @ zone.js:872
resolvePromise @ zone.js:818
(anonymous) @ zone.js:734
Promise.then (async)
(anonymous) @ zone.js:1072
ZoneAwarePromise @ zone.js:901
Ctor.then @ zone.js:1071
N @ system.js:4
(anonymous) @ system.js:4
ZoneDelegate.invoke @ zone.js:388
Zone.run @ zone.js:138
(anonymous) @ zone.js:882
ZoneDelegate.invokeTask @ zone.js:421
Zone.runTask @ zone.js:188
drainMicroTaskQueue @ zone.js:595
Promise.then (async)
scheduleMicroTask @ zone.js:578
ZoneDelegate.scheduleTask @ zone.js:410
Zone.scheduleTask @ zone.js:232
Zone.scheduleMicroTask @ zone.js:252
scheduleResolveOrReject @ zone.js:872
ZoneAwarePromise.then @ zone.js:992
i.import @ system.js:4
Ve.import @ system.js:4
(anonymous) @ system.import.js:1
steveblue commented 6 years ago

@alexeagle I tested the following:

angular@6.0.0-rc.1
rxjs@6.0.0-beta.4

--process_common_js_modules 20171023.0.1: builds OK, in browser bundle will load the root Route but same error as above on any route change 20180319.0.0: builds OK, breaks on load, can't load initial Route

--package_json_entry_names es2015 and rxjs FESM

20171023.0.1: OK 20180319.0.0: OK

--package_json_entry_names es2015 and rxjs _esm2015

20171023.0.1: Build fails, appears paths in rxjs esm2015 modules are tripping up closure compiler 20180319.0.0: Build fails, appears paths in rxjs esm2015 modules are tripping up closure compiler Errors like below:

node_modules/rxjs/_esm2015/operators/index.js:103:
Originally at:
node_modules/src/operators/index.ts:104: ERROR - Failed to load module "../internal/operators/zip"

node_modules/rxjs/_esm2015/operators/index.js:104:
Originally at:
node_modules/src/operators/index.ts:105: ERROR - Failed to load module "../internal/operators/zipAll"

153 error(s), 0 warning(s)
steveblue commented 6 years ago

OK, it would appear the issue with rxjs CJS may pertain to @angular/router as I am able to build this repo with angular@6.0.0-rc.1 and rxjs@6.0.0-beta.4 with --process_common_js_modules

alexeagle commented 6 years ago

Can you phrase the problem in terms of a PR to the integration/hello_world__closure directory that makes it use the router?

On Tue, Apr 3, 2018 at 2:25 PM Steve Belovarich notifications@github.com wrote:

OK, it would appear the issue may pertain to @angular/router as I am able to build this repo with angular@6.0.0-rc.1 and rxjs@6.0.0-beta.4 with --process_common_js_modules

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/angular/closure-demo/issues/32#issuecomment-378404338, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5I2pW56uQlga8-8GjYBXhKRCgLC2jks5tk-i4gaJpZM4TEPec .

steveblue commented 6 years ago

@alexeagle the rxjs esm2015 issue is reproducible in https://github.com/steveblue/closure-demo and the rxjs cjs issue with the router is reproducible in https://github.com/steveblue/angular6-closure-example if you downgrade to --process_common_js_modules

steveblue commented 6 years ago

OK @alexeagle I don't think using the cjs modules are a sustainable path forward with rxjs 6.0.0 IMHO

alexeagle commented 6 years ago

Fixed in 651ac47

@steveblue FYI my plan is to move development of this repo to https://github.com/angular/angular/tree/master/integration/hello_world__closure and automatically publish snapshots from green CI to this repo.