Skip to content

Commit 51a592c

Browse files
pkozlowski-opensourcejasonaden
authored andcommitted
fix(ivy): mark query as dirty upon view insertion (angular#28429)
PR Close angular#28429
1 parent fdc6e15 commit 51a592c

File tree

2 files changed

+48
-8
lines changed

2 files changed

+48
-8
lines changed

packages/core/src/render3/query.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,19 +191,21 @@ function copyQueriesToView(query: LQuery<any>| null): LQuery<any>|null {
191191

192192
function insertView(index: number, query: LQuery<any>| null) {
193193
while (query) {
194-
ngDevMode &&
195-
assertDefined(
196-
query.containerValues, 'View queries need to have a pointer to container values.');
194+
ngDevMode && assertViewQueryhasPointerToDeclarationContainer(query);
197195
query.containerValues !.splice(index, 0, query.values);
196+
197+
// mark a query as dirty only when inserted view had matching modes
198+
if (query.values.length) {
199+
query.list.setDirty();
200+
}
201+
198202
query = query.next;
199203
}
200204
}
201205

202206
function removeView(query: LQuery<any>| null) {
203207
while (query) {
204-
ngDevMode &&
205-
assertDefined(
206-
query.containerValues, 'View queries need to have a pointer to container values.');
208+
ngDevMode && assertViewQueryhasPointerToDeclarationContainer(query);
207209

208210
const containerValues = query.containerValues !;
209211
const viewValuesIdx = containerValues.indexOf(query.values);
@@ -219,6 +221,9 @@ function removeView(query: LQuery<any>| null) {
219221
}
220222
}
221223

224+
function assertViewQueryhasPointerToDeclarationContainer(query: LQuery<any>) {
225+
assertDefined(query.containerValues, 'View queries need to have a pointer to container values.');
226+
}
222227

223228
/**
224229
* Iterates over local names for a given node and returns directive index

packages/core/test/linker/query_integration_spec.ts

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, Component, ContentChild, ContentChildren, Directive, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef, asNativeElements} from '@angular/core';
1010
import {ComponentFixture, TestBed, async} from '@angular/core/testing';
1111
import {expect} from '@angular/platform-browser/testing/src/matchers';
12-
import {fixmeIvy, ivyEnabled, modifiedInIvy} from '@angular/private/testing';
12+
import {fixmeIvy, ivyEnabled, modifiedInIvy, onlyInIvy} from '@angular/private/testing';
1313
import {Subject} from 'rxjs';
1414

1515
import {stringify} from '../../src/util/stringify';
@@ -570,7 +570,7 @@ describe('Query API', () => {
570570
});
571571

572572
// Note: this test is just document our current behavior, which we do for performance reasons.
573-
fixmeIvy('FW-853: Query results are cleared if embedded views are detached / moved')
573+
modifiedInIvy('Query results from views are reported upon view insert / detach')
574574
.it('should not affect queries for projected templates if views are detached or moved',
575575
() => {
576576
const template = `<manual-projecting #q>
@@ -600,6 +600,41 @@ describe('Query API', () => {
600600
expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']);
601601
});
602602

603+
onlyInIvy('Query results from views are reported upon view insert / detach')
604+
.it('should update queries when a view is detached and re-inserted', () => {
605+
const template = `<manual-projecting #q>
606+
<ng-template let-x="x">
607+
<div [text]="x"></div>
608+
</ng-template>
609+
</manual-projecting>`;
610+
const view = createTestCmpAndDetectChanges(MyComp0, template);
611+
const q = view.debugElement.children[0].references !['q'] as ManualProjecting;
612+
expect(q.query.length).toBe(0);
613+
614+
const view1 = q.vc.createEmbeddedView(q.template, {'x': '1'});
615+
const view2 = q.vc.createEmbeddedView(q.template, {'x': '2'});
616+
617+
// 2 views were created and inserted so we've got 2 matching results
618+
view.detectChanges();
619+
expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']);
620+
621+
q.vc.detach(1);
622+
q.vc.detach(0);
623+
624+
// both views were detached so query results from those views should not be reported
625+
view.detectChanges();
626+
expect(q.query.map((d: TextDirective) => d.text)).toEqual([]);
627+
628+
q.vc.insert(view2);
629+
q.vc.insert(view1);
630+
631+
// previously detached views are re-inserted in the different order so:
632+
// - query results from the inserted views are reported again
633+
// - the order results from views reflects orders of views
634+
view.detectChanges();
635+
expect(q.query.map((d: TextDirective) => d.text)).toEqual(['2', '1']);
636+
});
637+
603638
fixmeIvy('FW-920: Queries in nested views are not destroyed properly')
604639
.it('should remove manually projected templates if their parent view is destroyed', () => {
605640
const template = `

0 commit comments

Comments
 (0)