From fc5b7edca488dce9ceec9ff845930cce69a9d676 Mon Sep 17 00:00:00 2001
From: Tobias Bosch <tbosch1009@gmail.com>
Date: Wed, 19 Nov 2014 14:54:07 -0800
Subject: [PATCH] feat(compiler): support `on-` and `[]`

---
 modules/core/src/compiler/element_binder.js   |  8 +++++--
 .../src/compiler/pipeline/compile_element.js  |  8 +++++++
 .../pipeline/element_binder_builder.js        | 10 +++++++++
 .../pipeline/element_binding_marker.js        |  2 ++
 .../pipeline/property_binding_parser.js       | 18 ++++++++++-----
 modules/core/src/compiler/view.js             | 22 ++++++++++++++++---
 .../pipeline/element_binder_builder_spec.js   | 20 ++++++++++++++++-
 .../pipeline/element_binding_marker_spec.js   | 11 +++++++++-
 .../pipeline/property_binding_parser_spec.js  | 10 +++++++++
 9 files changed, 97 insertions(+), 12 deletions(-)

diff --git a/modules/core/src/compiler/element_binder.js b/modules/core/src/compiler/element_binder.js
index b97ccb732b..6d1a704c2e 100644
--- a/modules/core/src/compiler/element_binder.js
+++ b/modules/core/src/compiler/element_binder.js
@@ -1,5 +1,6 @@
 import {ProtoElementInjector} from './element_injector';
 import {FIELD} from 'facade/lang';
+import {MapWrapper} from 'facade/collection';
 import {AnnotatedType} from './annotated_type';
 // Comment out as dartanalyzer does not look into @FIELD
 // import {List} from 'facade/collection';
