From a80ac0a8d3238f483a05252c437d4b321c42f5c7 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Tue, 23 May 2017 10:52:40 -0700 Subject: [PATCH] fix(core): make decorators closure safe (#16905) This is required as e.g. `token` from `@Inject` is accessed in string form via makeParamDecorator but as a property in the `ReflectiveInjector`. Closes #16889 as this is a more general fix. --- .../compiler/test/directive_resolver_spec.ts | 34 +++++++++++--- packages/core/src/di/metadata.ts | 12 ++--- packages/core/src/di/reflective_provider.ts | 4 +- packages/core/src/metadata/di.ts | 45 ++++-------------- packages/core/src/metadata/directives.ts | 46 ++++--------------- packages/core/src/metadata/ng_module.ts | 12 +---- packages/core/src/util/decorators.ts | 26 ++++------- .../core/test/reflection/reflector_spec.ts | 7 +-- packages/core/test/util/decorators_spec.ts | 12 +++-- 9 files changed, 76 insertions(+), 122 deletions(-) diff --git a/packages/compiler/test/directive_resolver_spec.ts b/packages/compiler/test/directive_resolver_spec.ts index b80b65b79f..7322277350 100644 --- a/packages/compiler/test/directive_resolver_spec.ts +++ b/packages/compiler/test/directive_resolver_spec.ts @@ -114,9 +114,15 @@ export function main() { it('should read out the Directive metadata', () => { const directiveMetadata = resolver.resolve(SomeDirective); - expect(directiveMetadata) - .toEqual(new Directive( - {selector: 'someDirective', inputs: [], outputs: [], host: {}, queries: {}})); + expect(directiveMetadata).toEqual(new Directive({ + selector: 'someDirective', + inputs: [], + outputs: [], + host: {}, + queries: {}, + exportAs: undefined, + providers: undefined + })); }); it('should throw if not matching metadata is found', () => { @@ -136,11 +142,25 @@ export function main() { class ChildWithDecorator extends Parent { } - expect(resolver.resolve(ChildNoDecorator)) - .toEqual(new Directive({selector: 'p', inputs: [], outputs: [], host: {}, queries: {}})); + expect(resolver.resolve(ChildNoDecorator)).toEqual(new Directive({ + selector: 'p', + inputs: [], + outputs: [], + host: {}, + queries: {}, + exportAs: undefined, + providers: undefined + })); - expect(resolver.resolve(ChildWithDecorator)) - .toEqual(new Directive({selector: 'c', inputs: [], outputs: [], host: {}, queries: {}})); + expect(resolver.resolve(ChildWithDecorator)).toEqual(new Directive({ + selector: 'c', + inputs: [], + outputs: [], + host: {}, + queries: {}, + exportAs: undefined, + providers: undefined + })); }); describe('inputs', () => { diff --git a/packages/core/src/di/metadata.ts b/packages/core/src/di/metadata.ts index 8d4c3a238c..91c25540bd 100644 --- a/packages/core/src/di/metadata.ts +++ b/packages/core/src/di/metadata.ts @@ -58,7 +58,7 @@ export interface Inject { token: any; } * @stable * @Annotation */ -export const Inject: InjectDecorator = makeParamDecorator('Inject', [['token', undefined]]); +export const Inject: InjectDecorator = makeParamDecorator('Inject', (token: any) => ({token})); /** @@ -104,7 +104,7 @@ export interface Optional {} * @stable * @Annotation */ -export const Optional: OptionalDecorator = makeParamDecorator('Optional', []); +export const Optional: OptionalDecorator = makeParamDecorator('Optional'); /** * Type of the Injectable decorator / constructor function. @@ -151,7 +151,7 @@ export interface Injectable {} * @stable * @Annotation */ -export const Injectable: InjectableDecorator = makeDecorator('Injectable', []); +export const Injectable: InjectableDecorator = makeDecorator('Injectable'); /** * Type of the Self decorator / constructor function. @@ -195,7 +195,7 @@ export interface Self {} * @stable * @Annotation */ -export const Self: SelfDecorator = makeParamDecorator('Self', []); +export const Self: SelfDecorator = makeParamDecorator('Self'); /** @@ -240,7 +240,7 @@ export interface SkipSelf {} * @stable * @Annotation */ -export const SkipSelf: SkipSelfDecorator = makeParamDecorator('SkipSelf', []); +export const SkipSelf: SkipSelfDecorator = makeParamDecorator('SkipSelf'); /** * Type of the Host decorator / constructor function. @@ -285,4 +285,4 @@ export interface Host {} * @stable * @Annotation */ -export const Host: HostDecorator = makeParamDecorator('Host', []); +export const Host: HostDecorator = makeParamDecorator('Host'); diff --git a/packages/core/src/di/reflective_provider.ts b/packages/core/src/di/reflective_provider.ts index 96fd580112..1243d8c024 100644 --- a/packages/core/src/di/reflective_provider.ts +++ b/packages/core/src/di/reflective_provider.ts @@ -225,7 +225,7 @@ function _extractToken( if (!Array.isArray(metadata)) { if (metadata instanceof Inject) { - return _createDependency(metadata['token'], optional, null); + return _createDependency(metadata.token, optional, null); } else { return _createDependency(metadata, optional, null); } @@ -240,7 +240,7 @@ function _extractToken( token = paramMetadata; } else if (paramMetadata instanceof Inject) { - token = paramMetadata['token']; + token = paramMetadata.token; } else if (paramMetadata instanceof Optional) { optional = true; diff --git a/packages/core/src/metadata/di.ts b/packages/core/src/metadata/di.ts index 86c7fcac25..64df0bffce 100644 --- a/packages/core/src/metadata/di.ts +++ b/packages/core/src/metadata/di.ts @@ -119,7 +119,7 @@ export interface Attribute { attributeName?: string; } * @Annotation */ export const Attribute: AttributeDecorator = - makeParamDecorator('Attribute', [['attributeName', undefined]]); + makeParamDecorator('Attribute', (attributeName?: string) => ({attributeName})); /** * Type of the Query metadata. @@ -207,14 +207,8 @@ export type ContentChildren = Query; export const ContentChildren: ContentChildrenDecorator = makePropDecorator( 'ContentChildren', - [ - ['selector', undefined], { - first: false, - isViewQuery: false, - descendants: false, - read: undefined, - } - ], + (selector?: any, data: any = {}) => + ({selector, first: false, isViewQuery: false, descendants: false, ...data}), Query); /** @@ -273,15 +267,8 @@ export type ContentChild = Query; * @Annotation */ export const ContentChild: ContentChildDecorator = makePropDecorator( - 'ContentChild', - [ - ['selector', undefined], { - first: true, - isViewQuery: false, - descendants: true, - read: undefined, - } - ], + 'ContentChild', (selector?: any, data: any = {}) => + ({selector, first: true, isViewQuery: false, descendants: true, ...data}), Query); /** @@ -339,15 +326,8 @@ export type ViewChildren = Query; * @Annotation */ export const ViewChildren: ViewChildrenDecorator = makePropDecorator( - 'ViewChildren', - [ - ['selector', undefined], { - first: false, - isViewQuery: true, - descendants: true, - read: undefined, - } - ], + 'ViewChildren', (selector?: any, data: any = {}) => + ({selector, first: false, isViewQuery: true, descendants: true, ...data}), Query); /** @@ -403,13 +383,6 @@ export type ViewChild = Query; * @Annotation */ export const ViewChild: ViewChildDecorator = makePropDecorator( - 'ViewChild', - [ - ['selector', undefined], { - first: true, - isViewQuery: true, - descendants: true, - read: undefined, - } - ], + 'ViewChild', (selector: any, data: any) => + ({selector, first: true, isViewQuery: true, descendants: true, ...data}), Query); diff --git a/packages/core/src/metadata/directives.ts b/packages/core/src/metadata/directives.ts index 646f7be8ec..5d4d9b9fe6 100644 --- a/packages/core/src/metadata/directives.ts +++ b/packages/core/src/metadata/directives.ts @@ -399,15 +399,8 @@ export interface Directive { * @stable * @Annotation */ -export const Directive: DirectiveDecorator = makeDecorator('Directive', { - selector: undefined, - inputs: undefined, - outputs: undefined, - host: undefined, - providers: undefined, - exportAs: undefined, - queries: undefined -}); +export const Directive: DirectiveDecorator = + makeDecorator('Directive', (dir: Directive = {}) => dir); /** * Type of the Component decorator / constructor function. @@ -691,26 +684,7 @@ export interface Component extends Directive { * @Annotation */ export const Component: ComponentDecorator = makeDecorator( - 'Component', { - selector: undefined, - inputs: undefined, - outputs: undefined, - host: undefined, - exportAs: undefined, - moduleId: undefined, - providers: undefined, - viewProviders: undefined, - changeDetection: ChangeDetectionStrategy.Default, - queries: undefined, - templateUrl: undefined, - template: undefined, - styleUrls: undefined, - styles: undefined, - animations: undefined, - encapsulation: undefined, - interpolation: undefined, - entryComponents: undefined - }, + 'Component', (c: Component = {}) => ({changeDetection: ChangeDetectionStrategy.Default, ...c}), Directive); /** @@ -750,10 +724,8 @@ export interface Pipe { * @stable * @Annotation */ -export const Pipe: PipeDecorator = makeDecorator('Pipe', { - name: undefined, - pure: true, -}); +export const Pipe: PipeDecorator = + makeDecorator('Pipe', (p: Pipe) => ({pure: true, ...p})); /** @@ -825,7 +797,7 @@ export interface Input { * @Annotation */ export const Input: InputDecorator = - makePropDecorator('Input', [['bindingPropertyName', undefined]]); + makePropDecorator('Input', (bindingPropertyName?: string) => ({bindingPropertyName})); /** * Type of the Output decorator / constructor function. @@ -891,7 +863,7 @@ export interface Output { bindingPropertyName?: string; } * @Annotation */ export const Output: OutputDecorator = - makePropDecorator('Output', [['bindingPropertyName', undefined]]); + makePropDecorator('Output', (bindingPropertyName?: string) => ({bindingPropertyName})); /** @@ -951,7 +923,7 @@ export interface HostBinding { hostPropertyName?: string; } * @Annotation */ export const HostBinding: HostBindingDecorator = - makePropDecorator('HostBinding', [['hostPropertyName', undefined]]); + makePropDecorator('HostBinding', (hostPropertyName?: string) => ({hostPropertyName})); /** @@ -1013,4 +985,4 @@ export interface HostListener { * @Annotation */ export const HostListener: HostListenerDecorator = - makePropDecorator('HostListener', [['eventName', undefined], ['args', []]]); + makePropDecorator('HostListener', (eventName?: string, args?: string[]) => ({eventName, args})); diff --git a/packages/core/src/metadata/ng_module.ts b/packages/core/src/metadata/ng_module.ts index 1cad93d0d8..6dc53d6c37 100644 --- a/packages/core/src/metadata/ng_module.ts +++ b/packages/core/src/metadata/ng_module.ts @@ -190,13 +190,5 @@ export interface NgModule { * @stable * @Annotation */ -export const NgModule: NgModuleDecorator = makeDecorator('NgModule', { - providers: undefined, - declarations: undefined, - imports: undefined, - exports: undefined, - entryComponents: undefined, - bootstrap: undefined, - schemas: undefined, - id: undefined, -}); \ No newline at end of file +export const NgModule: NgModuleDecorator = + makeDecorator('NgModule', (ngModule: NgModule) => ngModule); \ No newline at end of file diff --git a/packages/core/src/util/decorators.ts b/packages/core/src/util/decorators.ts index e8a218605e..0ac074677f 100644 --- a/packages/core/src/util/decorators.ts +++ b/packages/core/src/util/decorators.ts @@ -262,9 +262,9 @@ export function Class(clsDef: ClassDefinition): Type { * @suppress {globalThis} */ export function makeDecorator( - name: string, props: {[name: string]: any}, parentClass?: any, + name: string, props?: (...args: any[]) => any, parentClass?: any, chainFn?: (fn: Function) => void): (...args: any[]) => (cls: any) => any { - const metaCtor = makeMetadataCtor([props]); + const metaCtor = makeMetadataCtor(props); function DecoratorFactory(objOrType: any): (cls: any) => any { if (!(Reflect && Reflect.getOwnMetadata)) { @@ -301,25 +301,19 @@ export function makeDecorator( return DecoratorFactory; } -function makeMetadataCtor(props: ([string, any] | {[key: string]: any})[]): any { +function makeMetadataCtor(props?: (...args: any[]) => any): any { return function ctor(...args: any[]) { - props.forEach((prop, i) => { - const argVal = args[i]; - if (Array.isArray(prop)) { - // plain parameter - this[prop[0]] = argVal === undefined ? prop[1] : argVal; - } else { - for (const propName in prop) { - this[propName] = - argVal && argVal.hasOwnProperty(propName) ? argVal[propName] : prop[propName]; - } + if (props) { + const values = props(...args); + for (const propName in values) { + this[propName] = values[propName]; } - }); + } }; } export function makeParamDecorator( - name: string, props: ([string, any] | {[name: string]: any})[], parentClass?: any): any { + name: string, props?: (...args: any[]) => any, parentClass?: any): any { const metaCtor = makeMetadataCtor(props); function ParamDecoratorFactory(...args: any[]): any { if (this instanceof ParamDecoratorFactory) { @@ -356,7 +350,7 @@ export function makeParamDecorator( } export function makePropDecorator( - name: string, props: ([string, any] | {[key: string]: any})[], parentClass?: any): any { + name: string, props?: (...args: any[]) => any, parentClass?: any): any { const metaCtor = makeMetadataCtor(props); function PropDecoratorFactory(...args: any[]): any { diff --git a/packages/core/test/reflection/reflector_spec.ts b/packages/core/test/reflection/reflector_spec.ts index 9ec32d4e44..7cd91633b8 100644 --- a/packages/core/test/reflection/reflector_spec.ts +++ b/packages/core/test/reflection/reflector_spec.ts @@ -29,10 +29,11 @@ interface PropDecorator { } /** @Annotation */ const ClassDecorator = - makeDecorator('ClassDecorator', {value: undefined}); + makeDecorator('ClassDecorator', (data: any) => data); /** @Annotation */ const ParamDecorator = - makeParamDecorator('ParamDecorator', [['value', undefined]]); -/** @Annotation */ const PropDecorator = makePropDecorator('PropDecorator', [['value', undefined]]); + makeParamDecorator('ParamDecorator', (value: any) => ({value})); +/** @Annotation */ const PropDecorator = + makePropDecorator('PropDecorator', (value: any) => ({value})); class AType { constructor(public value: any) {} diff --git a/packages/core/test/util/decorators_spec.ts b/packages/core/test/util/decorators_spec.ts index 3626022f14..34735e968f 100644 --- a/packages/core/test/util/decorators_spec.ts +++ b/packages/core/test/util/decorators_spec.ts @@ -17,14 +17,15 @@ class DecoratedChild extends DecoratedParent {} export function main() { const Reflect = global['Reflect']; - const TerminalDecorator = makeDecorator('TerminalDecorator', {terminal: true}); + const TerminalDecorator = + makeDecorator('TerminalDecorator', (data: any) => ({terminal: true, ...data})); const TestDecorator = makeDecorator( - 'TestDecorator', {marker: undefined}, Object, (fn: any) => fn.Terminal = TerminalDecorator); + 'TestDecorator', (data: any) => data, Object, (fn: any) => fn.Terminal = TerminalDecorator); describe('Property decorators', () => { // https://github.com/angular/angular/issues/12224 it('should work on the "watch" property', () => { - const Prop = makePropDecorator('Prop', [['value', undefined]]); + const Prop = makePropDecorator('Prop', (value: any) => ({value})); class TestClass { @Prop('firefox!') @@ -36,13 +37,14 @@ export function main() { }); it('should work with any default plain values', () => { - const Default = makePropDecorator('Default', [['value', 5]]); + const Default = + makePropDecorator('Default', (data: any) => ({value: data != null ? data : 5})); expect(new Default(0)['value']).toEqual(0); }); it('should work with any object values', () => { // make sure we don't walk up the prototype chain - const Default = makePropDecorator('Default', [{value: 5}]); + const Default = makePropDecorator('Default', (data: any) => ({value: 5, ...data})); const value = Object.create({value: 10}); expect(new Default(value)['value']).toEqual(5); });