[2.2.0-rc.0] AoT can't add UpgradeComponent to @NgModule.entryComponents

[guojenman]

I'm submitting a ... (check one with "x")

[x] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

UpgradeComponent can not be added to entryComponents of @NgModule.

a) with JIT

1. compiles OK
2. upgraded component does not render
3. console error: Uncaught Error: Could not compile 'ForUpgradeDirective' because it is not a component

b) with AoT

1. compile FAILED
2. compile error:
   ERROR in ./_aot/module.ngfactory.ts
   (79,7): error TS2304: Cannot find name 'ForUpgradeDirectiveNgFactory'.

If you remove "ForUpgradeDirective" from entryComponents in src/module.ts

c) with JIT

1. compiles OK
2. displays OK, browser shows
   ng1: hello from ng1
   ng2: hello from ng1

d) with AoT

1. compiles OK
2. display NOT OK, browser shows
   ng1: hello from ng1
   ng2:

Expected behavior

Should compile & render UpgradeComponent when added to @NgModule.entryComponents

Minimal reproduction of the problem with instructions

test-upgrade.zip

for a & b above
a) npm run jit
b) npm run aot

for c & d above, remove ForUpgradeDirective from entryComponents and again run
c) npm run jit
d) npm run aot

What is the motivation / use case for changing the behavior?

Can't upgrade ng1 components and use AoT at the same time

Please tell us about your environment:

• Angular version: 2.2.0-rc.0

• Browser: [all]

• Language: [ TypeScript 2.0.2]

• Node (for AoT issues): node --version = v6.6.0

[manfredsteyer]

same here

[alexeagle]

@bourey is this one of the issues that you solved?

[bourey]

No, I don' think either the sample app I put together or the one vsavkin recently updated use UpgradeComponent.

[manfredsteyer]

Exactly. None of them upgrades an ng1 component. But there is an test case for this:
https://github.com/angular/angular/blob/master/modules/%40angular/upgrade/test/aot/integration/upgrade_component_spec.ts

[manfredsteyer]

Just tried it out with the new 2.2 release that has been released some moments ago. I'm still facing this issue. As currently we just have this test case and no running demo-app, I'm still wondering whether it is a bug or whether we (guojenman above and I) did something wrong.

Can please anyone elaborate on this?

[gkalpak]

I tried to reproduce the problem using test-upgrade.zip from above, but I get a different error related to webpack:

> node config/build_aot.js

concating ng1 libs...
concating ng2 libs...
Hash: 2e069513c7002a3a4fb1
Version: webpack 2.1.0-beta.26
Time: 89ms

ERROR in Entry module not found: Error: Can't resolve 'ts' in '/home/gkalpak/Dev/test-upgrade'

[guojenman]

latest beta of webpack no longer appends "-loader" to loaders, so I'm updating webpack config in zip and re-uploading. stay tuned...

[guojenman]

Updated the original post with fixed webpack config. Also uploading on this post for convenience.

test-upgrade.zip

[gkalpak]

First of all, you don't need the upgraded component to be listed in entryComponents, because it is not an entry component (i.e. it is only loaded as part of another component's template).

Still, AoT won't fully work, because the DirectiveWrapperBuilder fails to detect the presence of ngOnInit() (which is on UpgradeComponent's prototype) and thus the generated code never calls ngOnInit(), which is where UpgradeAdapter links the (already compiled) ng1 directive.
So, "no ngOnInit()" --> "no ng1 linking" --> "no inserting the compiled template into the DOM" 😞

The same issue affects other lifecycle hooks, such as $postLink, $onDestroy() etc.

The easy (but verbose) work-around is to add the lifecycle hooks on the upgraded component yourself so they can be detected during AoT compilation:

@Directive({...})
class ForUpgradeDirective extends UpgradeComponent {
  constructor(...) { ... }
  ngOnInit() { return super.ngOnInit(); }
  // Same for ngDoCheck, ngOnChanges, ngOnDestroy
  ...
}

I am not sure whether it is possible/desirable for the metadata collectors - used to...collect metadata about directives/components - to detect lifecycle hooks higher up in the prototype chain.

(I suspect this has been discussed before. Maybe @alexeagle knows more.)

[alexeagle]

It has been discussed - I just synced with @tbosch
Indeed the metadata.json currently does not capture the superclass. So the code generation cannot know that the type has lifecycle hooks.
There is a broader problem where JIT mode understands inherited lifecycle interfaces in some cases but not others - so Tobias would like to fix this in a more consistent way.
For now the general guidance is "no inheritance in components" so the explicit ngOnInit is the workaround.

[manfredsteyer]

Thanks Georgios and Alex! That works for me perfectly.

For some reason, the above mentioned test case also work without implementing those live-cycle-hooks.

[gkalpak]

tl;dr

Hook	Required?
ngOnInit	Always.
ngOnChanges	When you have bindings or rely on $onChanges().
ngDoCheck	When you have two-way (=) bindings.
ngOnDestroy	When you don't want to risk* leaky scopes or when you rely on $onDestroy().

_{*: Technically, this is not necessary in some situations (e.g. when a scope is destroyed higher up in the hierarchy), but it is something that is outside the control of the upgraded component, so I would not recommend leaving ngOnDestroy out.}

It works, because the forUpgrade component does not rely on on other hooks (e.g. it does not have bindings, does no cleanup on destroy etc). Here is which lifecycle hooks are used by UpgradeComponent and what for:

1. ngOnInit:
   • Resolve and bind required controllers on the ng1 controller instance.
   • Call ng1's $onInit() lifecycle hook.
   • Run ng1's linking phase (this is where the compiled template gets bound to the scope and inserted into the DOM).
   • Call ng1's $postLink() lifecycle hook.
2. ngOnChanges:
   • Update bindings on the ng1 controller or isolate scope.
   • Call ng1's $onChanges() lifecycle hook.
3. ngDoCheck:
   • Propagate changes in two-way (=) bindings from ng1 to ng2.
4. ngOnDestroy:
   • Destroy the scope created for the ng1 component/directive to live in.
   • Call ng1's $onDestroy() lifecycle hook.

So, depending on what features your ng1 component relies on, you might be able to get by with omitting some of the hooks.

It goes without saying, that the above info is based on the current (v2.2.0) implementation of ngUpgrade and may change in the future. If you want to be 100% safe, you have to implement and forward all lifecycle hooks on components that extend UpgradeComponent and will run in AoT mode.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}