perf(render): only create LightDom instances if the element has children

tbosch · tbosch · commit ca0970134396 · 2015-06-11T14:45:44.000-07:00
diff --git a/modules/angular2/src/render/dom/dom_renderer.ts b/modules/angular2/src/render/dom/dom_renderer.ts
@@ -276,11 +276,15 @@ export class DomRenderer extends Renderer {
     for (var binderIdx = 0; binderIdx < binders.length; binderIdx++) {
       var binder = binders[binderIdx];
       var element = boundElements[binderIdx];
+      var domEl = element.element;
 
       // lightDoms
       var lightDom = null;
-      if (isPresent(binder.componentId)) {
-        lightDom = this._shadowDomStrategy.constructLightDom(view, element.element);
+      // Note: for the root element we can't use the binder.elementIsEmpty
+      // information as we don't use the element from the ProtoView
+      // but an element from the document.
+      if (isPresent(binder.componentId) && (!binder.elementIsEmpty || isPresent(inplaceElement))) {
+        lightDom = this._shadowDomStrategy.constructLightDom(view, domEl);
       }
       element.lightDom = lightDom;
 
@@ -294,7 +298,7 @@ export class DomRenderer extends Renderer {
       // events
       if (isPresent(binder.eventLocals) && isPresent(binder.localEvents)) {
         for (var i = 0; i < binder.localEvents.length; i++) {
-          this._createEventListener(view, element.element, binderIdx, binder.localEvents[i].name,
+          this._createEventListener(view, domEl, binderIdx, binder.localEvents[i].name,
                                     binder.eventLocals);
         }
       }
diff --git a/modules/angular2/src/render/dom/view/element_binder.ts b/modules/angular2/src/render/dom/view/element_binder.ts
@@ -15,10 +15,11 @@ export class ElementBinder {
   distanceToParent: number;
   propertySetters: Map<string, SetterFn>;
   hostActions: Map<string, AST>;
+  elementIsEmpty: boolean;
 
   constructor({textNodeIndices, contentTagSelector, nestedProtoView, componentId, eventLocals,
                localEvents, globalEvents, hostActions, parentIndex, distanceToParent,
-               propertySetters}: {
+               propertySetters, elementIsEmpty}: {
     contentTagSelector?: string,
     textNodeIndices?: List<number>,
     nestedProtoView?: protoViewModule.DomProtoView,
@@ -29,7 +30,8 @@ export class ElementBinder {
     parentIndex?: number,
     distanceToParent?: number,
     propertySetters?: Map<string, SetterFn>,
-    hostActions?: Map<string, AST>
+    hostActions?: Map<string, AST>,
+    elementIsEmpty?: boolean
   } = {}) {
     this.textNodeIndices = textNodeIndices;
     this.contentTagSelector = contentTagSelector;
@@ -42,6 +44,7 @@ export class ElementBinder {
     this.parentIndex = parentIndex;
     this.distanceToParent = distanceToParent;
     this.propertySetters = propertySetters;
+    this.elementIsEmpty = elementIsEmpty;
   }
 }
 
diff --git a/modules/angular2/src/render/dom/view/proto_view_builder.ts b/modules/angular2/src/render/dom/view/proto_view_builder.ts
@@ -57,7 +57,8 @@ export class ProtoViewBuilder {
 
         MapWrapper.forEach(dbb.hostPropertyBindings, (_, hostPropertyName) => {
           MapWrapper.set(propertySetters, hostPropertyName,
-                         setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), hostPropertyName));
+                         setterFactory.createSetter(ebb.element, isPresent(ebb.componentId),
+                                                    hostPropertyName));
         });
 
         ListWrapper.forEach(dbb.hostActions, (hostAction) => {
@@ -73,7 +74,9 @@ export class ProtoViewBuilder {
       });
 
       MapWrapper.forEach(ebb.propertyBindings, (_, propertyName) => {
-        MapWrapper.set(propertySetters, propertyName, setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), propertyName));
+        MapWrapper.set(
+            propertySetters, propertyName,
+            setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), propertyName));
       });
 
       var nestedProtoView =
@@ -99,6 +102,7 @@ export class ProtoViewBuilder {
         textBindings: ebb.textBindings,
         readAttributes: ebb.readAttributes
       }));
+      var elementIsEmpty = this._isEmptyElement(ebb.element);
       ListWrapper.push(renderElementBinders, new ElementBinder({
                          textNodeIndices: ebb.textBindingIndices,
                          contentTagSelector: ebb.contentTagSelector,
@@ -112,7 +116,8 @@ export class ProtoViewBuilder {
                          localEvents: ebb.eventBuilder.buildLocalEvents(),
                          globalEvents: ebb.eventBuilder.buildGlobalEvents(),
                          hostActions: hostActions,
-                         propertySetters: propertySetters
+                         propertySetters: propertySetters,
+                         elementIsEmpty: elementIsEmpty
                        }));
     });
     return new api.ProtoViewDto({
@@ -126,6 +131,18 @@ export class ProtoViewBuilder {
       variableBindings: this.variableBindings
     });
   }
+
+  _isEmptyElement(el) {
+    var childNodes = DOM.childNodes(el);
+    for (var i = 0; i < childNodes.length; i++) {
+      var node = childNodes[i];
+      if ((DOM.isTextNode(node) && DOM.getText(node).trim().length > 0) ||
+          (DOM.isElementNode(node))) {
+        return false;
+      }
+    }
+    return true;
+  }
 }
 
 export class ElementBinderBuilder {
diff --git a/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts b/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts
@@ -70,6 +70,25 @@ export function main() {
              });
        }));
 
+    it('should not create LightDom instances if the host element is empty',
+       inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
+         tb.compileAll([
+             someComponent,
+             new ViewDefinition({
+                 componentId: 'someComponent', template: '<some-comp>  <!-- comment -->\n </some-comp>',
+                 directives: [someComponent]
+             })
+           ])
+             .then((protoViewDtos) => {
+               var rootView = tb.createRootView(protoViewDtos[0]);
+               var cmpView = tb.createComponentView(rootView.viewRef, 0, protoViewDtos[1]);
+               expect(cmpView.rawView.proto.elementBinders[0].componentId).toBe('someComponent');
+               expect(cmpView.rawView.boundElements[0].lightDom).toBe(null);
+
+               async.done();
+             });
+       }));
+
     it('should update text nodes', inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
          tb.compileAll([
              someComponent,
diff --git a/modules/angular2/test/render/dom/view/view_spec.ts b/modules/angular2/test/render/dom/view/view_spec.ts
@@ -48,20 +48,24 @@ export function main() {
       return new DomView(pv, [DOM.childNodes(root)[0]], [], boundElements);
     }
 
+    function createElementBinder(parentIndex:number = 0, distanceToParent:number = 1) {
+      return new ElementBinder({parentIndex: parentIndex, distanceToParent:distanceToParent, textNodeIndices:[]});
+    }
+
     describe('getDirectParentElement', () => {
 
       it('should return the DomElement of the direct parent', () => {
         var pv = createProtoView(
-            [new ElementBinder(), new ElementBinder({parentIndex: 0, distanceToParent: 1})]);
+            [createElementBinder(), createElementBinder(0, 1)]);
         var view = createView(pv, 2);
         expect(view.getDirectParentElement(1)).toBe(view.boundElements[0]);
       });
 
       it('should return null if the direct parent is not bound', () => {
         var pv = createProtoView([
-          new ElementBinder(),
-          new ElementBinder(),
-          new ElementBinder({parentIndex: 0, distanceToParent: 2})
+          createElementBinder(),
+          createElementBinder(),
+          createElementBinder(0,2)
         ]);
         var view = createView(pv, 3);
         expect(view.getDirectParentElement(2)).toBe(null);
