angular: Inheritance for Angular decorators is not well-defined

We will implement the following semantics:

1. class decorators are inherited, but never merged from parent into child class.
2. ctor parameters and decorators are inherited if child class does not define an own ctor
3. we inherit decorators that are defined on a parent method / property into the child class if the child class does not redefine this method / property
4. we inherit lifecycle methods

I.e. we don’t define a merge operation on the resulting metadata, but rather inheritance for properties / methods. This mirrors the inheritance of properties / methods themselves.

Reason: Introducing merging might break users. The semantics outlined here (especially the one for class decorators) match the way we always explained it to our users.

/cc @mhevery

@tycho01 yes, we are planning on supporting lamdas in Ng 4.0…

I found a few that seem related:

• #5415
• #5794
• #6781
• #6953
• #7191
• #8333
• #8700
• #10461
• #11060
• #11151
• #12063
• #8694 @Pipe inheritance - added by @vicb

@tbosch Is there any way to merge providers of a component superclass and subclass without having to explicitly declare both in the subclass constructor? I don’t want my subclass to have to know about the services that its superclass needs but can’t think how to accomplish this so long as providers are listed in the constructor of the component.

For example, given

export class ComponentSuperclass {
    constructor(private serviceA: ServiceA, private serviceB: ServiceB) {}
}

I would like to be able to write

export class ComponentSubclass extends ComponentSuperclass {
    constructor(private serviceC: ServiceC) {}
}

because my subclass should not have to know about what its parent is doing with ServiceA and ServiceB. But currently I must instead write

export class ComponentSubclass extends ComponentSuperclass {
    constructor(serviceA: ServiceA, serviceB: ServiceB, private serviceC: ServiceC) {
        super(serviceA, serviceB);
    }
}

To me this violates the open/closed principle.

Yes, this will work with Jit and AoT. The exact semantics are as follows:

Inheritance Semantics:

Decorators:

1. list the decorators of the class and its parents in the ancestor first order
2. only use the last decorator of each kind (e.g. @Component / …)

Constructor parameters: If a class inherits from a parent class and does not declare a constructor, it inherits the parent class constructor, and with it the parameter metadata of that parent class.

Lifecycle hooks: Follow the normal class inheritance model, i.e. lifecycle hooks of parent classes will be called even if the method is not overwritten in the child class.

Example

E.g. the following is a valid use of inheritance and it will also inherit all metadata:

@Directive({selector: 'someDir'})
class ParentDirective {
  constructor(someDep: SomeDep) {}

  ngOnInit() {}
}

class ChildDirective extends ParentDirective {}

@aluanhaddad But you can’t do that kind of refactoring with components, right?

To me, the inheritance-discussion revolves around three different intentions:

1. mixin You have a common behaviour/functionality, that you want to share between many components and thus attach it to them. I think this is what Angular directives are for. Unfortunately, this approach falls flat, because there is no way yet for a component to declare this dependency. See #8785.

2. decorator You want to enhance or constraint a single or multiple components by providing a new wrapper component. I believe this is the closest to delegation. Unfortunately, here is a problem, too, because passing ng-content to the wrapped component doesn’t work. See #8937.

3. subtype You want to modify or override the behaviour of a single existing component. One approach would be to derive from the component and face the problems mentioned in this thread, Another approach would be for the base component to provide events or inject a delegation service (provided ba a decorator). Either way, the base component would need to be designed so that it provides hooks for overriding functionality.

I don’t know if this makes sense, since I’m not a designer. But I would love if the Angular-documentation would add a topic about “DRY-approaches” in Angular so there is some guidance to supported strategies. Thanks for reading.

Use-Case: Template switching by selector

class ItemView {
  @Input() item;

  doSomething() {}
}

@Component({
  selector: 'item-view[type=grid]', templateUrl: '...' 
})
class GridItemView extends ItemView {}

@Component({
  selector: 'item-view[type=list]', templateUrl: '...' 
})
class ListItemView extends ItemView {}

@aluanhaddad no problem. And yes, we are working hard on making our AOT story better (watch mode, better errors, …)

Lifecycle methods from superclasses still not called in AOT compiled code (tested with 4.3.6 and 4.1.1), I found https://github.com/angular/angular/issues/12922 but it was closed and redirected here, not sure if you need another bug for this issue.

@phil123456 why do you expect updates on closed issues? There won’t be any updates. Inheritance is well supported since quite some time.

@aluanhaddad:

I’ll admit I’ve been having my share of continued frustration with AOT as well; was gonna use FP to generate pipes from an object of transform functions, but lambdas in AOT are a no-no.

Just checked yours out, guess we had a different approach in drying pipes at least, but pretty cool!

Definitely wish AOT wouldn’t mess with everything though… To me this whole thing about manually rewriting things to manually export lambdas seems pretty unfortunate/silly, it feels like something a machine could do as well as a human…

Thanks @zoechi !

This doesn’t solve the issue that @Directive annotation is completely ignored in derived class as mentioned in https://github.com/angular/angular/issues/12920

@laco0416 concrete example? I can’t imagine @Inject() on parameters on superclasses to be taken into account when injecting subclasses. The parameters need to be passed from the subclass using the super(...) call and this way decorators are meaningless.

any update on this ?

@dmitrysteblyuk I guess you should create a new issue.

@aluanhaddad: no credit due here, I pretty much just forked that one. 😛

@RodrigoQuesadaDev

I use more clever solutions when the DI framework/language allows me to; like when using Dagger/Kotlin or Spring/Java, which allow for method and field injection besides construction-time injection

