From 5926d2e2f7c89dbb2f1222ed08235891d96fa084 Mon Sep 17 00:00:00 2001
From: Pawel Kozlowski <pkozlowski.opensource@gmail.com>
Date: Thu, 12 Mar 2015 13:19:43 +0100
Subject: [PATCH] refactor: use attributes for directives matching

Closes #940
---
 .../compiler/pipeline/directive_parser.js     | 23 ++++--------
 .../pipeline/property_binding_parser.js       | 13 +++++--
 .../core/compiler/pipeline/view_splitter.js   |  2 ++
 .../test/core/compiler/integration_spec.js    | 35 +++++++++++++++++++
 .../pipeline/directive_parser_spec.js         |  8 +++--
 5 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/modules/angular2/src/core/compiler/pipeline/directive_parser.js b/modules/angular2/src/core/compiler/pipeline/directive_parser.js
index cf093dee1d..6117eae2e8 100644
--- a/modules/angular2/src/core/compiler/pipeline/directive_parser.js
+++ b/modules/angular2/src/core/compiler/pipeline/directive_parser.js
@@ -52,27 +52,16 @@ export class DirectiveParser extends CompileStep {
     for (var i=0; i < classList.length; i++) {
       cssSelector.addClassName(classList[i]);
     }
+
     MapWrapper.forEach(attrs, (attrValue, attrName) => {
-      if (isBlank(current.propertyBindings) ||
-        isPresent(current.propertyBindings) && !MapWrapper.contains(current.propertyBindings, attrName)) {
-        cssSelector.addAttribute(attrName, attrValue);
-      }
+      cssSelector.addAttribute(attrName, attrValue);
     });
-    if (isPresent(current.propertyBindings)) {
-      MapWrapper.forEach(current.propertyBindings, (expression, prop) => {
-        cssSelector.addAttribute(prop, expression.source);
-      });
-    }
-    if (isPresent(current.variableBindings)) {
-      MapWrapper.forEach(current.variableBindings, (value, name) => {
-        cssSelector.addAttribute(name, value);
-      });
-    }
+
     // Note: We assume that the ViewSplitter already did its work, i.e. template directive should
     // only be present on <template> elements any more!
     var isTemplateElement = DOM.isTemplateElement(current.element);
     var matchedProperties; // StringMap - used in dev mode to store all properties that have been matched
-    
+
     this._selectorMatcher.match(cssSelector, (selector, directive) => {
       matchedProperties = updateMatchedProperties(matchedProperties, selector, directive);
       checkDirectiveValidity(directive, current, isTemplateElement);
@@ -102,7 +91,7 @@ function updateMatchedProperties(matchedProperties, selector, directive) {
     if (isPresent(directive.annotation) && isPresent(directive.annotation.bind)) {
       var bindMap = directive.annotation.bind;
       StringMapWrapper.forEach(bindMap, (value, key) => {
-        // value is the name of the property that is intepreted
+        // value is the name of the property that is interpreted
         // e.g. 'myprop' or 'myprop | double' when a pipe is used to transform the property
 
         // keep the property name and remove the pipe
@@ -142,7 +131,7 @@ function checkMissingDirectives(current, matchedProperties, isTemplateElement) {
         if (!DOM.hasProperty(current.element, prop) && !isSpecialProperty(prop)) {
           if (!isPresent(matchedProperties) || !isPresent(StringMapWrapper.get(matchedProperties, prop))) {
             throw new BaseException(`Missing directive to handle '${prop}' in ${current.elementDescription}`);
-          } 
+          }
         }
       });
     }
diff --git a/modules/angular2/src/core/compiler/pipeline/property_binding_parser.js b/modules/angular2/src/core/compiler/pipeline/property_binding_parser.js
index 2a60a1248d..8e28cff2f7 100644
--- a/modules/angular2/src/core/compiler/pipeline/property_binding_parser.js
+++ b/modules/angular2/src/core/compiler/pipeline/property_binding_parser.js
@@ -40,27 +40,32 @@ export class PropertyBindingParser extends CompileStep {
     }
 
     var attrs = current.attrs();
+    var newAttrs = MapWrapper.create();
     var desc = current.elementDescription;
+
     MapWrapper.forEach(attrs, (attrValue, attrName) => {
       var bindParts = RegExpWrapper.firstMatch(BIND_NAME_REGEXP, attrName);
       if (isPresent(bindParts)) {
         if (isPresent(bindParts[1])) {
           // match: bind-prop
           current.addPropertyBinding(bindParts[4], this._parseBinding(attrValue, desc));
+          MapWrapper.set(newAttrs, bindParts[4], attrValue);
         } else if (isPresent(bindParts[2]) || isPresent(bindParts[7])) {
           // match: var-name / var-name="iden" / #name / #name="iden"
           var identifier = (isPresent(bindParts[4]) && bindParts[4] !== '') ?
               bindParts[4] : bindParts[8];
           var value = attrValue == '' ? '\$implicit' : attrValue;
           current.addVariableBinding(identifier, value);
+          MapWrapper.set(newAttrs, identifier, value);
         } else if (isPresent(bindParts[3])) {
-          // match: on-prop
+          // match: on-event
           current.addEventBinding(bindParts[4], this._parseAction(attrValue, desc));
         } else if (isPresent(bindParts[5])) {
           // match: [prop]
           current.addPropertyBinding(bindParts[5], this._parseBinding(attrValue, desc));
+          MapWrapper.set(newAttrs, bindParts[5], attrValue);
         } else if (isPresent(bindParts[6])) {
-          // match: (prop)
+          // match: (event)
           current.addEventBinding(bindParts[6], this._parseBinding(attrValue, desc));
         }
       } else {
@@ -70,6 +75,10 @@ export class PropertyBindingParser extends CompileStep {
         }
       }
     });
+
+    MapWrapper.forEach(newAttrs, (attrValue, attrName) => {
+      MapWrapper.set(attrs, attrName, attrValue);
+    });
   }
 
   _parseInterpolation(input:string, location:string):AST {
diff --git a/modules/angular2/src/core/compiler/pipeline/view_splitter.js b/modules/angular2/src/core/compiler/pipeline/view_splitter.js
index 68d7a273f4..685e1f5a22 100644
--- a/modules/angular2/src/core/compiler/pipeline/view_splitter.js
+++ b/modules/angular2/src/core/compiler/pipeline/view_splitter.js
@@ -109,8 +109,10 @@ export class ViewSplitter extends CompileStep {
       var binding = bindings[i];
       if (binding.keyIsVar) {
         compileElement.addVariableBinding(binding.key, binding.name);
+        MapWrapper.set(compileElement.attrs(), binding.key, binding.name);
       } else if (isPresent(binding.expression)) {
         compileElement.addPropertyBinding(binding.key, binding.expression);
+        MapWrapper.set(compileElement.attrs(), binding.key, binding.expression.source);
       } else {
         DOM.setAttribute(compileElement.element, binding.key, '');
       }
diff --git a/modules/angular2/test/core/compiler/integration_spec.js b/modules/angular2/test/core/compiler/integration_spec.js
index 40d928727e..ba33608055 100644
--- a/modules/angular2/test/core/compiler/integration_spec.js
+++ b/modules/angular2/test/core/compiler/integration_spec.js
@@ -243,6 +243,30 @@ export function main() {
         });
       });
 
+      it('should support directives where a selector matches property binding', function(done) {
+        tplResolver.setTemplate(MyComp,
+          new Template({
+            inline: '<p [id]="ctxProp"></p>',
+            directives: [IdComponent]
+          }));
+
+        compiler.compile(MyComp).then((pv) => {
+          createView(pv);
+
+          ctx.ctxProp = 'some_id';
+          cd.detectChanges();
+          expect(view.nodes[0].id).toEqual('some_id');
+          expect(DOM.getInnerHTML(view.nodes[0].shadowRoot.childNodes[0])).toEqual('Matched on id with some_id');
+
+          ctx.ctxProp = 'other_id';
+          cd.detectChanges();
+          expect(view.nodes[0].id).toEqual('other_id');
+          expect(DOM.getInnerHTML(view.nodes[0].shadowRoot.childNodes[0])).toEqual('Matched on id with other_id');
+
+          done();
+        });
+      });
+
       it('should support template directives via `<template>` elements.', (done) => {
         tplResolver.setTemplate(MyComp,
           new Template({
@@ -714,3 +738,14 @@ class DecoratorListeningEvent {
     this.msg = msg;
   }
 }
+
+@Component({
+  selector: '[id]',
+  bind: {'id': 'id'}
+})
+@Template({
+  inline: '<div>Matched on id with {{id}}</div>'
+})
+class IdComponent {
+  id: string;
+}
diff --git a/modules/angular2/test/core/compiler/pipeline/directive_parser_spec.js b/modules/angular2/test/core/compiler/pipeline/directive_parser_spec.js
index 3f03669b23..7c2c4172dc 100644
--- a/modules/angular2/test/core/compiler/pipeline/directive_parser_spec.js
+++ b/modules/angular2/test/core/compiler/pipeline/directive_parser_spec.js
@@ -39,10 +39,14 @@ export function main() {
           if (isPresent(propertyBindings)) {
             StringMapWrapper.forEach(propertyBindings, (v, k) => {
               current.addPropertyBinding(k, parser.parseBinding(v, null));
+              MapWrapper.set(current.attrs(), k, v);
             });
           }
           if (isPresent(variableBindings)) {
-            current.variableBindings = MapWrapper.createFromStringMap(variableBindings);
+            StringMapWrapper.forEach(variableBindings, (v, k) => {
+              current.addVariableBinding(k, v);
+              MapWrapper.set(current.attrs(), k, v);
+            });
           }
         }), new DirectiveParser(annotatedDirectives)]);
     }
@@ -89,7 +93,7 @@ export function main() {
            createPipeline().process(
              el('<div some-comp some-comp2></div>')
            );
-         }).toThrowError('Multiple component directives not allowed on the same element - check <div some-comp some-comp2>'); 
+         }).toThrowError('Multiple component directives not allowed on the same element - check <div some-comp some-comp2>');
       });
 
       it('should not allow component directives on <template> elements', () => {