fix(ViewContainer) removeChild called with null parent

divdavem · mhevery · commit bd48c927d0e6 · 2015-03-21T14:55:10.000-07:00
In view_container.js, templateElement.parentNode can be null when two template tags are nested in one another. Accessing the parent node through view.nodes[0].parentNode fixes the problem. closes angular#997 Closes angular#999
diff --git a/modules/angular2/src/core/compiler/view_container.js b/modules/angular2/src/core/compiler/view_container.js
@@ -130,7 +130,7 @@ export class ViewContainer {
     var detachedView = this.get(atIndex);
     ListWrapper.removeAt(this._views, atIndex);
     if (isBlank(this._lightDom)) {
-      ViewContainer.removeViewNodesFromParent(this.templateElement.parentNode, detachedView);
+      ViewContainer.removeViewNodes(detachedView);
     } else {
       this._lightDom.redistribute();
     }
@@ -173,8 +173,11 @@ export class ViewContainer {
     }
   }
 
-  static removeViewNodesFromParent(parent, view) {
-    for (var i = view.nodes.length - 1; i >= 0; --i) {
+  static removeViewNodes(view) {
+    var len = view.nodes.length;
+    if (len == 0) return;
+    var parent = view.nodes[0].parentNode;
+    for (var i = len - 1; i >= 0; --i) {
       DOM.removeChild(parent, view.nodes[i]);
     }
   }
diff --git a/modules/angular2/test/directives/for_spec.js b/modules/angular2/test/directives/for_spec.js
@@ -214,6 +214,31 @@ export function main() {
         cd.detectChanges();
         cd.detectChanges();
         expect(DOM.getText(view.nodes[0])).toEqual('a-2;b-2;|c-1;|');
+
+        component.items = [['e'], ['f', 'g']];
+        cd.detectChanges();
+        expect(DOM.getText(view.nodes[0])).toEqual('e-1;|f-2;g-2;|');
+
+        async.done();
+      });
+    }));
+
+    it('should repeat over nested arrays with no intermediate element', inject([AsyncTestCompleter], (async) => {
+      compileWithTemplate(
+          '<div><template [for] #item [of]="items">' +
+            '<div template="for #subitem of item">' +
+            '{{subitem}}-{{item.length}};' +
+          '</div></template></div>'
+      ).then((pv) => {
+        createView(pv);
+
+        component.items = [['a', 'b'], ['c']];
+        cd.detectChanges();
+        expect(DOM.getText(view.nodes[0])).toEqual('a-2;b-2;c-1;');
+
+        component.items = [['e'], ['f', 'g']];
+        cd.detectChanges();
+        expect(DOM.getText(view.nodes[0])).toEqual('e-1;f-2;g-2;');
         async.done();
       });
     }));
diff --git a/modules/angular2/test/directives/if_spec.js b/modules/angular2/test/directives/if_spec.js
@@ -116,6 +116,39 @@ export function main() {
       });
     }));
 
+    it('should handle nested if correctly', inject([AsyncTestCompleter], (async) => {
+      compileWithTemplate('<div><template [if]="booleanCondition"><copy-me *if="nestedBooleanCondition">hello</copy-me></template></div>').then((pv) => {
+        createView(pv);
+
+        component.booleanCondition = false;
+        cd.detectChanges();
+        expect(DOM.querySelectorAll(view.nodes[0], 'copy-me').length).toEqual(0);
+        expect(DOM.getText(view.nodes[0])).toEqual('');
+
+        component.booleanCondition = true;
+        cd.detectChanges();
+        expect(DOM.querySelectorAll(view.nodes[0], 'copy-me').length).toEqual(1);
+        expect(DOM.getText(view.nodes[0])).toEqual('hello');
+
+        component.nestedBooleanCondition = false;
+        cd.detectChanges();
+        expect(DOM.querySelectorAll(view.nodes[0], 'copy-me').length).toEqual(0);
+        expect(DOM.getText(view.nodes[0])).toEqual('');
+
+        component.nestedBooleanCondition = true;
+        cd.detectChanges();
+        expect(DOM.querySelectorAll(view.nodes[0], 'copy-me').length).toEqual(1);
+        expect(DOM.getText(view.nodes[0])).toEqual('hello');
+
+        component.booleanCondition = false;
+        cd.detectChanges();
+        expect(DOM.querySelectorAll(view.nodes[0], 'copy-me').length).toEqual(0);
+        expect(DOM.getText(view.nodes[0])).toEqual('');
+
+        async.done();
+      });
+    }));
+
     it('should update several nodes with if', inject([AsyncTestCompleter], (async) => {
       var templateString =
       '<div>' +
@@ -195,11 +228,13 @@ export function main() {
 @Component({selector: 'test-cmp'})
 class TestComponent {
   booleanCondition: boolean;
+  nestedBooleanCondition: boolean;
   numberCondition: number;
   stringCondition: string;
   functionCondition: Function;
   constructor() {
     this.booleanCondition = true;
+    this.nestedBooleanCondition = true;
     this.numberCondition = 1;
     this.stringCondition = "foo";
     this.functionCondition = function(s, n){

Original file line number	Diff line number	Diff line change
`@@ -130,7 +130,7 @@ export class ViewContainer {`
`130`	`130`	`var detachedView = this.get(atIndex);`
`131`	`131`	`ListWrapper.removeAt(this._views, atIndex);`
`132`	`132`	`if (isBlank(this._lightDom)) {`
`133`		`- ViewContainer.removeViewNodesFromParent(this.templateElement.parentNode, detachedView);`
	`133`	`+ ViewContainer.removeViewNodes(detachedView);`
`134`	`134`	`} else {`
`135`	`135`	`this._lightDom.redistribute();`
`136`	`136`	`}`
`@@ -173,8 +173,11 @@ export class ViewContainer {`
`173`	`173`	`}`
`174`	`174`	`}`
`175`	`175`
`176`		`- static removeViewNodesFromParent(parent, view) {`
`177`		`- for (var i = view.nodes.length - 1; i >= 0; --i) {`
	`176`	`+ static removeViewNodes(view) {`
	`177`	`+ var len = view.nodes.length;`
	`178`	`+ if (len == 0) return;`
	`179`	`+ var parent = view.nodes[0].parentNode;`
	`180`	`+ for (var i = len - 1; i >= 0; --i) {`
`178`	`181`	`DOM.removeChild(parent, view.nodes[i]);`
`179`	`182`	`}`
`180`	`183`	`}`