From 71b9d5539ba0bc307477bf5f0e9a31f5792fab85 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 | 24 ++++++++++++++----- packages/core/src/render3/util.ts | 3 ++- .../test/linker/query_integration_spec.ts | 23 +++++++++--------- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index dce6ae1d66..5e857aa766 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -11,7 +11,7 @@ import {attachPatchData} from './context_discovery'; import {callHooks} from './hooks'; import {LContainer, NATIVE, VIEWS, unusedValueExportToPlacateAjd as unused1} from './interfaces/container'; import {ComponentDef} from './interfaces/definition'; -import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeFlags, TNodeType, TProjectionNode, TViewNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node'; +import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeFlags, TNodeType, TViewNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node'; import {unusedValueExportToPlacateAjd as unused3} from './interfaces/projection'; import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, isProceduralRenderer, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer'; import {CLEANUP, CONTAINER_INDEX, FLAGS, HEADER_OFFSET, HOST_NODE, HookData, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, TVIEW, unusedValueExportToPlacateAjd as unused5} from './interfaces/view'; @@ -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 a90a6c3001..3f67b07d38 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 fcaf05e908..4b4e32f194 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',