From 4fd13d71c8f4e6efbc8e3ce9d485721941f1bd3d Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 27 Sep 2016 15:41:37 -0700 Subject: [PATCH] refactor(DirectiveResolver): cleanup --- .../compiler/src/directive_resolver.ts | 112 +++++++++--------- .../compiler/test/directive_resolver_spec.ts | 30 ++--- 2 files changed, 71 insertions(+), 71 deletions(-) diff --git a/modules/@angular/compiler/src/directive_resolver.ts b/modules/@angular/compiler/src/directive_resolver.ts index f01a86f3a4..5b473c86a7 100644 --- a/modules/@angular/compiler/src/directive_resolver.ts +++ b/modules/@angular/compiler/src/directive_resolver.ts @@ -9,14 +9,10 @@ import {Component, Directive, HostBinding, HostListener, Injectable, Input, Output, Query, Type, resolveForwardRef} from '@angular/core'; import {StringMapWrapper} from './facade/collection'; -import {isPresent, stringify} from './facade/lang'; +import {stringify} from './facade/lang'; import {ReflectorReader, reflector} from './private_import_core'; import {splitAtColon} from './util'; -function _isDirectiveMetadata(type: any): type is Directive { - return type instanceof Directive; -} - /* * Resolve a `Type` for {@link Directive}. * @@ -32,54 +28,57 @@ export class DirectiveResolver { * Return {@link Directive} for a given `Type`. */ resolve(type: Type, throwIfNotFound = true): Directive { - var typeMetadata = this._reflector.annotations(resolveForwardRef(type)); - if (isPresent(typeMetadata)) { - var metadata = typeMetadata.find(_isDirectiveMetadata); - if (isPresent(metadata)) { - var propertyMetadata = this._reflector.propMetadata(type); + const typeMetadata = this._reflector.annotations(resolveForwardRef(type)); + if (typeMetadata) { + const metadata = typeMetadata.find(isDirectiveMetadata); + if (metadata) { + const propertyMetadata = this._reflector.propMetadata(type); return this._mergeWithPropertyMetadata(metadata, propertyMetadata, type); } } + if (throwIfNotFound) { throw new Error(`No Directive annotation found on ${stringify(type)}`); } + return null; } private _mergeWithPropertyMetadata( dm: Directive, propertyMetadata: {[key: string]: any[]}, directiveType: Type): Directive { - var inputs: string[] = []; - var outputs: string[] = []; - var host: {[key: string]: string} = {}; - var queries: {[key: string]: any} = {}; + const inputs: string[] = []; + const outputs: string[] = []; + const host: {[key: string]: string} = {}; + const queries: {[key: string]: any} = {}; - StringMapWrapper.forEach(propertyMetadata, (metadata: any[], propName: string) => { - metadata.forEach(a => { + Object.keys(propertyMetadata).forEach((propName: string) => { + + propertyMetadata[propName].forEach(a => { if (a instanceof Input) { - if (isPresent(a.bindingPropertyName)) { + if (a.bindingPropertyName) { inputs.push(`${propName}: ${a.bindingPropertyName}`); } else { inputs.push(propName); } } else if (a instanceof Output) { const output: Output = a; - if (isPresent(output.bindingPropertyName)) { + if (output.bindingPropertyName) { outputs.push(`${propName}: ${output.bindingPropertyName}`); } else { outputs.push(propName); } } else if (a instanceof HostBinding) { const hostBinding: HostBinding = a; - if (isPresent(hostBinding.hostPropertyName)) { + if (hostBinding.hostPropertyName) { host[`[${hostBinding.hostPropertyName}]`] = propName; } else { host[`[${propName}]`] = propName; } } else if (a instanceof HostListener) { const hostListener: HostListener = a; - var args = isPresent(hostListener.args) ? (hostListener.args).join(', ') : ''; - host[`(${hostListener.eventName})`] = `${propName}(${args})`; + const args = hostListener.args || []; + host[`(${hostListener.eventName})`] = `${propName}(${args.join(',')})`; } else if (a instanceof Query) { queries[propName] = a; } @@ -91,13 +90,14 @@ export class DirectiveResolver { private _extractPublicName(def: string) { return splitAtColon(def, [null, def])[1].trim(); } private _merge( - dm: Directive, inputs: string[], outputs: string[], host: {[key: string]: string}, + directive: Directive, inputs: string[], outputs: string[], host: {[key: string]: string}, queries: {[key: string]: any}, directiveType: Type): Directive { - let mergedInputs: string[]; + const mergedInputs: string[] = inputs; - if (isPresent(dm.inputs)) { + if (directive.inputs) { const inputNames: string[] = - dm.inputs.map((def: string): string => this._extractPublicName(def)); + directive.inputs.map((def: string): string => this._extractPublicName(def)); + inputs.forEach((inputDef: string) => { const publicName = this._extractPublicName(inputDef); if (inputNames.indexOf(publicName) > -1) { @@ -105,16 +105,15 @@ export class DirectiveResolver { `Input '${publicName}' defined multiple times in '${stringify(directiveType)}'`); } }); - mergedInputs = dm.inputs.concat(inputs); - } else { - mergedInputs = inputs; + + mergedInputs.unshift(...directive.inputs); } - let mergedOutputs: string[]; + let mergedOutputs: string[] = outputs; - if (isPresent(dm.outputs)) { + if (directive.outputs) { const outputNames: string[] = - dm.outputs.map((def: string): string => this._extractPublicName(def)); + directive.outputs.map((def: string): string => this._extractPublicName(def)); outputs.forEach((outputDef: string) => { const publicName = this._extractPublicName(outputDef); @@ -123,47 +122,48 @@ export class DirectiveResolver { `Output event '${publicName}' defined multiple times in '${stringify(directiveType)}'`); } }); - mergedOutputs = dm.outputs.concat(outputs); - } else { - mergedOutputs = outputs; + mergedOutputs.unshift(...directive.outputs); } - var mergedHost = isPresent(dm.host) ? StringMapWrapper.merge(dm.host, host) : host; - var mergedQueries = - isPresent(dm.queries) ? StringMapWrapper.merge(dm.queries, queries) : queries; + const mergedHost = directive.host ? StringMapWrapper.merge(directive.host, host) : host; + const mergedQueries = + directive.queries ? StringMapWrapper.merge(directive.queries, queries) : queries; - if (dm instanceof Component) { + if (directive instanceof Component) { return new Component({ - selector: dm.selector, + selector: directive.selector, inputs: mergedInputs, outputs: mergedOutputs, host: mergedHost, - exportAs: dm.exportAs, - moduleId: dm.moduleId, + exportAs: directive.exportAs, + moduleId: directive.moduleId, queries: mergedQueries, - changeDetection: dm.changeDetection, - providers: dm.providers, - viewProviders: dm.viewProviders, - entryComponents: dm.entryComponents, - template: dm.template, - templateUrl: dm.templateUrl, - styles: dm.styles, - styleUrls: dm.styleUrls, - encapsulation: dm.encapsulation, - animations: dm.animations, - interpolation: dm.interpolation + changeDetection: directive.changeDetection, + providers: directive.providers, + viewProviders: directive.viewProviders, + entryComponents: directive.entryComponents, + template: directive.template, + templateUrl: directive.templateUrl, + styles: directive.styles, + styleUrls: directive.styleUrls, + encapsulation: directive.encapsulation, + animations: directive.animations, + interpolation: directive.interpolation }); - } else { return new Directive({ - selector: dm.selector, + selector: directive.selector, inputs: mergedInputs, outputs: mergedOutputs, host: mergedHost, - exportAs: dm.exportAs, + exportAs: directive.exportAs, queries: mergedQueries, - providers: dm.providers + providers: directive.providers }); } } } + +function isDirectiveMetadata(type: any): type is Directive { + return type instanceof Directive; +} diff --git a/modules/@angular/compiler/test/directive_resolver_spec.ts b/modules/@angular/compiler/test/directive_resolver_spec.ts index 28b4552435..d139eaf841 100644 --- a/modules/@angular/compiler/test/directive_resolver_spec.ts +++ b/modules/@angular/compiler/test/directive_resolver_spec.ts @@ -112,12 +112,12 @@ class SomeDirectiveWithoutMetadata {} export function main() { describe('DirectiveResolver', () => { - var resolver: DirectiveResolver; + let resolver: DirectiveResolver; beforeEach(() => { resolver = new DirectiveResolver(); }); it('should read out the Directive metadata', () => { - var directiveMetadata = resolver.resolve(SomeDirective); + const directiveMetadata = resolver.resolve(SomeDirective); expect(directiveMetadata) .toEqual(new Directive( {selector: 'someDirective', inputs: [], outputs: [], host: {}, queries: {}})); @@ -130,7 +130,7 @@ export function main() { }); it('should not read parent class Directive metadata', function() { - var directiveMetadata = resolver.resolve(SomeChildDirective); + const directiveMetadata = resolver.resolve(SomeChildDirective); expect(directiveMetadata) .toEqual(new Directive( {selector: 'someChildDirective', inputs: [], outputs: [], host: {}, queries: {}})); @@ -138,12 +138,12 @@ export function main() { describe('inputs', () => { it('should append directive inputs', () => { - var directiveMetadata = resolver.resolve(SomeDirectiveWithInputs); + const directiveMetadata = resolver.resolve(SomeDirectiveWithInputs); expect(directiveMetadata.inputs).toEqual(['c', 'a', 'b: renamed']); }); it('should work with getters and setters', () => { - var directiveMetadata = resolver.resolve(SomeDirectiveWithSetterProps); + const directiveMetadata = resolver.resolve(SomeDirectiveWithSetterProps); expect(directiveMetadata.inputs).toEqual(['a: renamed']); }); @@ -162,12 +162,12 @@ export function main() { describe('outputs', () => { it('should append directive outputs', () => { - var directiveMetadata = resolver.resolve(SomeDirectiveWithOutputs); + const directiveMetadata = resolver.resolve(SomeDirectiveWithOutputs); expect(directiveMetadata.outputs).toEqual(['c', 'a', 'b: renamed']); }); it('should work with getters and setters', () => { - var directiveMetadata = resolver.resolve(SomeDirectiveWithGetterOutputs); + const directiveMetadata = resolver.resolve(SomeDirectiveWithGetterOutputs); expect(directiveMetadata.outputs).toEqual(['a: renamed']); }); @@ -186,12 +186,12 @@ export function main() { describe('host', () => { it('should append host bindings', () => { - var directiveMetadata = resolver.resolve(SomeDirectiveWithHostBindings); + const directiveMetadata = resolver.resolve(SomeDirectiveWithHostBindings); expect(directiveMetadata.host).toEqual({'[c]': 'c', '[a]': 'a', '[renamed]': 'b'}); }); it('should append host listeners', () => { - var directiveMetadata = resolver.resolve(SomeDirectiveWithHostListeners); + const directiveMetadata = resolver.resolve(SomeDirectiveWithHostListeners); expect(directiveMetadata.host) .toEqual({'(c)': 'onC()', '(a)': 'onA()', '(b)': 'onB($event.value)'}); }); @@ -199,33 +199,33 @@ export function main() { describe('queries', () => { it('should append ContentChildren', () => { - var directiveMetadata = resolver.resolve(SomeDirectiveWithContentChildren); + const directiveMetadata = resolver.resolve(SomeDirectiveWithContentChildren); expect(directiveMetadata.queries) .toEqual({'cs': new ContentChildren('c'), 'as': new ContentChildren('a')}); }); it('should append ViewChildren', () => { - var directiveMetadata = resolver.resolve(SomeDirectiveWithViewChildren); + const directiveMetadata = resolver.resolve(SomeDirectiveWithViewChildren); expect(directiveMetadata.queries) .toEqual({'cs': new ViewChildren('c'), 'as': new ViewChildren('a')}); }); it('should append ContentChild', () => { - var directiveMetadata = resolver.resolve(SomeDirectiveWithContentChild); + const directiveMetadata = resolver.resolve(SomeDirectiveWithContentChild); expect(directiveMetadata.queries) .toEqual({'c': new ContentChild('c'), 'a': new ContentChild('a')}); }); it('should append ViewChild', () => { - var directiveMetadata = resolver.resolve(SomeDirectiveWithViewChild); + const directiveMetadata = resolver.resolve(SomeDirectiveWithViewChild); expect(directiveMetadata.queries) .toEqual({'c': new ViewChild('c'), 'a': new ViewChild('a')}); }); }); - describe('view', () => { + describe('Component', () => { it('should read out the template related metadata from the Component metadata', () => { - var compMetadata = resolver.resolve(ComponentWithTemplate); + const compMetadata: Component = resolver.resolve(ComponentWithTemplate); expect(compMetadata.template).toEqual('some template'); expect(compMetadata.styles).toEqual(['some styles']); });