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

fix(core): make decorators closure safe #16905

Merged
merged 1 commit into from May 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 28 additions & 8 deletions packages/compiler/test/directive_resolver_spec.ts
Expand Up @@ -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', () => {
Expand All @@ -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(ChildWithDecorator))
.toEqual(new Directive({selector: 'c', 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: {},
exportAs: undefined,
providers: undefined
}));
});

describe('inputs', () => {
Expand Down
12 changes: 6 additions & 6 deletions packages/core/src/di/metadata.ts
Expand Up @@ -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}));


/**
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -151,7 +151,7 @@ export interface Injectable {}
* @stable
* @Annotation
*/
export const Injectable: InjectableDecorator = <InjectableDecorator>makeDecorator('Injectable', []);
export const Injectable: InjectableDecorator = <InjectableDecorator>makeDecorator('Injectable');

/**
* Type of the Self decorator / constructor function.
Expand Down Expand Up @@ -195,7 +195,7 @@ export interface Self {}
* @stable
* @Annotation
*/
export const Self: SelfDecorator = makeParamDecorator('Self', []);
export const Self: SelfDecorator = makeParamDecorator('Self');


/**
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -285,4 +285,4 @@ export interface Host {}
* @stable
* @Annotation
*/
export const Host: HostDecorator = makeParamDecorator('Host', []);
export const Host: HostDecorator = makeParamDecorator('Host');
4 changes: 2 additions & 2 deletions packages/core/src/di/reflective_provider.ts
Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down
45 changes: 9 additions & 36 deletions packages/core/src/metadata/di.ts
Expand Up @@ -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.
Expand Down Expand Up @@ -207,14 +207,8 @@ export type ContentChildren = Query;
export const ContentChildren: ContentChildrenDecorator =
<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);

/**
Expand Down Expand Up @@ -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);

/**
Expand Down Expand Up @@ -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);

/**
Expand Down Expand Up @@ -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);
46 changes: 9 additions & 37 deletions packages/core/src/metadata/directives.ts
Expand Up @@ -399,15 +399,8 @@ export interface Directive {
* @stable
* @Annotation
*/
export const Directive: DirectiveDecorator = <DirectiveDecorator>makeDecorator('Directive', {
selector: undefined,
inputs: undefined,
outputs: undefined,
host: undefined,
providers: undefined,
exportAs: undefined,
queries: undefined
});
export const Directive: DirectiveDecorator =
<DirectiveDecorator>makeDecorator('Directive', (dir: Directive = {}) => dir);

/**
* Type of the Component decorator / constructor function.
Expand Down Expand Up @@ -691,26 +684,7 @@ export interface Component extends Directive {
* @Annotation
*/
export const Component: ComponentDecorator = <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);

/**
Expand Down Expand Up @@ -750,10 +724,8 @@ export interface Pipe {
* @stable
* @Annotation
*/
export const Pipe: PipeDecorator = <PipeDecorator>makeDecorator('Pipe', {
name: undefined,
pure: true,
});
export const Pipe: PipeDecorator =
<PipeDecorator>makeDecorator('Pipe', (p: Pipe) => ({pure: true, ...p}));


/**
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -891,7 +863,7 @@ export interface Output { bindingPropertyName?: string; }
* @Annotation
*/
export const Output: OutputDecorator =
makePropDecorator('Output', [['bindingPropertyName', undefined]]);
makePropDecorator('Output', (bindingPropertyName?: string) => ({bindingPropertyName}));


/**
Expand Down Expand Up @@ -951,7 +923,7 @@ export interface HostBinding { hostPropertyName?: string; }
* @Annotation
*/
export const HostBinding: HostBindingDecorator =
makePropDecorator('HostBinding', [['hostPropertyName', undefined]]);
makePropDecorator('HostBinding', (hostPropertyName?: string) => ({hostPropertyName}));


/**
Expand Down Expand Up @@ -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}));
12 changes: 2 additions & 10 deletions packages/core/src/metadata/ng_module.ts
Expand Up @@ -190,13 +190,5 @@ export interface NgModule {
* @stable
* @Annotation
*/
export const NgModule: NgModuleDecorator = <NgModuleDecorator>makeDecorator('NgModule', {
providers: undefined,
declarations: undefined,
imports: undefined,
exports: undefined,
entryComponents: undefined,
bootstrap: undefined,
schemas: undefined,
id: undefined,
});
export const NgModule: NgModuleDecorator =
<NgModuleDecorator>makeDecorator('NgModule', (ngModule: NgModule) => ngModule);
26 changes: 10 additions & 16 deletions packages/core/src/util/decorators.ts
Expand Up @@ -262,9 +262,9 @@ export function Class(clsDef: ClassDefinition): Type<any> {
* @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)) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 4 additions & 3 deletions packages/core/test/reflection/reflector_spec.ts
Expand Up @@ -29,10 +29,11 @@ interface PropDecorator {
}

/** @Annotation */ const ClassDecorator =
<ClassDecoratorFactory>makeDecorator('ClassDecorator', {value: undefined});
<ClassDecoratorFactory>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) {}
Expand Down