-
Notifications
You must be signed in to change notification settings - Fork 30
Open
Description
I have 2 components which both extend the same base class. One of the 2 classes has a @ViewParent, when running the code in the metadata the @ViewParent is actually also required on the base class. I'm suspecting this is because of using an object reference where a copy should be used in addRequireToMetadata.
(I think addBindingToMetadata suffers the same bug, but haven't tested it yet)
Example:
// This component is required by the base class so the base class' `require` is stored as metadata
@Component({
selector: 'test-component',
})
export class TestComponent {
}
// Abstract class which has a `@ViewParent`, the require metadata will
// be stored and later polluted by `ComponentA`'s `@ViewParent`
export abstract class ChildClass {
@ViewParent('testComponent') private testComponentCtrl: TestComponent;
}
// When this class is registered, it will try to register the `ngModel` ViewParent
// but with the way `addRequireToMetadata` and `Reflect` works
// it will get the metadata object from `ChildClass` and register the require on there
@Component({
selector: 'component-a'
template: '<div>A</div>`
})
class ComponentA extends ChildClass {
@ViewParent('ngModel') private ngModelCtrl: ng.INgModelController;
}
// This component doesn't require `ngModel`, but because `ChildClass` is polluted it now also requires `ngModel`
@Component({
selector: 'component-b',
template: '<div>B</div>
})
class ComponentB extends ChildClass {
}
addRequireToMetadata code:
/** @internal */
function addRequireToMetadata(target, key, controller) {
var targetConstructor = target.constructor;
// `getMetadata` will call `OrdinaryHasMetadata` which in turn will call `OrdinaryGetPrototypeOf` returning the
// require of `ChildClass` instead of `ComponentA` because `ComponentA`'s prototype is `ChildClass`
var require = getMetadata(metadataKeys.require, targetConstructor) || {};
// Here the object reference of the require of `ChildClass` is now used instead
// of a copy, so ChildClass will now require `ngModel`
// What should happen is that if require already exists, a copy of
// the object should be made and used for writing
require[key] = controller;
// Metadata is stored correctly under `ComponentA`, but the `ChildClass` require
// object reference is already polluted
defineMetadata(metadataKeys.require, require, targetConstructor);
}
Proposed fix:
function addRequireToMetadata(target: any, key: string, controller: string) {
const targetConstructor = target.constructor;
const definedMetadata = getMetadata(metadataKeys.require, targetConstructor);
// Make a copy of the already stored metadata, this will fix the bug if it's actually the prototype's metadata
const require = definedMetadata !== undefined ? {...definedMetadata} : {};
require[key] = controller;
defineMetadata(metadataKeys.require, require, targetConstructor);
}
Metadata
Metadata
Assignees
Labels
No labels