Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Component Class Inheritance & Decorator bugs #203

Open
mtraynham opened this issue Jun 22, 2017 · 1 comment
Open

Component Class Inheritance & Decorator bugs #203

mtraynham opened this issue Jun 22, 2017 · 1 comment

Comments

@mtraynham
Copy link

mtraynham commented Jun 22, 2017

As a preamble on how I came across these bugs, I have a form that uses a variety of components, from string inputs to selects, etc. Some of the components may be more custom than others, but they all share a set of input/output bindings and may share some @Host components.

I tried this with both core-js and reflect-metadata with the same results.

The two issues I found with Plunkr and examples on how to recreate:


@Input (and potentially @Output) decorators are not inherited if the child component class specifies it's own @Input.

The code below has:

  1. an AbstractComponent with @Input a,
  2. a child class AComponent which correctly inherits that @Input a
  3. a child class BComponent which creates @Input b, and does not inherit @Input a

The expected output of this code should be a string, a a b. The actual output is just a b

Here is this bugs Plunkr example (all code is in app.module.ts).

import {Component, Input, NgModule} from 'ng-metadata/core';
import {platformBrowserDynamic} from 'ng-metadata/platform-browser-dynamic';

abstract class AbstractComponent {
    @Input('@') public a: string;
}

@Component({
    selector: 'aComponent',
    template: '<span>{{$ctrl.a}}</span>'
})
export class AComponent extends AbstractComponent  { }

@Component({
    selector: 'bComponent',
    template: '<span>{{$ctrl.a}} {{$ctrl.b}}</span>'
})
export class BComponent extends AbstractComponent  {
    @Input('@') public b: string;
}

@Component({
    selector: 'app',
    template: `
        <a-component a="a"></a-component>
        <b-component a="a" b="b"></b-component>
    `
})
export class AppComponent { }

@NgModule({
    declarations: [AComponent, BComponent, AppComponent],
    exports: [AppComponent],
    bootstrap: [AppComponent]
})
export default class AppModule { }

platformBrowserDynamic().bootstrapModule(AppModule);

@Host (and potentially @Self/@SkipSelf) constructor decorators incorrectly get applied to the parent class, adding parameter information to all child classes.

The code below has:

  1. an AbstractComponent with constructor @Inject('form') @Host of an IFormController,
  2. a child class AComponent which also @Inject('$http'), the IHttpService
  3. a child class BComponent which also @Inject('app') @Host the parent AppComponent.

I believe the BComponent's @Host on the second parameter is getting applied to every second parameter of any child of AbstractComponent and thus the $http services becomes a required directive.

Here is this bugs Plunkr example (all code is in app.module.ts).

This actually breaks Angular with an error logged as:

VM71 angular.js:14525 Error: [$compile:ctreq] Controller '$http', required by directive 'aComponent', can't be found!
http://errors.angularjs.org/1.6.4/$compile/ctreq?p0=%24http&p1=aComponent
    at eval (VM71 angular.js:66)
    at getControllers (VM71 angular.js:9896)
    at getControllers (VM71 angular.js:9903)
    at nodeLinkFn (VM71 angular.js:9796)
    at compositeLinkFn (VM71 angular.js:9055)
    at nodeLinkFn (VM71 angular.js:9809)
    at compositeLinkFn (VM71 angular.js:9055)
    at nodeLinkFn (VM71 angular.js:9809)
    at compositeLinkFn (VM71 angular.js:9055)
    at nodeLinkFn (VM71 angular.js:9809)
import {IFormController, IHttpService} from 'angular';
import {Component, Host, Inject, NgModule} from 'ng-metadata/core';
import {platformBrowserDynamic} from 'ng-metadata/platform-browser-dynamic';

abstract class AbstractComponent {
    private formController: IFormController;

    constructor (@Inject('form') @Host() formController: IFormController) {
        this.formController = formController;
    }
}

@Component({
    selector: 'aComponent'
})
export class AComponent extends AbstractComponent  {
    private $http: IHttpService;

    constructor (@Inject('form') @Host() formController: IFormController,
                 @Inject('$http') $http: IHttpService) {
        super(formController);
        this.$http = $http;
    }
}

@Component({
    selector: 'bComponent'
})
export class BComponent extends AbstractComponent  {
    private app: AppComponent;

    constructor (@Inject('form') @Host() formController: IFormController,
                 @Inject('app') @Host() app: AppComponent) {
        super(formController);
        this.app = app;
    }
}

@Component({
    selector: 'app',
    template: `
        <form>
            <a-component></a-component>
            <b-component></b-component>
        </form>
    `
})
export class AppComponent { }

@NgModule({
    declarations: [AComponent, BComponent, AppComponent],
    exports: [AppComponent],
    bootstrap: [AppComponent]
})
export default class AppModule { }

platformBrowserDynamic().bootstrapModule(AppModule);
@mtraynham
Copy link
Author

mtraynham commented Jul 17, 2017

I've been looking more at this. Angular 2 had an issue and relevant commit for this as well, which seems like it makes no upstream changes to either reflect library.

For propMetadata, they are checking if the class extends another class and merging all property metadata fields here.

For parameters, It looks like they are collecting the annotations themselves for only the child class and seperately for the parent here, deduping as necessary.

Here's a simplified version of debugging this...

import 'core-js/es7/reflect';
import {reflector} from 'ng-metadata/src/core/reflection/reflection';
import {Input} from 'ng-metadata/src/core/directives/decorators';
import {Host, Inject} from 'ng-metadata/src/core/di/decorators';

abstract class AbstractPropertyComponent {
    @Input() public a: string;
}

export class APropertyComponent extends AbstractPropertyComponent  {
    @Input() public b: string;
}

const aPropMetadata: {[p: string]: any[]} = reflector.propMetadata(APropertyComponent);
console.log(aPropMetadata);

abstract class AbstractConstructorComponent {
    private form: any;
    constructor (@Inject('form') @Host() form: any) {
        this.form = form;
    }
}

class AConstructorComponent extends AbstractConstructorComponent {
    private $http: any;
    constructor (@Inject('form') @Host() form: any,
                 @Inject('$http') $http: any) {
        super(form);
        this.$http = $http;
    }
}

class BConstructorComponent extends AbstractConstructorComponent {
    private app: any;
    constructor (@Inject('form') @Host() form: any,
                 @Inject('app') @Host() app: any) {
        super(form);
        this.app = app;
    }
}

const aConstructorAnnotations: any[][] = reflector.parameters(AConstructorComponent);
console.log(aConstructorAnnotations);
const bConstructorAnnotations: any[][] = reflector.parameters(BConstructorComponent);
console.log(bConstructorAnnotations);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant