From ca0970134396fb863db58011aedc66e5bcb64020 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Wed, 10 Jun 2015 15:54:10 -0700 Subject: [PATCH] perf(render): only create `LightDom` instances if the element has children --- .../angular2/src/render/dom/dom_renderer.ts | 10 +++++--- .../src/render/dom/view/element_binder.ts | 7 ++++-- .../src/render/dom/view/proto_view_builder.ts | 23 ++++++++++++++++--- .../dom/dom_renderer_integration_spec.ts | 19 +++++++++++++++ .../test/render/dom/view/view_spec.ts | 12 ++++++---- 5 files changed, 59 insertions(+), 12 deletions(-) diff --git a/modules/angular2/src/render/dom/dom_renderer.ts b/modules/angular2/src/render/dom/dom_renderer.ts index 05655a1a8c..29d4d2c3aa 100644 --- 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 index 248a8844eb..c9e78d5a0d 100644 --- 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; hostActions: Map; + elementIsEmpty: boolean; constructor({textNodeIndices, contentTagSelector, nestedProtoView, componentId, eventLocals, localEvents, globalEvents, hostActions, parentIndex, distanceToParent, - propertySetters}: { + propertySetters, elementIsEmpty}: { contentTagSelector?: string, textNodeIndices?: List, nestedProtoView?: protoViewModule.DomProtoView, @@ -29,7 +30,8 @@ export class ElementBinder { parentIndex?: number, distanceToParent?: number, propertySetters?: Map, - hostActions?: Map + hostActions?: Map, + 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 index d838167ff7..fb79833566 100644 --- 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 index 508af8bf97..360dc15db0 100644 --- 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: ' \n ', + 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 index 3d3f6f82cc..4eeef9669c 100644 --- 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);