@@ -11,12 +12,15 @@ export class ElementBinder {
   @FIELD('final templateDirective:AnnotatedType')
   @FIELD('final textNodeIndices:List<int>')
   @FIELD('hasElementPropertyBindings:bool')
-  constructor(protoElementInjector: ProtoElementInjector, componentDirective:AnnotatedType, templateDirective:AnnotatedType) {
+  constructor(
+    protoElementInjector: ProtoElementInjector, componentDirective:AnnotatedType, templateDirective:AnnotatedType) {
     this.protoElementInjector = protoElementInjector;
     this.componentDirective = componentDirective;
     this.templateDirective = templateDirective;
+    // updated later when events are bound
+    this.events = null;
     // updated later when text nodes are bound
-    this.textNodeIndices = [];
+    this.textNodeIndices = null;
     // updated later when element properties are bound
     this.hasElementPropertyBindings = false;
     // updated later, so we are able to resolve cycles
diff --git a/modules/core/src/compiler/pipeline/compile_element.js b/modules/core/src/compiler/pipeline/compile_element.js
index 30fa4d971f..83245972ba 100644
--- a/modules/core/src/compiler/pipeline/compile_element.js
+++ b/modules/core/src/compiler/pipeline/compile_element.js
@@ -20,6 +20,7 @@ export class CompileElement {
     this._classList = null;
     this.textNodeBindings = null;
     this.propertyBindings = null;
+    this.eventBindings = null;
     this.variableBindings = null;
     this.decoratorDirectives = null;
     this.templateDirective = null;
@@ -84,6 +85,13 @@ export class CompileElement {
     MapWrapper.set(this.variableBindings, contextName, templateName);
   }
 
+  addEventBinding(eventName:string, expression:ASTWithSource) {
+    if (isBlank(this.eventBindings)) {
+      this.eventBindings = MapWrapper.create();
+    }
+    MapWrapper.set(this.eventBindings, eventName, expression);
+  }
+
   addDirective(directive:AnnotatedType) {
     var annotation = directive.annotation;
     if (annotation instanceof Decorator) {
diff --git a/modules/core/src/compiler/pipeline/element_binder_builder.js b/modules/core/src/compiler/pipeline/element_binder_builder.js
index 10d8b1786c..21233f4adf 100644
--- a/modules/core/src/compiler/pipeline/element_binder_builder.js
+++ b/modules/core/src/compiler/pipeline/element_binder_builder.js
@@ -33,6 +33,7 @@ import {CompileControl} from './compile_control';
  * - CompileElement#inheritedProtoElementInjector
  * - CompileElement#textNodeBindings
  * - CompileElement#propertyBindings
+ * - CompileElement#eventBindings
  * - CompileElement#decoratorDirectives
  * - CompileElement#componentDirective
  * - CompileElement#templateDirective
@@ -60,6 +61,9 @@ export class ElementBinderBuilder extends CompileStep {
       if (isPresent(current.propertyBindings)) {
         this._bindElementProperties(protoView, current);
       }
+      if (isPresent(current.eventBindings)) {
+        this._bindEvents(protoView, current);
+      }
       this._bindDirectiveProperties(this._collectDirectives(current), current);
     } else if (isPresent(parent)) {
       elementBinder = parent.inheritedElementBinder;
@@ -79,6 +83,12 @@ export class ElementBinderBuilder extends CompileStep {
     });
   }
 
+  _bindEvents(protoView, compileElement) {
+    MapWrapper.forEach(compileElement.eventBindings, (expression, eventName) => {
+      protoView.bindEvent(eventName,  expression.ast);
+    });
+  }
+
   _collectDirectives(compileElement) {
     var directives;
     if (isPresent(compileElement.decoratorDirectives)) {
diff --git a/modules/core/src/compiler/pipeline/element_binding_marker.js b/modules/core/src/compiler/pipeline/element_binding_marker.js
index 5adf802bd8..441fd67f6a 100644
--- a/modules/core/src/compiler/pipeline/element_binding_marker.js
+++ b/modules/core/src/compiler/pipeline/element_binding_marker.js
@@ -19,6 +19,7 @@ const NG_BINDING_CLASS = 'ng-binding';
  * - CompileElement#textNodeBindings
  * - CompileElement#propertyBindings
  * - CompileElement#variableBindings
+ * - CompileElement#eventBindings
  * - CompileElement#decoratorDirectives
  * - CompileElement#componentDirective
  * - CompileElement#templateDirective
@@ -29,6 +30,7 @@ export class ElementBindingMarker extends CompileStep {
       (isPresent(current.textNodeBindings) && MapWrapper.size(current.textNodeBindings)>0) ||
       (isPresent(current.propertyBindings) && MapWrapper.size(current.propertyBindings)>0) ||
       (isPresent(current.variableBindings) && MapWrapper.size(current.variableBindings)>0) ||
+      (isPresent(current.eventBindings) && MapWrapper.size(current.eventBindings)>0) ||
       (isPresent(current.decoratorDirectives) && current.decoratorDirectives.length > 0) ||
       isPresent(current.templateDirective) ||
       isPresent(current.componentDirective);
diff --git a/modules/core/src/compiler/pipeline/property_binding_parser.js b/modules/core/src/compiler/pipeline/property_binding_parser.js
index 79cc94dc86..47961dc860 100644
--- a/modules/core/src/compiler/pipeline/property_binding_parser.js
+++ b/modules/core/src/compiler/pipeline/property_binding_parser.js
@@ -12,13 +12,15 @@ import {CompileControl} from './compile_control';
 import {interpolationToExpression} from './text_interpolation_parser';
 
 // TODO(tbosch): Cannot make this const/final right now because of the transpiler...
-var BIND_NAME_REGEXP = RegExpWrapper.create('^(?:(?:(bind)|(let))-(.+))|\\[([^\\]]+)\\]');
+var BIND_NAME_REGEXP = RegExpWrapper.create('^(?:(?:(bind)|(let)|(on))-(.+))|\\[([^\\]]+)\\]|\\(([^\\]]+)\\)');
 
 /**
  * Parses the property bindings on a single element.
  *
  * Fills:
  * - CompileElement#propertyBindings
+ * - CompileElement#eventBindings
+ * - CompileElement#variableBindings
  */
 export class PropertyBindingParser extends CompileStep {
   constructor(parser:Parser) {
@@ -32,7 +34,7 @@ export class PropertyBindingParser extends CompileStep {
       if (isPresent(bindParts)) {
         if (isPresent(bindParts[1])) {
           // match: bind-prop
-          current.addPropertyBinding(bindParts[3], this._parser.parseBinding(attrValue));
+          current.addPropertyBinding(bindParts[4], this._parser.parseBinding(attrValue));
         } else if (isPresent(bindParts[2])) {
           // match: let-prop
           // Note: We assume that the ViewSplitter already did its work, i.e. template directive should
@@ -40,10 +42,16 @@ export class PropertyBindingParser extends CompileStep {
           if (!(current.element instanceof TemplateElement)) {
             throw new BaseException('let-* is only allowed on <template> elements!');
           }
-          current.addVariableBinding(bindParts[3], attrValue);
-        } else if (isPresent(bindParts[4])) {
+          current.addVariableBinding(bindParts[4], attrValue);
+        } else if (isPresent(bindParts[3])) {
+          // match: on-prop
+          current.addEventBinding(bindParts[4], this._parser.parseAction(attrValue));
+        } else if (isPresent(bindParts[5])) {
           // match: [prop]
-          current.addPropertyBinding(bindParts[4], this._parser.parseBinding(attrValue));
+          current.addPropertyBinding(bindParts[5], this._parser.parseBinding(attrValue));
+        } else if (isPresent(bindParts[6])) {
+          // match: (prop)
+          current.addEventBinding(bindParts[6], this._parser.parseBinding(attrValue));
         }
       } else {
         var expression = interpolationToExpression(attrValue);
diff --git a/modules/core/src/compiler/view.js b/modules/core/src/compiler/view.js
index 3f8cefc972..e844cbcd8c 100644
--- a/modules/core/src/compiler/view.js
+++ b/modules/core/src/compiler/view.js
@@ -141,6 +141,9 @@ export class ProtoView {
    */
   bindTextNode(indexInParent:int, expression:AST) {
     var elBinder = this.elementBinders[this.elementBinders.length-1];
+    if (isBlank(elBinder.textNodeIndices)) {
+      elBinder.textNodeIndices = ListWrapper.create();
+    }
     ListWrapper.push(elBinder.textNodeIndices, indexInParent);
     this.protoWatchGroup.watch(expression, this.textNodesWithBindingCount++);
   }
@@ -162,6 +165,17 @@ export class ProtoView {
     );
   }
 
+  /**
+   * Adds an event binding for the last created ElementBinder via bindElement
+   */
+  bindEvent(eventName:string, expression:AST) {
+    var elBinder = this.elementBinders[this.elementBinders.length-1];
+    if (isBlank(elBinder.events)) {
+      elBinder.events = MapWrapper.create();
+    }
+    MapWrapper.set(elBinder.events, eventName, expression);
+  }
+
   /**
    * Adds a directive property binding for the last created ElementBinder via bindElement
    */
@@ -228,9 +242,11 @@ export class ProtoView {
   }
 
   static _collectTextNodes(allTextNodes, element, indices) {
-    var childNodes = DOM.templateAwareRoot(element).childNodes;
-    for (var i = 0; i < indices.length; ++i) {
-      ListWrapper.push(allTextNodes, childNodes[indices[i]]);
+    if (isPresent(indices)) {
+      var childNodes = DOM.templateAwareRoot(element).childNodes;
+      for (var i = 0; i < indices.length; ++i) {
+        ListWrapper.push(allTextNodes, childNodes[indices[i]]);
+      }
     }
   }
 
diff --git a/modules/core/test/compiler/pipeline/element_binder_builder_spec.js b/modules/core/test/compiler/pipeline/element_binder_builder_spec.js
index b0fc1b4381..b904e02e83 100644
--- a/modules/core/test/compiler/pipeline/element_binder_builder_spec.js
+++ b/modules/core/test/compiler/pipeline/element_binder_builder_spec.js
@@ -27,7 +27,7 @@ export function main() {
   describe('ElementBinderBuilder', () => {
     var evalContext, view, changeDetector;
 
-    function createPipeline({textNodeBindings, propertyBindings, directives, protoElementInjector
+    function createPipeline({textNodeBindings, propertyBindings, eventBindings, directives, protoElementInjector
     }={}) {
       var reflector = new Reflector();
       var closureMap = new ClosureMap();
@@ -55,6 +55,12 @@ export function main() {
               }
               hasBinding = true;
             }
+            if (isPresent(current.element.getAttribute('event-binding'))) {
+              MapWrapper.forEach(eventBindings, (v,k) => {
+                current.addEventBinding(k, parser.parseAction(v));
+              });
+              hasBinding = true;
+            }
             if (isPresent(protoElementInjector)) {
               current.inheritedProtoElementInjector = protoElementInjector;
             }
@@ -172,6 +178,18 @@ export function main() {
       expect(DOM.getProperty(view.nodes[0], 'elprop2')).toEqual('b');
     });
 
+    it('should bind events', () => {
+      var eventBindings = MapWrapper.createFromStringMap({
+        'event1': '1+1'
+      });
+      var pipeline = createPipeline({eventBindings: eventBindings});
+      var results = pipeline.process(createElement('<div viewroot event-binding></div>'));
+      var pv = results[0].inheritedProtoView;
+
+      var ast = MapWrapper.get(pv.elementBinders[0].events, 'event1');
+      expect(ast.eval(null)).toBe(2);
+    });
+
     it('should bind directive properties', () => {
       var propertyBindings = MapWrapper.createFromStringMap({
         'boundprop1': 'prop1',
diff --git a/modules/core/test/compiler/pipeline/element_binding_marker_spec.js b/modules/core/test/compiler/pipeline/element_binding_marker_spec.js
index 7115bbc983..94f8943112 100644
--- a/modules/core/test/compiler/pipeline/element_binding_marker_spec.js
+++ b/modules/core/test/compiler/pipeline/element_binding_marker_spec.js
@@ -16,7 +16,7 @@ import {Component} from 'core/annotations/component';
 export function main() {
   describe('ElementBindingMarker', () => {
 
-    function createPipeline({textNodeBindings, propertyBindings, variableBindings, directives}={}) {
+    function createPipeline({textNodeBindings, propertyBindings, variableBindings, eventBindings, directives}={}) {
       var reflector = new Reflector();
       return new CompilePipeline([
         new MockStep((parent, current, control) => {
@@ -29,6 +29,9 @@ export function main() {
             if (isPresent(variableBindings)) {
               current.variableBindings = variableBindings;
             }
+            if (isPresent(eventBindings)) {
+              current.eventBindings = eventBindings;
+            }
             if (isPresent(directives)) {
               for (var i=0; i<directives.length; i++) {
                 current.addDirective(reflector.annotatedType(directives[i]));
@@ -62,6 +65,12 @@ export function main() {
       assertBinding(results[0], true);
     });
 
+    it('should mark elements with event bindings', () => {
+      var eventBindings = MapWrapper.createFromStringMap({'click': 'expr'});
+      var results = createPipeline({eventBindings: eventBindings}).process(createElement('<div></div>'));
+      assertBinding(results[0], true);
+    });
+
     it('should mark elements with decorator directives', () => {
       var results = createPipeline({
         directives: [SomeDecoratorDirective]
diff --git a/modules/core/test/compiler/pipeline/property_binding_parser_spec.js b/modules/core/test/compiler/pipeline/property_binding_parser_spec.js
index 4c0dd24663..fea0d0228d 100644
--- a/modules/core/test/compiler/pipeline/property_binding_parser_spec.js
+++ b/modules/core/test/compiler/pipeline/property_binding_parser_spec.js
@@ -41,6 +41,16 @@ export function main() {
         createPipeline().process(createElement('<div let-a="b"></div>'))
       }).toThrowError('let-* is only allowed on <template> elements!');
     });
+
+    it('should detect () syntax', () => {
+      var results = createPipeline().process(createElement('<div (click)="b()"></div>'));
+      expect(MapWrapper.get(results[0].eventBindings, 'click').source).toEqual('b()');
+    });
+
+    it('should detect on- syntax', () => {
+      var results = createPipeline().process(createElement('<div on-click="b()"></div>'));
+      expect(MapWrapper.get(results[0].eventBindings, 'click').source).toEqual('b()');
+    });
   });
 }