😃 pls read this http://olivergierke.de/2013/11/why-field-injection-is-evil/

Thank you so much!

Note that it’s possible to work around this limitation at runtime. Good luck getting AoT compilation to work with inheritance.

I feel a bit cheated as TS is all about classes and inheritance. On top of that the language seems to get in the way of tooling. For example, ngc has difficult time generating code and requires a lot of monkey patching to work, e.g. changing the accessibility of props to public.

Incidentally, I think that language-level support of partial classes can help with the scenarios ngc is aiming at.

@alxhub In your code sample, there is no inheritance between A and B, didn’t you forget extends A in B definition ?

Complementing the related issues list of @zoechi:

• #7968
• #11144

@aluanhaddad No problem mate, i invented it myself, now i do not have to re copy code into all components that share the same template and i dont have to use the api to build it ether.

@tycho01 That is not what I mean, it has nothing to do with EventEmitter sorry for any confusion I mean delegation in a more general sense.

I mean that if you have a class hierarchy such that class D extends class B, but you want D to be unaware of B’s dependencies, you should refactor your code such that D no longer extends B but rather delegates to a B instance. This instance will be injected into D and D will delegate to it.

Here is a concrete example.

b.ts

import { Dep1 } from './dep1';
import { Dep2 } from './dep2';
import { Dep3 } from './dep3';
import { Dep4 } from './dep4';

@Injectable() export default class B {

  constructor(
    readonly dep1: Dep1, 
    readonly dep2: Dep2, 
    readonly dep3: Dep3, 
    readonly dep4: Dep4
  ) { }

  getSomething() { return ... }
}

d.ts Before refactoring

import B './b';
import { Dep1 } from './dep1';
import { Dep2 } from './dep2';
import { Dep3 } from './dep3';
import { Dep4 } from './dep4';
import { Dep5 } from './dep5';

@Injectable() export default class D extends B {

  constructor(dep1: Dep1, dep2: Dep2, dep3: Dep3, dep4: Dep4, readonly dep5: Dep5) {
    super(dep1, dep2, dep3, dep4);
  }

  getSomethingElse() { ... }
}

d.ts After refactoring

import B './b';
import { Dep5 } from './dep5';

@Injectable() export default class D {

  constructor(readonly b: B, readonly dep5: Dep5) { }

  getSomething() {
    return this.b.getSomething();
  }

  getSomethingElse() { ... }
}

@ghetolay: let me try to see if I’m following you correctly, because I think I’m coming to different conclusions from yours.

[I]t is easier to merge on an override strategy than it is to override on a merge strategy. You can’t really override a prop on a merge strategy, you’ll have to redefine all the props (the whole @Component) while if you want to merge on an override strategy you just need to redefine the property you want to merge (for example styles: ).

• PR behavior: override. If the parent’s decorator was exactly what you needed, you’re good. If you want any property different, you’ll have to redo them. You say “property you want to merge (for example styles: )”. I suppose that’s right, though it assumes you’ve exported the parent’s decorator properties before using them there; Angular won’t otherwise expose them to you to reuse (be it in adjusted form).

• Alternative behavior: naive ‘merge’. This is essentially an override on a per-property basis. If you wanted the same as the parent’s decorator properties, you’re good. If you wanted to change a property, you’re good, just override it. If you wanted to remove one, override it with null. If you wanted to override an array, go ahead. If you wanted to add to an array, you have a problem, since unless you’ve manually exported the original for reuse, you won’t have it available. AFAICT, this approach does not yet bring much controversy so far.

• My current behavior: merge with array concat / object merge. For scalar properties, identical to the above. For arrays/objects, this facilitates adding at the expense of an easy way to remove array items.

Let’s go over the properties for a bit here:

export interface Directive {
  selector?: string;
  inputs?: string[];
  outputs?: string[];
  host?: {[key: string]: string};
  providers?: Provider[];
  exportAs?: string;
  queries?: {[key: string]: any};
}

export interface Component extends Directive {
  changeDetection?: ChangeDetectionStrategy;
  viewProviders?: Provider[];
  moduleId?: string;
  templateUrl?: string;
  template?: string;
  styleUrls?: string[];
  styles?: string[];
  animations?: AnimationEntryMetadata[];
  encapsulation?: ViewEncapsulation;
  interpolation?: [string, string];
  entryComponents?: Array<Type<any>|any[]>;
}

export interface Pipe {
  name: string;
  pure?: boolean;
}

export interface Input {
  bindingPropertyName?: string;
}

export interface Output { bindingPropertyName?: string; }

export interface HostBinding { hostPropertyName?: string; }

With that, the latter proposal becomes largely about whether the following properties would be desirable enough to more easily add to even at the potential expense of not being able to override existing items easily (mostly applies to arrays):

• Directive:
  • arrays: inputs, outputs, providers
  • objects: host, queries
• Component: also these (arrays): viewProviders, styleUrls, styles, animations, entryComponents

My interpretation here: having ‘too much’ of these could likely not hurt except potentially in the case of styles, in which case on could technically still override them at the style level, though it’d beg the question why you’d have the parent pass on a property you’d deemed not worth inheriting. I’d be inclined to consider the ability to easily add to these as out-weighing that.

That being said, I don’t feel that strongly about this further step concerning arrays/objects. I think the bigger gain to be had here is the less controversial step of allowing to override on the property basis rather than just on the level of the entire decorator.

With current approach it might be reasonable to split @Component and @Directive into set of decorators: @Style, @Template, @Selector etc. to add granularity to inheritance.