From c1fb9c265c6eee5ab169db514ccba0cf60df879a Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Wed, 23 Jan 2019 21:11:13 -0800 Subject: [PATCH] fix(ivy): save queries at the correct indices (#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 #28327 --- packages/core/src/render3/instructions.ts | 3 +- packages/core/src/render3/query.ts | 8 +- .../inherit_definition_feature_spec.ts | 147 +++++++++++------- packages/core/test/render3/query_spec.ts | 8 +- .../core/test/render3/styling/players_spec.ts | 4 +- .../test/render3/view_container_ref_spec.ts | 4 +- 6 files changed, 107 insertions(+), 67 deletions(-) diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index ea978e5120..a4e496bed0 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -2930,6 +2930,7 @@ export function store(index: number, value: T): void { const adjustedIndex = index + HEADER_OFFSET; if (adjustedIndex >= tView.data.length) { tView.data[adjustedIndex] = null; + tView.blueprint[adjustedIndex] = null; } lView[adjustedIndex] = value; } @@ -3069,4 +3070,4 @@ function getTViewCleanup(view: LView): any[] { function loadComponentRenderer(tNode: TNode, lView: LView): Renderer3 { const componentLView = lView[tNode.index] as LView; return componentLView[RENDERER]; -} \ No newline at end of file +} diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index 747acaa573..45dad1b18e 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -23,7 +23,7 @@ import {unusedValueExportToPlacateAjd as unused1} from './interfaces/definition' import {unusedValueExportToPlacateAjd as unused2} from './interfaces/injector'; import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, unusedValueExportToPlacateAjd as unused3} from './interfaces/node'; import {LQueries, unusedValueExportToPlacateAjd as unused4} from './interfaces/query'; -import {LView, TVIEW} from './interfaces/view'; +import {HEADER_OFFSET, LView, TVIEW} from './interfaces/view'; import {getCurrentViewQueryIndex, getIsParent, getLView, getOrCreateCurrentQueries, setCurrentViewQueryIndex} from './state'; import {isContentQueryHost} from './util'; import {createElementRef, createTemplateRef} from './view_engine_compatibility'; @@ -407,7 +407,7 @@ export function viewQuery( } const index = getCurrentViewQueryIndex(); const viewQuery: QueryList = query(predicate, descend, read); - store(index, viewQuery); + store(index - HEADER_OFFSET, viewQuery); setCurrentViewQueryIndex(index + 1); return viewQuery; } @@ -418,5 +418,5 @@ export function viewQuery( export function loadViewQuery(): T { const index = getCurrentViewQueryIndex(); setCurrentViewQueryIndex(index + 1); - return load(index); -} \ No newline at end of file + return load(index - HEADER_OFFSET); +} diff --git a/packages/core/test/render3/inherit_definition_feature_spec.ts b/packages/core/test/render3/inherit_definition_feature_spec.ts index 9e4e170e6f..b86faf755c 100644 --- a/packages/core/test/render3/inherit_definition_feature_spec.ts +++ b/packages/core/test/render3/inherit_definition_feature_spec.ts @@ -9,7 +9,7 @@ import {Inject, InjectionToken, QueryList} from '../../src/core'; import {ComponentDef, DirectiveDef, InheritDefinitionFeature, NgOnChangesFeature, ProvidersFeature, RenderFlags, allocHostVars, bind, defineBase, defineComponent, defineDirective, directiveInject, element, elementProperty, loadViewQuery, queryRefresh, viewQuery} from '../../src/render3/index'; -import {ComponentFixture, createComponent} from './render_util'; +import {ComponentFixture, createComponent, getDirectiveOnNode} from './render_util'; describe('InheritDefinitionFeature', () => { it('should inherit lifecycle hooks', () => { @@ -363,48 +363,10 @@ describe('InheritDefinitionFeature', () => { expect(divEl.title).toEqual('new-title'); }); - it('should compose viewQuery (basic mechanics check)', () => { - const log: Array<[string, RenderFlags, any]> = []; + describe('view query', () => { - class SuperComponent { - static ngComponentDef = defineComponent({ - type: SuperComponent, - template: () => {}, - consts: 0, - vars: 0, - selectors: [['', 'superDir', '']], - viewQuery: (rf: RenderFlags, ctx: T) => { - log.push(['super', rf, ctx]); - }, - factory: () => new SuperComponent(), - }); - } + const SomeComp = createComponent('some-comp', (rf: RenderFlags, ctx: any) => {}); - class SubComponent extends SuperComponent { - static ngComponentDef = defineComponent({ - type: SubComponent, - template: () => {}, - consts: 0, - vars: 0, - selectors: [['', 'subDir', '']], - viewQuery: (directiveIndex: number, elementIndex: number) => { - log.push(['sub', directiveIndex, elementIndex]); - }, - factory: () => new SubComponent(), - features: [InheritDefinitionFeature] - }); - } - - const subDef = SubComponent.ngComponentDef as ComponentDef; - - const context = {foo: 'bar'}; - - subDef.viewQuery !(1, context); - - expect(log).toEqual([['super', 1, context], ['sub', 1, context]]); - }); - - it('should compose viewQuery (query logic check)', () => { /* * class SuperComponent { * @ViewChildren('super') superQuery; @@ -417,7 +379,7 @@ describe('InheritDefinitionFeature', () => { template: () => {}, consts: 0, vars: 0, - selectors: [['', 'superDir', '']], + selectors: [['super-comp']], viewQuery: (rf: RenderFlags, ctx: any) => { if (rf & RenderFlags.Create) { viewQuery(['super'], false); @@ -435,6 +397,7 @@ describe('InheritDefinitionFeature', () => { /** *
*
+ * * class SubComponent extends SuperComponent { * @ViewChildren('sub') subQuery; * } @@ -447,11 +410,12 @@ describe('InheritDefinitionFeature', () => { if (rf & RenderFlags.Create) { element(0, 'div', ['id', 'sub'], ['sub', '']); element(2, 'div', ['id', 'super'], ['super', '']); + element(4, 'some-comp'); } }, - consts: 4, + consts: 5, vars: 0, - selectors: [['', 'subDir', '']], + selectors: [['sub-comp']], viewQuery: (rf: RenderFlags, ctx: any) => { if (rf & RenderFlags.Create) { viewQuery(['sub'], false); @@ -463,23 +427,98 @@ describe('InheritDefinitionFeature', () => { } }, factory: () => new SubComponent(), - features: [InheritDefinitionFeature] + features: [InheritDefinitionFeature], + directives: [SomeComp] }); } - const fixture = new ComponentFixture(SubComponent); - const check = (key: string): void => { - const qList = (fixture.component as any)[`${key}Query`] as QueryList; - expect(qList.length).toBe(1); - expect(qList.first.nativeElement).toEqual(fixture.hostElement.querySelector(`#${key}`)); - expect(qList.first.nativeElement.id).toEqual(key); - }; + it('should compose viewQuery (basic mechanics check)', () => { + const log: Array<[string, RenderFlags, any]> = []; + + class SuperComponent { + static ngComponentDef = defineComponent({ + type: SuperComponent, + template: () => {}, + consts: 0, + vars: 0, + selectors: [['', 'superDir', '']], + viewQuery: (rf: RenderFlags, ctx: T) => { + log.push(['super', rf, ctx]); + }, + factory: () => new SuperComponent(), + }); + } + + class SubComponent extends SuperComponent { + static ngComponentDef = defineComponent({ + type: SubComponent, + template: () => {}, + consts: 0, + vars: 0, + selectors: [['', 'subDir', '']], + viewQuery: (directiveIndex: number, elementIndex: number) => { + log.push(['sub', directiveIndex, elementIndex]); + }, + factory: () => new SubComponent(), + features: [InheritDefinitionFeature] + }); + } + + const subDef = SubComponent.ngComponentDef as ComponentDef; + + const context = {foo: 'bar'}; + + subDef.viewQuery !(1, context); + + expect(log).toEqual([['super', 1, context], ['sub', 1, context]]); + }); + + + + it('should compose viewQuery (query logic check)', () => { + const fixture = new ComponentFixture(SubComponent); + + const check = (key: string): void => { + const qList = (fixture.component as any)[`${key}Query`] as QueryList; + expect(qList.length).toBe(1); + expect(qList.first.nativeElement).toEqual(fixture.hostElement.querySelector(`#${key}`)); + expect(qList.first.nativeElement.id).toEqual(key); + }; + + check('sub'); + check('super'); + }); + + it('should work with multiple viewQuery comps', () => { + let subCompOne !: SubComponent; + let subCompTwo !: SubComponent; + + const App = createComponent('app', (rf: RenderFlags, ctx: any) => { + if (rf & RenderFlags.Create) { + element(0, 'sub-comp'); + element(1, 'sub-comp'); + } + subCompOne = getDirectiveOnNode(0); + subCompTwo = getDirectiveOnNode(1); + }, 2, 0, [SubComponent, SuperComponent]); + + const fixture = new ComponentFixture(App); + + const check = (comp: SubComponent): void => { + const qList = comp.subQuery as QueryList; + expect(qList.length).toBe(1); + expect(qList.first.nativeElement).toEqual(fixture.hostElement.querySelector('#sub')); + expect(qList.first.nativeElement.id).toEqual('sub'); + }; + + check(subCompOne); + check(subCompTwo); + }); - check('sub'); - check('super'); }); + it('should compose contentQueries', () => { const log: string[] = []; diff --git a/packages/core/test/render3/query_spec.ts b/packages/core/test/render3/query_spec.ts index fa70e15733..611a7f9513 100644 --- a/packages/core/test/render3/query_spec.ts +++ b/packages/core/test/render3/query_spec.ts @@ -1305,14 +1305,14 @@ describe('query', () => { 0, Cmpt_Template_1, 2, 0, 'ng-template', null, ['foo', ''], templateRefExtractor); template( - 1, Cmpt_Template_1, 2, 0, 'ng-template', null, ['bar', ''], + 2, Cmpt_Template_1, 2, 0, 'ng-template', null, ['bar', ''], templateRefExtractor); template( - 2, Cmpt_Template_1, 2, 0, 'ng-template', null, ['baz', ''], + 4, Cmpt_Template_1, 2, 0, 'ng-template', null, ['baz', ''], templateRefExtractor); } }, - 3, 0, [], [], + 6, 0, [], [], function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { viewQuery(TemplateRef as any, false); @@ -2160,7 +2160,7 @@ describe('query', () => { element(0, 'div', null, ['foo', '']); } }, - 1, 0, [], [], + 2, 0, [], [], function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { viewQuery(['foo'], false); diff --git a/packages/core/test/render3/styling/players_spec.ts b/packages/core/test/render3/styling/players_spec.ts index c39541beb9..2f77e83943 100644 --- a/packages/core/test/render3/styling/players_spec.ts +++ b/packages/core/test/render3/styling/players_spec.ts @@ -279,8 +279,8 @@ class SuperComp { vars: 0, template: (rf: RenderFlags, ctx: SuperComp) => { if (rf & RenderFlags.Create) { - elementStart(1, 'div'); - element(2, 'child-comp', ['child', ''], ['child', 'child']); + elementStart(0, 'div'); + element(1, 'child-comp', ['child', ''], ['child', 'child']); elementEnd(); } }, diff --git a/packages/core/test/render3/view_container_ref_spec.ts b/packages/core/test/render3/view_container_ref_spec.ts index 16564a8110..00de54645e 100644 --- a/packages/core/test/render3/view_container_ref_spec.ts +++ b/packages/core/test/render3/view_container_ref_spec.ts @@ -2060,10 +2060,10 @@ describe('ViewContainerRef', () => { vars: 0, template: (rf: RenderFlags, ctx: DynamicCompWithViewQueries) => { if (rf & RenderFlags.Create) { - element(1, 'div', ['bar', ''], ['foo', '']); + element(0, 'div', ['bar', ''], ['foo', '']); } // testing only - fooEl = getNativeByIndex(1, getLView()); + fooEl = getNativeByIndex(0, getLView()); }, viewQuery: function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) {