OnInit is not respecting inheritance

[trinarytree]

Suppose you have this class hierarchy
A
|
B
and A implements OnInit but B does not explicitly declare that it implements OnInit, only that it extends A. Then angular will (incorrectly) not call ngOnInit when it creates an instance of B.

[PatrickJS]

are you calling super.ngOnInit?

[zoechi]

If B doesn't have (override) ngOnInit() there is no place where super.ngOnInit() could be called.

[PatrickJS]

This is the setup correct?

import {bootstrap} from 'angular2/platform/browser';
import {Component, OnInit} from 'angular2/core';

class B {
  ngOnInit() {
    console.log('B ngOnInit');
  }
}

@Component({ selector: 'a-b', template: 'AB' })
class A extends B implements OnInit {
  constructor() {
    super();
  }
  ngOnInit() {
    super.ngOnInit(); // call B's ngOnInit
    console.log('A ngOnInit');
  }
}

bootstrap(A);

[zoechi]

@gdi2290 > but B does not explicitly declare that it implements OnInit, only that it extends A

Ok, so you mean this only refers to implements OnInit. I had the impression that this isn't necessary in TS anyway for lifecycle methods to work (in Dart which I use, implements OnInit is mandatory).

@trinarytree can you please confirm?

[robwormald]

i can't reproduce this at all. please provide a plunker reproduction? See https://plnkr.co/edit/f3kcWBPF64IRCGUmUKPF?p=preview for a functioning version.

Note that implements OnInit is purely a compile time construct, it has no effect during runtime (in TS)

[trinarytree]

I'm amazed at how many comments this got between fri 22:52 and sat. I don't normally work so much on saturday morning. :-)

@gdi2290

are you calling super.ngOnInit?

No. A already implements ngOnInit, and in my case B is fine with inheriting A's version.

This is the setup correct?

No. It looks like this (in Dart):

abstract class A implements OnInit {
  @override
  void ngOnInit() {
    print('A#ngOnInit');  // This never gets called, but should.  Hence the bug report.
  }
}

@Component(
    selector: 'b',
    template: 'angular should but does not call my ngOnInit'
)
class B extends A {}

I suspect that this bug only happens when Angular compiles Dart code - it might not show up in TypeScript. Imagine what would happen if the code transformer used reflection incorrectly, asking whether B explicitly declared that it implements OnInit instead of asking whether it's a subclass.

[zoechi]

Could you please try to add implements OnInit to B and report back if this makes it work?

[ThorstenHans]

having the same issue here, giving the advice of @zoechi a chance now

[ThorstenHans]

for me the following combination works in beta2

// file base.ts
import {OnInit} from 'angular2/core';

export abstract class BaseComponent implements OnInit{

  ngOnInit(){
    console.log('base init');
  }
}

// file child.ts
import {Component, OnInit, Input,ChangeDetectionStrategy} from 'angular2/core';
import {BaseComponent} from './base';

@Component({
    selector: 'child-1',
    templateUrl: 'app/components/children/child.html'
})
export class Child1Component extends BaseComponent{
    constructor(){
        super()
    }
}

Also when I remove abstract it keeps working
Removing the constructor of the ChildComponent has also no side effects

[trinarytree]

If you add implements OnInit to B, then, when Angular creates an instance of B, yes it will call A's ngOnInit, but this isn't what the bug is about. The problem is when B does not make such a declaration but relies upon the fact that B extends A, which implements OnInit. In that case, B is a subtype of OnInit, i.e. if b is an instance of B, then b is OnInit is true.
The behavior I get in Dart differs from what I get in TypeScript (plunker example).
The TypeScript behavior is correct, the Dart behavior is wrong.

When I search the Angular2 code, I can't find any references to isSubtypeOf, which I would expect if this were to work correctly. I do however see what looks like a somewhat buggy use of superinterfaces (you can see that _interfacesFromMirror ignores the case when the given class has an interface I1 that implements an interface I2). Anyway, that doesn't yet fully explain the bug at hand. I can't follow the code that decides whether to call ngOnInit - it has something to do with ProtoRecord, which has no docs.

[kegluneq]

This is the intended behavior of Dart's compiled output - you must explicitly declare that B implements OnInit. I'll make sure this is surfaced in the documentation - sorry for the unexpected + undocumented behavior.

This was a design decision to keep the build process fast. Supporting this would mean that changes to an arbitrary file base.dart could affect the actions taken at compile time for class subclass.dart - we effectively need knowledge of the full application to make the work.

[kegluneq]

I got a recommendation to add a dev-mode-only check to throw if it the user has not added the requried implements On*, and I think this is a great idea.

Spun off #6849 to track.

[zoechi]

@kegluneq #4222 seems to tell the opposite

[kegluneq]

That's correct, #4222 will be reverted and I will add some better comments & documentation explaining why things work the way they do. https://goo.gl/b07Kii is one initial piece, and I'm in contact with the folks who are writing the Dart docs on angular.io to ensure that this behavior is well documented.

[zoechi]

@kegluneq that's real sad. That makes inheritance and mixins pretty much useless.

[kegluneq]

I agree that it's sad 😞

I disagree that it makes inheritance & mixins useless. They do require additional boilerplate to function the way you expect, and this does violate encapsulation, but they can still be used effectively.

[zoechi]

@kegluneq
If I only have to add implements SomeLifecycle then it isn't too bad.
It also depends how #6953 will turn out.

[Tomucha]

I just run into this one too (in Dart).

@kegluneq Please reconsider this:
This is the intended behavior of Dart's compiled output - you must explicitly declare that B implements OnInit. I'll make sure this is surfaced in the documentation - sorry for the unexpected + undocumented behavior.

I would say that your "intended bahavior" violates basic OOP contracts :-/ If "A extends B" and "B is OnInit" then "A is OnInit" too.

Real world example: Imagine backend/admin application, with global menu and big area for main content (Google Cloud Console, for example). I would create "BaseContentComponent" and all main screens would inherit from it: "MessagesContentComponent extends BaseContentComponent".

BaseContentCompoment declares what is "a content" - it has a headline, it has a sub-menu and probably it has some lifecycle callback, let's say BaseContentComponent implements OnInit to do some generic stuff.

Now I have to declare "MessagesContentComponent extends BaseContentComponent implements OnInit" ... I don't like it, but I can do it. Now let's say I create 50 content components like this. And then I decide to add new lifecycle callback to BaseContentComponent - for example OnDestroy I want to put each visited screen into "history/shortcuts" panel. Now I have to go through all 50 content components to add OnDestroy? Noooooooooooo ...

[kegluneq]

@Tomucha , I agree with you and am working toward a world where this isn't the case. I wrote a doc to explain the state of things right now and the direction we're heading in to solve this issue.

[Tomucha]

@kegluneq Great, thanks!

[pkozlowski-opensource]

Let's centralize discussion on inheritance in #11606. Closing as a duplicate.

[johnnytshi]

The master issue is closed

Is this now fixed for AOT? Or is there another issue tracking OnInit in case of AOT?

[yubaoquan]

I also run into this, with typescript 3.3.3. The ngOnInit of subclass won't get called if I omit a declaration of ngOnInit. So in every subclass I have to add implements OnInit and

ngOnInit() {
  super.ngOnInit();
}

This is pretty rediculous.
What's more, this bug doesn't appear in develop mode, but in production packed environment. I Think I should create another issue.

[NickStrupat]

is the minification to blame?

[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.}