From 5ebc0da640f9e654b3111a0e33ce7844fe884605 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Wed, 30 Jan 2019 12:18:27 +0100 Subject: [PATCH] fix(ivy): remove query results from destroyed embedded views (#28445) PR Close #28445 --- .../core/src/render3/node_manipulation.ts | 22 ++++++++++++++---- packages/core/src/render3/util.ts | 3 ++- .../test/linker/query_integration_spec.ts | 23 +++++++++---------- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 4fb1b6ce057b6..d7faf85de5550 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -242,14 +242,17 @@ export function destroyViewTree(rootView: LView): void { while (viewOrContainer) { let next: LView|LContainer|null = null; - if (viewOrContainer.length >= HEADER_OFFSET) { + if (isLContainer(viewOrContainer)) { + // If container, traverse down to its first LView. + const container = viewOrContainer as LContainer; + const viewsInContainer = container[VIEWS]; + if (viewsInContainer.length) { + next = viewsInContainer[0]; + } + } else { // If LView, traverse down to child. const view = viewOrContainer as LView; if (view[TVIEW].childIndex > -1) next = getLViewChild(view); - } else { - // If container, traverse down to its first LView. - const container = viewOrContainer as LContainer; - if (container[VIEWS].length) next = container[VIEWS][0]; } if (next == null) { @@ -258,6 +261,15 @@ export function destroyViewTree(rootView: LView): void { while (viewOrContainer && !viewOrContainer ![NEXT] && viewOrContainer !== rootView) { cleanUpView(viewOrContainer); viewOrContainer = getParentState(viewOrContainer, rootView); + if (isLContainer(viewOrContainer)) { + // this view will be destroyed so we need to notify queries that a view is detached + const viewsInContainer = (viewOrContainer as LContainer)[VIEWS]; + for (let viewToDetach of viewsInContainer) { + if (viewToDetach[QUERIES]) { + viewToDetach[QUERIES] !.removeView(); + } + } + } } cleanUpView(viewOrContainer || rootView); next = viewOrContainer && viewOrContainer ![NEXT]; diff --git a/packages/core/src/render3/util.ts b/packages/core/src/render3/util.ts index a90a6c3001b5a..3f67b07d38536 100644 --- a/packages/core/src/render3/util.ts +++ b/packages/core/src/render3/util.ts @@ -127,7 +127,8 @@ export function isComponentDef(def: DirectiveDef): def is ComponentDef return (def as ComponentDef).template !== null; } -export function isLContainer(value: RElement | RComment | LContainer | StylingContext): boolean { +export function isLContainer( + value: RElement | RComment | LContainer | LView | StylingContext | null): boolean { // Styling contexts are also arrays, but their first index contains an element node return Array.isArray(value) && value.length === LCONTAINER_LENGTH; } diff --git a/packages/core/test/linker/query_integration_spec.ts b/packages/core/test/linker/query_integration_spec.ts index fcaf05e908703..4b4e32f1944d8 100644 --- a/packages/core/test/linker/query_integration_spec.ts +++ b/packages/core/test/linker/query_integration_spec.ts @@ -633,25 +633,24 @@ describe('Query API', () => { expect(q.query.map((d: TextDirective) => d.text)).toEqual(['2', '1']); }); - fixmeIvy('FW-920: Queries in nested views are not destroyed properly') - .it('should remove manually projected templates if their parent view is destroyed', () => { - const template = ` + it('should remove manually projected templates if their parent view is destroyed', () => { + const template = `
`; - const view = createTestCmp(MyComp0, template); - const q = view.debugElement.children[0].references !['q']; - view.componentInstance.shouldShow = true; - view.detectChanges(); + const view = createTestCmp(MyComp0, template); + const q = view.debugElement.children[0].references !['q']; + view.componentInstance.shouldShow = true; + view.detectChanges(); - expect(q.query.length).toBe(1); + expect(q.query.length).toBe(1); - view.componentInstance.shouldShow = false; - view.detectChanges(); - expect(q.query.length).toBe(0); - }); + view.componentInstance.shouldShow = false; + view.detectChanges(); + expect(q.query.length).toBe(0); + }); modifiedInIvy('https://github.com/angular/angular/issues/15117 fixed in ivy') .it('should not throw if a content template is queried and created in the view during change detection',