refactor(EventHandler): have a single instance of event handlers per ProtoView

modules/angular2/src/core/compiler/element_injector.js

@@ -1,6 +1,6 @@
 import {FIELD, isPresent, isBlank, Type, int, BaseException} from 'angular2/src/facade/lang';
 import {Math} from 'angular2/src/facade/math';
-import {List, ListWrapper, MapWrapper, StringMap, StringMapWrapper} from 'angular2/src/facade/collection';
+import {List, ListWrapper, MapWrapper} from 'angular2/src/facade/collection';
 import {Injector, Key, Dependency, bind, Binding, NoProviderError, ProviderError, CyclicDependencyError} from 'angular2/di';
 import {Parent, Ancestor} from 'angular2/src/core/annotations/visibility';
 import {EventEmitter, PropertySetter} from 'angular2/src/core/annotations/di';
@@ -18,8 +18,6 @@ var MAX_DEPTH = Math.pow(2, 30) - 1;
 
 var _undefined = new Object();
 
-var _noop = function(_) {};
-
 var _staticKeys;
 
 class StaticKeys {
@@ -272,9 +270,8 @@ export class ProtoElementInjector  {
     }
   }
 
-  instantiate(parent:ElementInjector, host:ElementInjector, events,
+  instantiate(parent:ElementInjector, host:ElementInjector, reflector: Reflector):ElementInjector {
-     reflector: Reflector):ElementInjector {
+    return new ElementInjector(this, parent, host, reflector);
-    return new ElementInjector(this, parent, host, events, reflector);
   }
 
   directParent(): ProtoElementInjector {
@@ -327,11 +324,10 @@ export class ElementInjector extends TreeNode {
   _obj9:any;
   _preBuiltObjects;
   _constructionCounter;
-  _events:StringMap;
   _refelector: Reflector;
 
   constructor(proto:ProtoElementInjector, parent:ElementInjector, host:ElementInjector,
-    events: StringMap, reflector: Reflector) {
+    reflector: Reflector) {
     super(parent);
     if (isPresent(parent) && isPresent(host)) {
       throw new BaseException('Only either parent or host is allowed');
@@ -350,7 +346,6 @@ export class ElementInjector extends TreeNode {
     this._preBuiltObjects = null;
     this._lightDomAppInjector = null;
     this._shadowDomAppInjector = null;
-    this._events = events;
     this._obj0 = null;
     this._obj1 = null;
     this._obj2 = null;
@@ -515,13 +510,9 @@ export class ElementInjector extends TreeNode {
 
   _buildEventEmitter(dep) {
     var view = this._getPreBuiltObjectByKeyId(StaticKeys.instance().viewId);
-    if (isPresent(this._events)) {
+    return (event) => {
-      var eventMap = StringMapWrapper.get(this._events, dep.eventEmitterName);
+      view.triggerEventHandlers(dep.eventEmitterName, event, this._proto.index);
-      if (isPresent(eventMap)) {
+    };
-        return ProtoView.buildEventCallback(eventMap, view, this._proto.index);
-      }
-    }
-    return _noop;
   }
 
   _buildPropSetter(dep) {

modules/angular2/src/core/compiler/view.js

@@ -213,6 +213,23 @@ export class View {
     this._dehydrateContext();
   }
 
+  /**
+   * Triggers the event handlers for the element and the directives.
+   *
+   * This method is intended to be called from directive EventEmitters.
+   *
+   * @param {string} eventName
+   * @param {*} eventObj
+   * @param {int} binderIndex
+   */
+  triggerEventHandlers(eventName: string, eventObj, binderIndex: int) {
+    var handlers = this.proto.eventHandlers[binderIndex];
+    if (isBlank(handlers)) return;
+    var handler = StringMapWrapper.get(handlers, eventName);
+    if (isBlank(handler)) return;
+    handler(eventObj, this);
+  }
+
   onRecordChange(directiveMemento, records:List) {
     this._invokeMementos(records);
     if (directiveMemento instanceof DirectiveMemento) {
@@ -278,6 +295,8 @@ export class ProtoView {
   shadowDomStrategy: ShadowDomStrategy;
   _viewPool: ViewPool;
   stylePromises: List<Promise>;
+  // List<Map<eventName, handler>>, indexed by binder index
+  eventHandlers: List;
 
   constructor(
       template,
@@ -297,6 +316,7 @@ export class ProtoView {
     this.shadowDomStrategy = shadowDomStrategy;
     this._viewPool = new ViewPool(VIEW_POOL_CAPACITY);
     this.stylePromises = [];
+    this.eventHandlers = [];
   }
 
   // TODO(rado): hostElementInjector should be moved to hydrate phase.
@@ -346,6 +366,7 @@ export class ProtoView {
     var view = new View(this, viewNodes, this.protoChangeDetector, this.protoContextLocals);
     var binders = this.elementBinders;
     var elementInjectors = ListWrapper.createFixedSize(binders.length);
+    var eventHandlers = ListWrapper.createFixedSize(binders.length);
     var rootElementInjectors = [];
     var textNodes = [];
     var elementsWithPropertyBindings = [];
@@ -368,11 +389,9 @@ export class ProtoView {
       if (isPresent(protoElementInjector)) {
         if (isPresent(protoElementInjector.parent)) {
           var parentElementInjector = elementInjectors[protoElementInjector.parent.index];
-          elementInjector = protoElementInjector.instantiate(parentElementInjector, null,
+          elementInjector = protoElementInjector.instantiate(parentElementInjector, null, reflector);
-            binder.events, reflector);
         } else {
-          elementInjector = protoElementInjector.instantiate(null, hostElementInjector,
+          elementInjector = protoElementInjector.instantiate(null, hostElementInjector, reflector);
-            binder.events, reflector);
           ListWrapper.push(rootElementInjectors, elementInjector);
         }
       }
@@ -427,15 +446,20 @@ export class ProtoView {
 
       // events
       if (isPresent(binder.events)) {
+        eventHandlers[binderIdx] = StringMapWrapper.create();
         StringMapWrapper.forEach(binder.events, (eventMap, eventName) => {
+          var handler = ProtoView.buildEventHandler(eventMap, binderIdx);
+          StringMapWrapper.set(eventHandlers[binderIdx], eventName, handler);
           if (isBlank(elementInjector) || !elementInjector.hasEventEmitter(eventName)) {
-            var handler = ProtoView.buildEventCallback(eventMap, view, binderIdx);
+            eventManager.addEventListener(element, eventName,
-            eventManager.addEventListener(element, eventName, handler);
+              (event) => { handler(event, view); });
           }
         });
       }
     }
 
+    this.eventHandlers = eventHandlers;
+
     view.init(elementInjectors, rootElementInjectors, textNodes, elementsWithPropertyBindings,
       viewContainers, preBuiltObjects, componentChildViews);
 
@@ -447,24 +471,15 @@ export class ProtoView {
   }
 
   /**
-   * Create an event callback invoked in the context of the enclosing View
+   * Creates an event handler.
-   *
-   * @param {AST} expr
-   * @param {View} view
-   * @returns {Function}
-   */
-
-  /**
-   * Creates the event callback.
    *
    * @param {Map} eventMap Map directiveIndexes to expressions
-   * @param {View} view
    * @param {int} injectorIdx
    * @returns {Function}
    */
-  static buildEventCallback(eventMap: Map, view:View, injectorIdx: int) {
+  static buildEventHandler(eventMap: Map, injectorIdx: int) {
     var locals = MapWrapper.create();
-    return (event) => {
+    return (event, view) => {
       // Most of the time the event will be fired only when the view is in the live document.
       // However, in a rare circumstance the view might get dehydrated, in between the event
       // queuing up and firing.

modules/angular2/test/core/compiler/element_injector_spec.js

@@ -1,18 +1,19 @@
 import {describe, ddescribe, it, iit, xit, xdescribe, expect, beforeEach, SpyObject, proxy, el} from 'angular2/test_lib';
 import {isBlank, isPresent, FIELD, IMPLEMENTS} from 'angular2/src/facade/lang';
-import {ListWrapper, MapWrapper, List} from 'angular2/src/facade/collection';
+import {ListWrapper, MapWrapper, List, StringMapWrapper} from 'angular2/src/facade/collection';
 import {ProtoElementInjector, PreBuiltObjects, DirectiveBinding} from 'angular2/src/core/compiler/element_injector';
 import {Parent, Ancestor} from 'angular2/src/core/annotations/visibility';
 import {EventEmitter, PropertySetter} from 'angular2/src/core/annotations/di';
 import {onDestroy} from 'angular2/src/core/annotations/annotations';
 import {Optional, Injector, Inject, bind} from 'angular2/di';
-import {View} from 'angular2/src/core/compiler/view';
+import {ProtoView, View} from 'angular2/src/core/compiler/view';
 import {ViewContainer} from 'angular2/src/core/compiler/view_container';
 import {NgElement} from 'angular2/src/core/dom/element';
 import {LightDom, SourceLightDom, DestinationLightDom} from 'angular2/src/core/compiler/shadow_dom_emulation/light_dom';
 import {Directive} from 'angular2/src/core/annotations/annotations';
 import {BindingPropagationConfig} from 'angular2/src/core/compiler/binding_propagation_config';
 import {reflector} from 'angular2/src/reflection/reflection';
+import {DynamicProtoChangeDetector} from 'angular2/change_detection';
 
 @proxy
 @IMPLEMENTS(View)
@@ -130,7 +131,7 @@ export function main() {
     if (isBlank(lightDomAppInjector)) lightDomAppInjector = new Injector([]);
 
     var proto = new ProtoElementInjector(null, 0, bindings, isPresent(shadowDomAppInjector));
-    var inj = proto.instantiate(null, null, null, reflector);
+    var inj = proto.instantiate(null, null, reflector);
     var preBuilt = isPresent(preBuiltObjects) ? preBuiltObjects : defaultPreBuiltObjects;
 
     inj.instantiateDirectives(lightDomAppInjector, shadowDomAppInjector, preBuilt);
@@ -143,12 +144,12 @@ export function main() {
     var inj = new Injector([]);
 
     var protoParent = new ProtoElementInjector(null, 0, parentBindings);
-    var parent = protoParent.instantiate(null, null, null, reflector);
+    var parent = protoParent.instantiate(null, null, reflector);
 
     parent.instantiateDirectives(inj, null, parentPreBuildObjects);
 
     var protoChild = new ProtoElementInjector(protoParent, 1, childBindings, false, 1);
-    var child = protoChild.instantiate(parent, null, null, reflector);
+    var child = protoChild.instantiate(parent, null, reflector);
     child.instantiateDirectives(inj, null, defaultPreBuiltObjects);
 
     return child;
@@ -161,11 +162,11 @@ export function main() {
     var shadowInj = inj.createChild([]);
 
     var protoParent = new ProtoElementInjector(null, 0, hostBindings, true);
-    var host = protoParent.instantiate(null, null, null, reflector);
+    var host = protoParent.instantiate(null, null, reflector);
     host.instantiateDirectives(inj, shadowInj, hostPreBuildObjects);
 
     var protoChild = new ProtoElementInjector(protoParent, 0, shadowBindings, false, 1);
-    var shadow = protoChild.instantiate(null, host, null, reflector);
+    var shadow = protoChild.instantiate(null, host, reflector);
     shadow.instantiateDirectives(shadowInj, null, null);
 
     return shadow;
@@ -198,9 +199,9 @@ export function main() {
         var protoChild1 = new ProtoElementInjector(protoParent, 1, []);
         var protoChild2 = new ProtoElementInjector(protoParent, 2, []);
 
-        var p = protoParent.instantiate(null, null, null, reflector);
+        var p = protoParent.instantiate(null, null, reflector);
-        var c1 = protoChild1.instantiate(p, null, null, reflector);
+        var c1 = protoChild1.instantiate(p, null, reflector);
-        var c2 = protoChild2.instantiate(p, null, null, reflector);
+        var c2 = protoChild2.instantiate(p, null, reflector);
 
         expect(humanize(p, [
           [p, 'parent'],
@@ -215,8 +216,8 @@ export function main() {
           var protoParent = new ProtoElementInjector(null, 0, []);
           var protoChild = new ProtoElementInjector(protoParent, 1, [], false, distance);
 
-          var p = protoParent.instantiate(null, null, null, reflector);
+          var p = protoParent.instantiate(null, null, reflector);
-          var c = protoChild.instantiate(p, null, null, reflector);
+          var c = protoChild.instantiate(p, null, reflector);
 
           expect(c.directParent()).toEqual(p);
         });
@@ -226,8 +227,8 @@ export function main() {
           var protoParent = new ProtoElementInjector(null, 0, []);
           var protoChild = new ProtoElementInjector(protoParent, 1, [], false, distance);
 
-          var p = protoParent.instantiate(null, null, null, reflector);
+          var p = protoParent.instantiate(null, null, reflector);
-          var c = protoChild.instantiate(p, null, null, reflector);
+          var c = protoChild.instantiate(p, null, reflector);
 
           expect(c.directParent()).toEqual(null);
         });
@@ -477,8 +478,16 @@ export function main() {
 
     describe('event emitters', () => {
       it('should be injectable and callable', () => {
-        var inj = injector([NeedsEventEmitter]);
+        var called = false;
+        var handlers = StringMapWrapper.create();
+        StringMapWrapper.set(handlers, 'click', (e, view) => { called = true;});
+        var pv = new ProtoView(null, null, null);
+        pv.eventHandlers = [handlers];
+        var view = new View(pv, null, new DynamicProtoChangeDetector(null), MapWrapper.create());
+        var preBuildObject = new PreBuiltObjects(view, null, null, null, null);
+        var inj = injector([NeedsEventEmitter], null, null, preBuildObject);
         inj.get(NeedsEventEmitter).click();
+        expect(called).toEqual(true);
       });
 
       it('should be queryable through hasEventEmitter', () => {

modules/angular2/test/core/compiler/view_container_spec.js

@@ -72,7 +72,7 @@ export function main() {
       parentView = createView([dom.childNodes[0]]);
       protoView = new ProtoView(el('<div>hi</div>'), new DynamicProtoChangeDetector(null),
         new NativeShadowDomStrategy(null));
-      elementInjector = new ElementInjector(null, null, null, null, reflector);
+      elementInjector = new ElementInjector(null, null, null, reflector);
       viewContainer = new ViewContainer(parentView, insertionElement, protoView, elementInjector,
         null, reflector);
       customViewWithOneNode = createView([el('<div>single</div>')]);

modules/angular2/test/core/compiler/view_spec.js

@@ -264,7 +264,7 @@ export function main() {
           pv.bindElement(testProtoElementInjector);
 
           var hostProtoInjector = new ProtoElementInjector(null, 0, []);
-          var hostInjector = hostProtoInjector.instantiate(null, null, null, reflector);
+          var hostInjector = hostProtoInjector.instantiate(null, null, reflector);
           var view;
           expect(() => view = pv.instantiate(hostInjector, null, reflector)).not.toThrow();
           expect(testProtoElementInjector.parentElementInjector).toBe(view.elementInjectors[0]);
@@ -279,7 +279,7 @@ export function main() {
           pv.bindElement(testProtoElementInjector);
 
           var hostProtoInjector = new ProtoElementInjector(null, 0, []);
-          var hostInjector = hostProtoInjector.instantiate(null, null, null, reflector);
+          var hostInjector = hostProtoInjector.instantiate(null, null, reflector);
           expect(() => pv.instantiate(hostInjector, null, reflector)).not.toThrow();
           expect(testProtoElementInjector.parentElementInjector).toBeNull();
           expect(testProtoElementInjector.hostElementInjector).toBe(hostInjector);
@@ -751,11 +751,10 @@ class TestProtoElementInjector extends ProtoElementInjector {
     super(parent, index, bindings, firstBindingIsComponent);
   }
 
-  instantiate(parent:ElementInjector, host:ElementInjector, events,
+  instantiate(parent:ElementInjector, host:ElementInjector, reflector: Reflector):ElementInjector {
-              reflector: Reflector):ElementInjector {
     this.parentElementInjector = parent;
     this.hostElementInjector = host;
-    return super.instantiate(parent, host, events, reflector);
+    return super.instantiate(parent, host, reflector);
   }
 }
 

modules/benchmarks/src/element_injector/element_injector_benchmark.js

@@ -33,11 +33,11 @@ export function main() {
 
   var bindings = [A, B, C];
   var proto = new ProtoElementInjector(null, 0, bindings);
-  var elementInjector = proto.instantiate(null, null, null, null);
+  var elementInjector = proto.instantiate(null, null, null);
 
   function instantiate () {
     for (var i = 0; i < iterations; ++i) {
-      var ei = proto.instantiate(null, null, null, null);
+      var ei = proto.instantiate(null, null, null);
       ei.instantiateDirectives(appInjector, null, null);
     }
   }