Skip to content

Commit c1fb9c2

Browse files
karajasonaden
authored andcommitted
fix(ivy): save queries at the correct indices (angular#28327)
Previous to this change, we were storing view queries at the wrong index. This is because we were passing a raw index to the store() instruction instead of an adjusted index (i.e. an index that does not include the HEADER_OFFSET). We had an additional issue where TView.blueprint was not backfilled when TView.data was backfilled, so new component instances created from the blueprint would end up having a shorter LView. Both of these problems together led to the Material demo app failing with Ivy. This commit fixes those discrepancies. PR Close angular#28327
1 parent d4ecffe commit c1fb9c2

File tree

6 files changed

+107
-67
lines changed

6 files changed

+107
-67
lines changed

packages/core/src/render3/instructions.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2930,6 +2930,7 @@ export function store<T>(index: number, value: T): void {
29302930
const adjustedIndex = index + HEADER_OFFSET;
29312931
if (adjustedIndex >= tView.data.length) {
29322932
tView.data[adjustedIndex] = null;
2933+
tView.blueprint[adjustedIndex] = null;
29332934
}
29342935
lView[adjustedIndex] = value;
29352936
}
@@ -3069,4 +3070,4 @@ function getTViewCleanup(view: LView): any[] {
30693070
function loadComponentRenderer(tNode: TNode, lView: LView): Renderer3 {
30703071
const componentLView = lView[tNode.index] as LView;
30713072
return componentLView[RENDERER];
3072-
}
3073+
}

packages/core/src/render3/query.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {unusedValueExportToPlacateAjd as unused1} from './interfaces/definition'
2323
import {unusedValueExportToPlacateAjd as unused2} from './interfaces/injector';
2424
import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, unusedValueExportToPlacateAjd as unused3} from './interfaces/node';
2525
import {LQueries, unusedValueExportToPlacateAjd as unused4} from './interfaces/query';
26-
import {LView, TVIEW} from './interfaces/view';
26+
import {HEADER_OFFSET, LView, TVIEW} from './interfaces/view';
2727
import {getCurrentViewQueryIndex, getIsParent, getLView, getOrCreateCurrentQueries, setCurrentViewQueryIndex} from './state';
2828
import {isContentQueryHost} from './util';
2929
import {createElementRef, createTemplateRef} from './view_engine_compatibility';
@@ -407,7 +407,7 @@ export function viewQuery<T>(
407407
}
408408
const index = getCurrentViewQueryIndex();
409409
const viewQuery: QueryList<T> = query<T>(predicate, descend, read);
410-
store(index, viewQuery);
410+
store(index - HEADER_OFFSET, viewQuery);
411411
setCurrentViewQueryIndex(index + 1);
412412
return viewQuery;
413413
}
@@ -418,5 +418,5 @@ export function viewQuery<T>(
418418
export function loadViewQuery<T>(): T {
419419
const index = getCurrentViewQueryIndex();
420420
setCurrentViewQueryIndex(index + 1);
421-
return load<T>(index);
422-
}
421+
return load<T>(index - HEADER_OFFSET);
422+
}

packages/core/test/render3/inherit_definition_feature_spec.ts

Lines changed: 93 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import {Inject, InjectionToken, QueryList} from '../../src/core';
1010
import {ComponentDef, DirectiveDef, InheritDefinitionFeature, NgOnChangesFeature, ProvidersFeature, RenderFlags, allocHostVars, bind, defineBase, defineComponent, defineDirective, directiveInject, element, elementProperty, loadViewQuery, queryRefresh, viewQuery} from '../../src/render3/index';
1111

12-
import {ComponentFixture, createComponent} from './render_util';
12+
import {ComponentFixture, createComponent, getDirectiveOnNode} from './render_util';
1313

