Skip to content

Commit

Permalink
fix(ivy): correctly handle queries with embedded views (#24418)
Browse files Browse the repository at this point in the history
This PR takes care of all the remaining cases where embedded view definition
and insertion points are different.

PR Close #24418
  • Loading branch information
pkozlowski-opensource authored and mhevery committed Jun 11, 2018
1 parent 5e8bf2f commit 014949f
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 12 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/render3/interfaces/query.ts
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/node_manipulation.ts
Expand Up @@ -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;
Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/render3/query.ts
Expand Up @@ -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');
Expand Down
88 changes: 80 additions & 8 deletions packages/core/test/render3/query_spec.ts
Expand Up @@ -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;
}
});

Expand All @@ -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', () => {

Expand Down Expand Up @@ -873,16 +875,16 @@ 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();
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');

directiveInstance !.insertTpl(tpl1 !, {idx: 1}, 1);
directiveInstances[0].insertTpl(tpl1 !, {idx: 1}, 1);
fixture.update();
expect(qList.length).toBe(4);
qListArr = qList.toArray();
Expand All @@ -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<{}>;

/**
* <ng-template #tpl let-idx="idx" let-container_idx="container_idx">
* <div #foo [id]="'foo_'+container_idx+'_'+idx"></div>
* </ng-template>
*
* <ng-template viewInserter #vi1="vi"></ng-template>
* <ng-template viewInserter #vi2="vi"></ng-template>
*/
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<QueryList<any>>(0)) && (ctx.query = tmp as QueryList<any>);
}

}, [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);
});
});

Expand Down

0 comments on commit 014949f

Please sign in to comment.