diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 3fcfd2a99a..d6ee59f71a 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/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); diff --git a/packages/core/test/render3/common_integration_spec.ts b/packages/core/test/render3/common_integration_spec.ts index d09acb934a..c78872fa1d 100644 --- a/packages/core/test/render3/common_integration_spec.ts +++ b/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(''); }); + + 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']], + // + // + 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) { + 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(''); + + // this will fill the list + fixture.component.toggle(); + fixture.update(); + expect(fixture.html) + .toEqual(''); + + button.click(); + fixture.update(); + + expect(fixture.html).toEqual(''); + + button.click(); + fixture.update(); + expect(fixture.html) + .toEqual(''); + }); }); describe('ngIf', () => {