ngParty / ng-metadata

Angular 2 decorators and utils for Angular 1.x
https://hotell.gitbooks.io/ng-metadata/content/
MIT License
355 stars 46 forks source link

Ref #203 - Inheritance issues #207

Open mtraynham opened 7 years ago

mtraynham commented 7 years ago

Please see #203 for the overall issue. I'll summarize changes here:

propMetadata

Previously class property metadata was not being inherited by child classes from parents. For example:

class A { @Input() public foo: any; }
class B extends A { @Input() public bar: any; }

If you requested propMetadata for class B, you would only get bar. The first commit of this overlays all parent properties. A simple Object.assign should suffice, because we should expect all parent keys to typically be different from the child keys.

class A { @Input() public foo: any; }
class B extends A { public foo: any; } // this is not really appropriate

parameters

Previously class constructor decorators were being initialized with their parent constructor parameters. Reflect.getMetadata will return the parent class stored item if the child does not yet exist in the cache. Because of this, the ParamDecorator would get the parent array reference for the first time seeing a child class. It would then manipulate the same reference, and set it back in Reflect with the child's class key.

After all decorates were processed, the Reflect cache would end up with the same constructor decorator array reference for a parent and all it's children, albeit it may be stored at different keys (AConstructor, BConstructor, CConstructor, etc...). This means that any class within a hierarchy has ultimately added to an ever growing single set of decorators. A child could end up with 5 or 6 different @Inject tokens on a single parameter.

After thinking about it for a while, I don't think initializing with the parent is correct. The constructor of a child could be expressed in a completely different layout from the parent.

class A { constructor (@Inject('form') form: any, @Inject('$http') $http: any) { .. } }
class B extends A { constructor ($http: any, form: any) { .. } }

It seems more appropriate to store the class by itself, and if the constructor was empty, it does access it's parents.

aciccarello commented 7 years ago

Nice PR @mtraynham :+1: . That metadata stuff is a bit complicated.