refactor(DirectiveResolver): cleanup

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<any>, throwIfNotFound = true): Directive {
-    var typeMetadata = this._reflector.annotations(resolveForwardRef(type));
+    const typeMetadata = this._reflector.annotations(resolveForwardRef(type));
-    if (isPresent(typeMetadata)) {
+    if (typeMetadata) {
-      var metadata = typeMetadata.find(_isDirectiveMetadata);
+      const metadata = typeMetadata.find(isDirectiveMetadata);
-      if (isPresent(metadata)) {
+      if (metadata) {
-        var propertyMetadata = this._reflector.propMetadata(type);
+        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<any>): Directive {
-    var inputs: string[] = [];
+    const inputs: string[] = [];
-    var outputs: string[] = [];
+    const outputs: string[] = [];
-    var host: {[key: string]: string} = {};
+    const host: {[key: string]: string} = {};
-    var queries: {[key: string]: any} = {};
+    const queries: {[key: string]: any} = {};
 
-    StringMapWrapper.forEach(propertyMetadata, (metadata: any[], propName: string) => {
+    Object.keys(propertyMetadata).forEach((propName: string) => {
-      metadata.forEach(a => {
+
+      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) ? (<any[]>hostListener.args).join(', ') : '';
+          const args = hostListener.args || [];
-          host[`(${hostListener.eventName})`] = `${propName}(${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<any>): 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.unshift(...directive.inputs);
-      mergedInputs = 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);
+      mergedOutputs.unshift(...directive.outputs);
-    } else {
-      mergedOutputs = outputs;
     }
 
-    var mergedHost = isPresent(dm.host) ? StringMapWrapper.merge(dm.host, host) : host;
+    const mergedHost = directive.host ? StringMapWrapper.merge(directive.host, host) : host;
-    var mergedQueries =
+    const mergedQueries =
-        isPresent(dm.queries) ? StringMapWrapper.merge(dm.queries, queries) : queries;
+        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,
+        exportAs: directive.exportAs,
-        moduleId: dm.moduleId,
+        moduleId: directive.moduleId,
         queries: mergedQueries,
-        changeDetection: dm.changeDetection,
+        changeDetection: directive.changeDetection,
-        providers: dm.providers,
+        providers: directive.providers,
-        viewProviders: dm.viewProviders,
+        viewProviders: directive.viewProviders,
-        entryComponents: dm.entryComponents,
+        entryComponents: directive.entryComponents,
-        template: dm.template,
+        template: directive.template,
-        templateUrl: dm.templateUrl,
+        templateUrl: directive.templateUrl,
-        styles: dm.styles,
+        styles: directive.styles,
-        styleUrls: dm.styleUrls,
+        styleUrls: directive.styleUrls,
-        encapsulation: dm.encapsulation,
+        encapsulation: directive.encapsulation,
-        animations: dm.animations,
+        animations: directive.animations,
-        interpolation: dm.interpolation
+        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;
+}

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 = <Component>resolver.resolve(ComponentWithTemplate);
+        const compMetadata: Component = resolver.resolve(ComponentWithTemplate);
         expect(compMetadata.template).toEqual('some template');
         expect(compMetadata.styles).toEqual(['some styles']);
       });