fix(renderer): handle empty fragments correctly

tbosch · tbosch · commit 61c73576c870 · 2015-07-16T16:18:58.000-07:00
Closes #3100
diff --git a/modules/angular2/src/render/dom/dom_renderer.ts b/modules/angular2/src/render/dom/dom_renderer.ts
@@ -83,8 +83,10 @@ export class DomRenderer extends Renderer {
   attachFragmentAfterFragment(previousFragmentRef: RenderFragmentRef,
                               fragmentRef: RenderFragmentRef) {
     var previousFragmentNodes = resolveInternalDomFragment(previousFragmentRef);
-    var sibling = previousFragmentNodes[previousFragmentNodes.length - 1];
-    moveNodesAfterSibling(sibling, resolveInternalDomFragment(fragmentRef));
+    if (previousFragmentNodes.length > 0) {
+      var sibling = previousFragmentNodes[previousFragmentNodes.length - 1];
+      moveNodesAfterSibling(sibling, resolveInternalDomFragment(fragmentRef));
+    }
   }
 
   attachFragmentAfterElement(elementRef: RenderElementRef, fragmentRef: RenderFragmentRef) {
@@ -253,7 +255,7 @@ export class DomRenderer extends Renderer {
 }
 
 function moveNodesAfterSibling(sibling, nodes) {
-  if (isPresent(DOM.parentElement(sibling))) {
+  if (nodes.length > 0 && isPresent(DOM.parentElement(sibling))) {
     for (var i = 0; i < nodes.length; i++) {
       DOM.insertBefore(sibling, nodes[i]);
     }
diff --git a/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts b/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts
@@ -197,6 +197,33 @@ export function main() {
              });
        }));
 
+    it('should add and remove empty fragments',
+       inject([AsyncTestCompleter, DomTestbed], (async, tb: DomTestbed) => {
+         tb.compileAndMerge(someComponent,
+                            [
+                              new ViewDefinition({
+                                componentId: 'someComponent',
+                                template: '<template></template><template></template>',
+                                directives: []
+                              })
+                            ])
+             .then((protoViewMergeMappings) => {
+               var rootView = tb.createView(protoViewMergeMappings[0]);
+
+               var elr = elRef(rootView.viewRef, 1);
+               expect(rootView.hostElement).toHaveText('');
+               var fragment = rootView.fragments[1];
+               var fragment2 = rootView.fragments[2];
+               tb.renderer.attachFragmentAfterElement(elr, fragment);
+               tb.renderer.attachFragmentAfterFragment(fragment, fragment2);
+               tb.renderer.detachFragment(fragment);
+               tb.renderer.detachFragment(fragment2);
+               expect(rootView.hostElement).toHaveText('');
+
+               async.done();
+             });
+       }));
+
     it('should handle events', inject([AsyncTestCompleter, DomTestbed], (async, tb: DomTestbed) => {
          tb.compileAndMerge(someComponent,
                             [

Original file line number	Diff line number	Diff line change
`@@ -83,8 +83,10 @@ export class DomRenderer extends Renderer {`
`83`	`83`	`attachFragmentAfterFragment(previousFragmentRef: RenderFragmentRef,`
`84`	`84`	`fragmentRef: RenderFragmentRef) {`
`85`	`85`	`var previousFragmentNodes = resolveInternalDomFragment(previousFragmentRef);`
`86`		`- var sibling = previousFragmentNodes[previousFragmentNodes.length - 1];`
`87`		`- moveNodesAfterSibling(sibling, resolveInternalDomFragment(fragmentRef));`
	`86`	`+ if (previousFragmentNodes.length > 0) {`
	`87`	`+ var sibling = previousFragmentNodes[previousFragmentNodes.length - 1];`
	`88`	`+ moveNodesAfterSibling(sibling, resolveInternalDomFragment(fragmentRef));`
	`89`	`+ }`
`88`	`90`	`}`
`89`	`91`
`90`	`92`	`attachFragmentAfterElement(elementRef: RenderElementRef, fragmentRef: RenderFragmentRef) {`
`@@ -253,7 +255,7 @@ export class DomRenderer extends Renderer {`
`253`	`255`	`}`
`254`	`256`
`255`	`257`	`function moveNodesAfterSibling(sibling, nodes) {`
`256`		`- if (isPresent(DOM.parentElement(sibling))) {`
	`258`	`+ if (nodes.length > 0 && isPresent(DOM.parentElement(sibling))) {`
`257`	`259`	`for (var i = 0; i < nodes.length; i++) {`
`258`	`260`	`DOM.insertBefore(sibling, nodes[i]);`
`259`	`261`	`}`