fix(ivy): properly destroy view trees where root is an embedded view …

packages/core/src/render3/node_manipulation.ts

@@ -209,7 +209,7 @@ export function addRemoveViewFromContainer(
 }
 
 /**
- * Traverses the tree of component views and containers to remove listeners and
+ * Traverses down and up the tree of views and containers to remove listeners and
  * call onDestroy callbacks.
  *
  * Notes:
@@ -222,7 +222,11 @@ export function addRemoveViewFromContainer(
  *  @param rootView The view to destroy
  */
 export function destroyViewTree(rootView: LView): void {
-  let viewOrContainer: LViewOrLContainer|null = rootView;
+  // A view to cleanup doesn't have children so we should not try to descend down the view tree.
+  if (!rootView.child) {
+    return cleanUpView(rootView);
+  }
+  let viewOrContainer: LViewOrLContainer|null = rootView.child;
 
   while (viewOrContainer) {
     let next: LViewOrLContainer|null = null;
@@ -232,13 +236,16 @@ export function destroyViewTree(rootView: LView): void {
     } else if (viewOrContainer.child) {
       next = viewOrContainer.child;
     } else if (viewOrContainer.next) {
+      // Only move to the side and clean if operating below rootView -
+      // otherwise we would start cleaning up sibling views of the rootView.
       cleanUpView(viewOrContainer as LView);
       next = viewOrContainer.next;
     }
 
     if (next == null) {
-      // If the viewOrContainer is the rootView, then the cleanup is done twice.
-      // Without this check, ngOnDestroy would be called twice for a directive on an element.
+      // If the viewOrContainer is the rootView and next is null it means that we are dealing
+      // with a root view that doesn't have children. We didn't descend into child views
+      // so no need to go back up the views tree.
       while (viewOrContainer && !viewOrContainer !.next && viewOrContainer !== rootView) {
         cleanUpView(viewOrContainer as LView);
         viewOrContainer = getParentState(viewOrContainer, rootView);

packages/core/test/render3/common_integration_spec.ts

@@ -9,8 +9,9 @@
 import {NgForOfContext} from '@angular/common';
 
 import {defineComponent} from '../../src/render3/index';
-import {bind, container, elementEnd, elementProperty, elementStart, interpolation3, text, textBinding, tick} from '../../src/render3/instructions';
+import {bind, container, elementEnd, elementProperty, elementStart, interpolation1, interpolation3, listener, text, textBinding, tick} from '../../src/render3/instructions';
 import {RenderFlags} from '../../src/render3/interfaces/definition';
+
 import {NgForOf, NgIf} from './common_with_def';
 import {ComponentFixture} from './render_util';
 
@@ -123,6 +124,81 @@ describe('@angular/common integration', () => {
           .toEqual('<ul><li>0 of 3: first</li><li>1 of 3: middle</li><li>2 of 3: second</li></ul>');
     });
 
+
+    it('should retain parent view listeners when the NgFor destroy views', () => {
+
+      class MyApp {
+        private _data: number[] = [1, 2, 3];
+        items: number[] = [];
+
+        toggle() {
+          if (this.items.length) {
+            this.items = [];
+          } else {
+            this.items = this._data;
+          }
+        }
+
+        static ngComponentDef = defineComponent({
+          type: MyApp,
+          factory: () => new MyApp(),
+          selectors: [['my-app']],
+          // <button (click)="toggle()">Toggle List</button>
+          // <ul>
+          //   <li *ngFor="let item of items">{{index}}</li>
+          // </ul>
+          template: (rf: RenderFlags, myApp: MyApp) => {
+            if (rf & RenderFlags.Create) {
+              elementStart(0, 'button');
+              {
+                listener('click', function() { return myApp.toggle(); });
+                text(1, 'Toggle List');
+              }
+              elementEnd();
+              elementStart(2, 'ul');
+              { container(3, liTemplate, undefined, ['ngForOf', '']); }
+              elementEnd();
+            }
+            if (rf & RenderFlags.Update) {
+              elementProperty(3, 'ngForOf', bind(myApp.items));
+            }
+
+            function liTemplate(rf1: RenderFlags, row: NgForOfContext<string>) {
+              if (rf1 & RenderFlags.Create) {
+                elementStart(0, 'li');
+                { text(1); }
+                elementEnd();
+              }
+              if (rf1 & RenderFlags.Update) {
+                textBinding(1, interpolation1('', row.$implicit, ''));
+              }
+            }
+          },
+          directives: () => [NgForOf]
+        });
+      }
+
+      const fixture = new ComponentFixture(MyApp);
+      const button = fixture.hostElement.querySelector('button') !;
+
+      expect(fixture.html).toEqual('<button>Toggle List</button><ul></ul>');
+
+      // this will fill the list
+      fixture.component.toggle();
+      fixture.update();
+      expect(fixture.html)
+          .toEqual('<button>Toggle List</button><ul><li>1</li><li>2</li><li>3</li></ul>');
+
+      button.click();
+      fixture.update();
+
+      expect(fixture.html).toEqual('<button>Toggle List</button><ul></ul>');
+
+      button.click();
+      fixture.update();
+      expect(fixture.html)
+          .toEqual('<button>Toggle List</button><ul><li>1</li><li>2</li><li>3</li></ul>');
+    });
   });
 
   describe('ngIf', () => {