From 014949f74c0b736f62622157cece6d2e07db753e Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Mon, 11 Jun 2018 11:51:16 +0200 Subject: [PATCH] fix(ivy): correctly handle queries with embedded views (#24418) This PR takes care of all the remaining cases where embedded view definition and insertion points are different. PR Close #24418 --- packages/core/src/render3/interfaces/query.ts | 2 +- .../core/src/render3/node_manipulation.ts | 2 +- packages/core/src/render3/query.ts | 7 +- packages/core/test/render3/query_spec.ts | 88 +++++++++++++++++-- 4 files changed, 87 insertions(+), 12 deletions(-) diff --git a/packages/core/src/render3/interfaces/query.ts b/packages/core/src/render3/interfaces/query.ts index 93fe026799..331b3be81f 100644 --- a/packages/core/src/render3/interfaces/query.ts +++ b/packages/core/src/render3/interfaces/query.ts @@ -49,7 +49,7 @@ export interface LQueries { * Notify `LQueries` that an `LView` has been removed from `LContainer`. As a result all * the matching nodes from this view should be removed from container's queries. */ - removeView(removeIndex: number): void; + removeView(): void; /** * Add additional `QueryList` to track. diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index c695f43457..a04d6359ec 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -393,7 +393,7 @@ export function detachView(container: LContainerNode, removeIndex: number): LVie // Notify query that view has been removed const removedLview = viewNode.data; if (removedLview[QUERIES]) { - removedLview[QUERIES] !.removeView(removeIndex); + removedLview[QUERIES] !.removeView(); } // Unsets the attached flag viewNode.data[FLAGS] &= ~LViewFlags.Attached; diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index f2cdec8a15..241df49d51 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -176,13 +176,16 @@ export class LQueries_ implements LQueries { add(this.deep, node); } - removeView(index: number): void { + removeView(): void { let query = this.deep; while (query) { ngDevMode && assertDefined( query.containerValues, 'View queries need to have a pointer to container values.'); - const removed = query.containerValues !.splice(index, 1); + + const containerValues = query.containerValues !; + const viewValuesIdx = containerValues.indexOf(query.values); + const removed = containerValues.splice(viewValuesIdx, 1); // mark a query as dirty only when removed view had matching modes ngDevMode && assertEqual(removed.length, 1, 'removed.length'); diff --git a/packages/core/test/render3/query_spec.ts b/packages/core/test/render3/query_spec.ts index 7382fabfa3..9c87c9edeb 100644 --- a/packages/core/test/render3/query_spec.ts +++ b/packages/core/test/render3/query_spec.ts @@ -693,15 +693,17 @@ describe('query', () => { describe('ViewContainerRef', () => { - let directiveInstance: ViewContainerManipulatorDirective|null = null; + let directiveInstances: ViewContainerManipulatorDirective[] = []; class ViewContainerManipulatorDirective { static ngDirectiveDef = defineDirective({ type: ViewContainerManipulatorDirective, selectors: [['', 'vc', '']], factory: () => { - return directiveInstance = - new ViewContainerManipulatorDirective(injectViewContainerRef()); + const directiveInstance = + new ViewContainerManipulatorDirective(injectViewContainerRef()); + directiveInstances.push(directiveInstance); + return directiveInstance; } }); @@ -714,7 +716,7 @@ describe('query', () => { remove(index?: number) { this._vcRef.remove(index); } } - beforeEach(() => { directiveInstance = null; }); + beforeEach(() => { directiveInstances = []; }); it('should report results in views inserted / removed by ngIf', () => { @@ -873,8 +875,8 @@ describe('query', () => { expect(qList.length).toBe(1); expect(qList.first.nativeElement.getAttribute('id')).toBe('middle'); - directiveInstance !.insertTpl(tpl1 !, {idx: 0}, 0); - directiveInstance !.insertTpl(tpl2 !, {idx: 1}, 1); + directiveInstances[0].insertTpl(tpl1 !, {idx: 0}, 0); + directiveInstances[0].insertTpl(tpl2 !, {idx: 1}, 1); fixture.update(); expect(qList.length).toBe(3); let qListArr = qList.toArray(); @@ -882,7 +884,7 @@ describe('query', () => { expect(qListArr[1].nativeElement.getAttribute('id')).toBe('middle'); expect(qListArr[2].nativeElement.getAttribute('id')).toBe('foo2_1'); - directiveInstance !.insertTpl(tpl1 !, {idx: 1}, 1); + directiveInstances[0].insertTpl(tpl1 !, {idx: 1}, 1); fixture.update(); expect(qList.length).toBe(4); qListArr = qList.toArray(); @@ -891,13 +893,83 @@ describe('query', () => { expect(qListArr[2].nativeElement.getAttribute('id')).toBe('middle'); expect(qListArr[3].nativeElement.getAttribute('id')).toBe('foo2_1'); - directiveInstance !.remove(1); + directiveInstances[0].remove(1); fixture.update(); expect(qList.length).toBe(3); qListArr = qList.toArray(); expect(qListArr[0].nativeElement.getAttribute('id')).toBe('foo1_0'); expect(qListArr[1].nativeElement.getAttribute('id')).toBe('middle'); expect(qListArr[2].nativeElement.getAttribute('id')).toBe('foo2_1'); + + directiveInstances[0].remove(1); + fixture.update(); + expect(qList.length).toBe(2); + qListArr = qList.toArray(); + expect(qListArr[0].nativeElement.getAttribute('id')).toBe('foo1_0'); + expect(qListArr[1].nativeElement.getAttribute('id')).toBe('middle'); + }); + + // https://stackblitz.com/edit/angular-7vvo9j?file=src%2Fapp%2Fapp.component.ts + it('should report results when the same TemplateRef is inserted into different ViewContainerRefs', + () => { + let tpl: TemplateRef<{}>; + + /** + * + *
+ * + * + * + * + */ + const Cmpt = createComponent('cmpt', function(rf: RenderFlags, ctx: any) { + let tmp: any; + if (rf & RenderFlags.Create) { + query(0, ['foo'], true, QUERY_READ_FROM_NODE); + + container(1, (rf: RenderFlags, ctx: {idx: number, container_idx: number}) => { + if (rf & RenderFlags.Create) { + elementStart(0, 'div', null, ['foo', '']); + elementEnd(); + } + if (rf & RenderFlags.Update) { + elementProperty(0, 'id', bind('foo_' + ctx.container_idx + '_' + ctx.idx)); + } + }, null, []); + + container(2, undefined, null, [AttributeMarker.SELECT_ONLY, 'vc']); + container(3, undefined, null, [AttributeMarker.SELECT_ONLY, 'vc']); + } + + if (rf & RenderFlags.Update) { + tpl = getOrCreateTemplateRef(getOrCreateNodeInjectorForNode(load(1))); + queryRefresh(tmp = load>(0)) && (ctx.query = tmp as QueryList); + } + + }, [ViewContainerManipulatorDirective]); + + const fixture = new ComponentFixture(Cmpt); + const qList = fixture.component.query; + + expect(qList.length).toBe(0); + + directiveInstances[0].insertTpl(tpl !, {idx: 0, container_idx: 0}, 0); + directiveInstances[1].insertTpl(tpl !, {idx: 0, container_idx: 1}, 0); + fixture.update(); + expect(qList.length).toBe(2); + let qListArr = qList.toArray(); + expect(qListArr[0].nativeElement.getAttribute('id')).toBe('foo_1_0'); + expect(qListArr[1].nativeElement.getAttribute('id')).toBe('foo_0_0'); + + directiveInstances[0].remove(); + fixture.update(); + expect(qList.length).toBe(1); + qListArr = qList.toArray(); + expect(qListArr[0].nativeElement.getAttribute('id')).toBe('foo_1_0'); + + directiveInstances[1].remove(); + fixture.update(); + expect(qList.length).toBe(0); }); });