1414
describe('InheritDefinitionFeature', () => {
1515
it('should inherit lifecycle hooks', () => {
@@ -363,48 +363,10 @@ describe('InheritDefinitionFeature', () => {
363363
expect(divEl.title).toEqual('new-title');
364364
});
365365

366-
it('should compose viewQuery (basic mechanics check)', () => {
367-
const log: Array<[string, RenderFlags, any]> = [];
366+
describe('view query', () => {
368367

369-
class SuperComponent {
370-
static ngComponentDef = defineComponent({
371-
type: SuperComponent,
372-
template: () => {},
373-
consts: 0,
374-
vars: 0,
375-
selectors: [['', 'superDir', '']],
376-
viewQuery: <T>(rf: RenderFlags, ctx: T) => {
377-
log.push(['super', rf, ctx]);
378-
},
379-
factory: () => new SuperComponent(),
380-
});
381-
}
382-
383-
class SubComponent extends SuperComponent {
384-
static ngComponentDef = defineComponent({
385-
type: SubComponent,
386-
template: () => {},
387-
consts: 0,
388-
vars: 0,
389-
selectors: [['', 'subDir', '']],
390-
viewQuery: (directiveIndex: number, elementIndex: number) => {
391-
log.push(['sub', directiveIndex, elementIndex]);
392-
},
393-
factory: () => new SubComponent(),
394-
features: [InheritDefinitionFeature]
395-
});
396-
}
397-
398-
const subDef = SubComponent.ngComponentDef as ComponentDef<any>;
399-
400-
const context = {foo: 'bar'};
401-
402-
subDef.viewQuery !(1, context);
403-
404-
expect(log).toEqual([['super', 1, context], ['sub', 1, context]]);
405-
});
368+
const SomeComp = createComponent('some-comp', (rf: RenderFlags, ctx: any) => {});
406369

407-
it('should compose viewQuery (query logic check)', () => {
408370
/*
409371
* class SuperComponent {
410372
* @ViewChildren('super') superQuery;
@@ -417,7 +379,7 @@ describe('InheritDefinitionFeature', () => {
417379
template: () => {},
418380
consts: 0,
419381
vars: 0,
420-
selectors: [['', 'superDir', '']],
382+
selectors: [['super-comp']],
421383
viewQuery: <T>(rf: RenderFlags, ctx: any) => {
422384
if (rf & RenderFlags.Create) {
423385
viewQuery(['super'], false);
@@ -435,6 +397,7 @@ describe('InheritDefinitionFeature', () => {
435397
/**
436398
* <div id="sub" #sub></div>
437399
* <div id="super" #super></div>
400+
* <some-comp></some-comp>
438401
* class SubComponent extends SuperComponent {
439402
* @ViewChildren('sub') subQuery;
440403
* }
@@ -447,11 +410,12 @@ describe('InheritDefinitionFeature', () => {
447410
if (rf & RenderFlags.Create) {
448411
element(0, 'div', ['id', 'sub'], ['sub', '']);
449412
element(2, 'div', ['id', 'super'], ['super', '']);
413+
element(4, 'some-comp');
450414
}
451415
},
452-
consts: 4,
416+
consts: 5,
453417
vars: 0,
454-
selectors: [['', 'subDir', '']],
418+
selectors: [['sub-comp']],
455419
viewQuery: (rf: RenderFlags, ctx: any) => {
456420
if (rf & RenderFlags.Create) {
457421
viewQuery(['sub'], false);
@@ -463,23 +427,98 @@ describe('InheritDefinitionFeature', () => {
463427
}
464428
},
465429
factory: () => new SubComponent(),
466-
features: [InheritDefinitionFeature]
430+
features: [InheritDefinitionFeature],
431+
directives: [SomeComp]
467432
});
468433
}
469434

470-
const fixture = new ComponentFixture(SubComponent);
471435

472-
const check = (key: string): void => {
473-
const qList = (fixture.component as any)[`${key}Query`] as QueryList<any>;
474-
expect(qList.length).toBe(1);
475-
expect(qList.first.nativeElement).toEqual(fixture.hostElement.querySelector(`#${key}`));
476-
expect(qList.first.nativeElement.id).toEqual(key);
477-
};
436+
it('should compose viewQuery (basic mechanics check)', () => {
437+
const log: Array<[string, RenderFlags, any]> = [];
438+
439+
class SuperComponent {
440+
static ngComponentDef = defineComponent({
441+
type: SuperComponent,
442+
template: () => {},
443+
consts: 0,
444+
vars: 0,
445+
selectors: [['', 'superDir', '']],
446+
viewQuery: <T>(rf: RenderFlags, ctx: T) => {
447+
log.push(['super', rf, ctx]);
448+
},
449+
factory: () => new SuperComponent(),
450+
});
451+
}
452+
453+
class SubComponent extends SuperComponent {
454+
static ngComponentDef = defineComponent({
455+
type: SubComponent,
456+
template: () => {},
457+
consts: 0,
458+
vars: 0,
459+
selectors: [['', 'subDir', '']],
460+
viewQuery: (directiveIndex: number, elementIndex: number) => {
461+
log.push(['sub', directiveIndex, elementIndex]);
462+
},
463+
factory: () => new SubComponent(),
464+
features: [InheritDefinitionFeature]
465+
});
466+
}
467+
468+
const subDef = SubComponent.ngComponentDef as ComponentDef<any>;
469+
470+
const context = {foo: 'bar'};
471+
472+
subDef.viewQuery !(1, context);
473+
474+
expect(log).toEqual([['super', 1, context], ['sub', 1, context]]);
475+
});
476+
477+
478+
479+
it('should compose viewQuery (query logic check)', () => {
480+
const fixture = new ComponentFixture(SubComponent);
481+
482+
const check = (key: string): void => {
483+
const qList = (fixture.component as any)[`${key}Query`] as QueryList<any>;
484+
expect(qList.length).toBe(1);
485+
expect(qList.first.nativeElement).toEqual(fixture.hostElement.querySelector(`#${key}`));
486+
expect(qList.first.nativeElement.id).toEqual(key);
487+
};
488+
489+
check('sub');
490+
check('super');
491+
});
492+
493+
it('should work with multiple viewQuery comps', () => {
494+
let subCompOne !: SubComponent;
495+
let subCompTwo !: SubComponent;
496+
497+
const App = createComponent('app', (rf: RenderFlags, ctx: any) => {
498+
if (rf & RenderFlags.Create) {
499+
element(0, 'sub-comp');
500+
element(1, 'sub-comp');
501+
}
502+
subCompOne = getDirectiveOnNode(0);
503+
subCompTwo = getDirectiveOnNode(1);
504+
}, 2, 0, [SubComponent, SuperComponent]);
505+
506+
const fixture = new ComponentFixture(App);
507+
508+
const check = (comp: SubComponent): void => {
509+
const qList = comp.subQuery as QueryList<any>;
510+
expect(qList.length).toBe(1);
511+
expect(qList.first.nativeElement).toEqual(fixture.hostElement.querySelector('#sub'));
512+
expect(qList.first.nativeElement.id).toEqual('sub');
513+
};
514+
515+
check(subCompOne);
516+
check(subCompTwo);
517+
});
478518

479-
check('sub');
480-
check('super');
481519
});
482520

521+
483522
it('should compose contentQueries', () => {
484523
const log: string[] = [];
485524

packages/core/test/render3/query_spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,14 +1305,14 @@ describe('query', () => {
13051305
0, Cmpt_Template_1, 2, 0, 'ng-template', null, ['foo', ''],
13061306
templateRefExtractor);
13071307
template(
1308-
1, Cmpt_Template_1, 2, 0, 'ng-template', null, ['bar', ''],
1308+
2, Cmpt_Template_1, 2, 0, 'ng-template', null, ['bar', ''],
13091309
templateRefExtractor);
13101310
template(
1311-
2, Cmpt_Template_1, 2, 0, 'ng-template', null, ['baz', ''],
1311+
4, Cmpt_Template_1, 2, 0, 'ng-template', null, ['baz', ''],
13121312
templateRefExtractor);
13131313
}
13141314
},
1315-
3, 0, [], [],
1315+
6, 0, [], [],
13161316
function(rf: RenderFlags, ctx: any) {
13171317
if (rf & RenderFlags.Create) {
13181318
viewQuery(TemplateRef as any, false);
@@ -2160,7 +2160,7 @@ describe('query', () => {
21602160
element(0, 'div', null, ['foo', '']);
21612161
}
21622162
},
2163-
1, 0, [], [],
2163+
2, 0, [], [],
21642164
function(rf: RenderFlags, ctx: any) {
21652165
if (rf & RenderFlags.Create) {
21662166
viewQuery(['foo'], false);

packages/core/test/render3/styling/players_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,8 @@ class SuperComp {
279279
vars: 0,
280280
template: (rf: RenderFlags, ctx: SuperComp) => {
281281
if (rf & RenderFlags.Create) {
282-
elementStart(1, 'div');
283-
element(2, 'child-comp', ['child', ''], ['child', 'child']);
282+
elementStart(0, 'div');
283+
element(1, 'child-comp', ['child', ''], ['child', 'child']);
284284
elementEnd();
285285
}
286286
},

packages/core/test/render3/view_container_ref_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2060,10 +2060,10 @@ describe('ViewContainerRef', () => {
20602060
vars: 0,
20612061
template: (rf: RenderFlags, ctx: DynamicCompWithViewQueries) => {
20622062
if (rf & RenderFlags.Create) {
2063-
element(1, 'div', ['bar', ''], ['foo', '']);
2063+
element(0, 'div', ['bar', ''], ['foo', '']);
20642064
}
20652065
// testing only
2066-
fooEl = getNativeByIndex(1, getLView());
2066+
fooEl = getNativeByIndex(0, getLView());
20672067
},
20682068
viewQuery: function(rf: RenderFlags, ctx: any) {
20692069
if (rf & RenderFlags.Create) {

0 commit comments

Comments
 (0)