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

…without children

The bug fixed here steams from the fact that we are traversing too far up
in the views tree hierarchy in the destroyViewTree function.

The logic in destroyViewTree is off if we start removal at an embedded view
without any child views. For such a case we should just clean up (cleanUpView)
this one view without paying attention to next / parent views.

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:
@@ -231,21 +231,24 @@ export function destroyViewTree(rootView: LView): void {
       next = viewOrContainer.views[0].data;
     } else if (viewOrContainer.child) {
       next = viewOrContainer.child;
-    } else if (viewOrContainer.next) {
+    } else if (viewOrContainer.next && viewOrContainer !== rootView) {
+      // 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 descent 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);
       }
       cleanUpView(viewOrContainer as LView || rootView);
 
-      next = viewOrContainer && viewOrContainer.next;
+      next = viewOrContainer !== rootView && viewOrContainer ? viewOrContainer.next : null;
     }
     viewOrContainer = next;
   }

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}} of {{count}}: {{item}}</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', () => {