feat(core): allow users to define timing of ViewChild/ContentChild queries (#28810)

Prior to this commit, the timing of `ViewChild`/`ContentChild` query
resolution depended on the results of each query. If any results
for a particular query were nested inside embedded views (e.g.
*ngIfs), that query would be resolved after change detection ran.
Otherwise, the query would be resolved as soon as nodes were created.

This inconsistency in resolution timing had the potential to cause
confusion because query results would sometimes be available in
ngOnInit, but sometimes wouldn't be available until ngAfterContentInit
or ngAfterViewInit. Code depending on a query result could suddenly
stop working as soon as an *ngIf or an *ngFor was added to the template.

With this commit, users can dictate when they want a particular
`ViewChild` or `ContentChild` query to be resolved with the `static`
flag. For example, one can mark a particular query as `static: false`
to ensure change detection always runs before its results are set:

```ts
@ContentChild('foo', {static: false}) foo !: ElementRef;
```

This means that even if there isn't a query result wrapped in an
*ngIf or an *ngFor now, adding one to the template later won't change
the timing of the query resolution and potentially break your component.

Similarly, if you know that your query needs to be resolved earlier
(e.g. you need results in an ngOnInit hook), you can mark it as
`static: true`.

```ts
@ViewChild(TemplateRef, {static: true}) foo !: TemplateRef;
```

Note: this means that your component will not support *ngIf results.

If you do not supply a `static` option when creating your `ViewChild` or
`ContentChild` query, the default query resolution timing will kick in.

Note: This new option only applies to `ViewChild` and `ContentChild`
queries, not `ViewChildren` or `ContentChildren` queries, as those types
already resolve after CD runs.

PR Close #28810
This commit is contained in:
Kara Erickson 2019-02-18 14:38:14 -08:00 committed by Igor Minar
parent 5e68e35112
commit 19afb791b4
7 changed files with 362 additions and 135 deletions

View File

@ -164,6 +164,7 @@ export interface CompileQueryMetadata {
first: boolean;
propertyName: string;
read: CompileTokenMetadata;
static?: boolean;
}
/**

View File

@ -29,6 +29,7 @@ export interface Query {
read: any;
isViewQuery: boolean;
selector: any;
static?: boolean;
}
export const createContentChildren = makeMetadataFactory<Query>(

View File

@ -1157,7 +1157,8 @@ export class CompileMetadataResolver {
selectors,
first: q.first,
descendants: q.descendants, propertyName,
read: q.read ? this._getTokenMetadata(q.read) : null !
read: q.read ? this._getTokenMetadata(q.read) : null !,
static: q.static
};
}

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {CompileDirectiveMetadata, CompilePipeSummary, rendererTypeName, tokenReference, viewClassName} from '../compile_metadata';
import {CompileDirectiveMetadata, CompilePipeSummary, CompileQueryMetadata, rendererTypeName, tokenReference, viewClassName} from '../compile_metadata';
import {CompileReflector} from '../compile_reflector';
import {BindingForm, BuiltinConverter, EventHandlerVars, LocalResolver, convertActionBinding, convertPropertyBinding, convertPropertyBindingBuiltins} from '../compiler_util/expression_converter';
import {ArgumentType, BindingFlags, ChangeDetectionStrategy, NodeFlags, QueryBindingType, QueryValueType, ViewFlags} from '../core';
@ -145,7 +145,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
const queryId = queryIndex + 1;
const bindingType = query.first ? QueryBindingType.First : QueryBindingType.All;
const flags =
NodeFlags.TypeViewQuery | calcStaticDynamicQueryFlags(queryIds, queryId, query.first);
NodeFlags.TypeViewQuery | calcStaticDynamicQueryFlags(queryIds, queryId, query);
this.nodes.push(() => ({
sourceSpan: null,
nodeFlags: flags,
@ -493,7 +493,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
dirAst.directive.queries.forEach((query, queryIndex) => {
const queryId = dirAst.contentQueryStartId + queryIndex;
const flags =
NodeFlags.TypeContentQuery | calcStaticDynamicQueryFlags(queryIds, queryId, query.first);
NodeFlags.TypeContentQuery | calcStaticDynamicQueryFlags(queryIds, queryId, query);
const bindingType = query.first ? QueryBindingType.First : QueryBindingType.All;
this.nodes.push(() => ({
sourceSpan: dirAst.sourceSpan,
@ -1081,11 +1081,11 @@ function elementEventNameAndTarget(
}
function calcStaticDynamicQueryFlags(
queryIds: StaticAndDynamicQueryIds, queryId: number, isFirst: boolean) {
queryIds: StaticAndDynamicQueryIds, queryId: number, query: CompileQueryMetadata) {
let flags = NodeFlags.None;
// Note: We only make queries static that query for a single item.
// This is because of backwards compatibility with the old view compiler...
if (isFirst && (queryIds.staticQueryIds.has(queryId) || !queryIds.dynamicQueryIds.has(queryId))) {
if (query.first && shouldResolveAsStaticQuery(queryIds, queryId, query)) {
flags |= NodeFlags.StaticQuery;
} else {
flags |= NodeFlags.DynamicQuery;
@ -1093,6 +1093,16 @@ function calcStaticDynamicQueryFlags(
return flags;
}
function shouldResolveAsStaticQuery(
queryIds: StaticAndDynamicQueryIds, queryId: number, query: CompileQueryMetadata): boolean {
// If query.static has been set by the user, use that value to determine whether
// the query is static. If none has been set, sort the query into static/dynamic
// based on query results (i.e. dynamic if CD needs to run to get all results).
return query.static ||
query.static == null &&
(queryIds.staticQueryIds.has(queryId) || !queryIds.dynamicQueryIds.has(queryId));
}
export function elementEventFullName(target: string | null, name: string): string {
return target ? `${target}:${name}` : name;
}

View File

@ -102,6 +102,7 @@ export interface Query {
read: any;
isViewQuery: boolean;
selector: any;
static?: boolean;
}
/**
@ -199,6 +200,12 @@ export interface ContentChildDecorator {
*
* * **selector** - the directive type or the name used for querying.
* * **read** - read a different token from the queried element.
* * **static** - whether or not to resolve query results before change detection runs (i.e.
* return static results only). If this option is not provided, the compiler will fall back
* to its default behavior, which is to use query results to determine the timing of query
* resolution. If any query results are inside a nested view (e.g. *ngIf), the query will be
* resolved after change detection runs. Otherwise, it will be resolved before change detection
* runs.
*
* @usageNotes
* ### Example
@ -211,8 +218,8 @@ export interface ContentChildDecorator {
*
* @Annotation
*/
(selector: Type<any>|Function|string, opts?: {read?: any}): any;
new (selector: Type<any>|Function|string, opts?: {read?: any}): ContentChild;
(selector: Type<any>|Function|string, opts?: {read?: any, static?: boolean}): any;
new (selector: Type<any>|Function|string, opts?: {read?: any, static?: boolean}): ContentChild;
}
/**
@ -311,6 +318,12 @@ export interface ViewChildDecorator {
*
* * **selector** - the directive type or the name used for querying.
* * **read** - read a different token from the queried elements.
* * **static** - whether or not to resolve query results before change detection runs (i.e.
* return static results only). If this option is not provided, the compiler will fall back
* to its default behavior, which is to use query results to determine the timing of query
* resolution. If any query results are inside a nested view (e.g. *ngIf), the query will be
* resolved after change detection runs. Otherwise, it will be resolved before change detection
* runs.
*
* Supported selectors include:
* * any class with the `@Component` or `@Directive` decorator
@ -337,8 +350,8 @@ export interface ViewChildDecorator {
*
* @Annotation
*/
(selector: Type<any>|Function|string, opts?: {read?: any}): any;
new (selector: Type<any>|Function|string, opts?: {read?: any}): ViewChild;
(selector: Type<any>|Function|string, opts?: {read?: any, static?: boolean}): any;
new (selector: Type<any>|Function|string, opts?: {read?: any, static?: boolean}): ViewChild;
}
/**

View File

@ -6,19 +6,23 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Component, ContentChild, ContentChildren, ElementRef, QueryList, TemplateRef, Type, ViewChild, ViewChildren} from '@angular/core';
import {Component, ContentChild, ContentChildren, Directive, ElementRef, Input, QueryList, TemplateRef, Type, ViewChild, ViewChildren} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {expect} from '@angular/platform-browser/testing/src/matchers';
import {onlyInIvy} from '@angular/private/testing';
import {fixmeIvy, onlyInIvy} from '@angular/private/testing';
describe('query logic', () => {
beforeEach(() => {
TestBed.configureTestingModule({declarations: [AppComp, QueryComp, SimpleCompA, SimpleCompB]});
TestBed.configureTestingModule({
declarations: [
AppComp, QueryComp, SimpleCompA, SimpleCompB, StaticViewQueryComp, TextDirective,
SubclassStaticViewQueryComp, StaticContentQueryComp, SubclassStaticContentQueryComp
]
});
});
it('should return Component instances when Components are labelled and retrieved via View query',
() => {
describe('view queries', () => {
it('should return Component instances when Components are labeled and retrieved', () => {
const template = `
<div><simple-comp-a #viewQuery></simple-comp-a></div>
<div><simple-comp-b #viewQuery></simple-comp-b></div>
@ -30,37 +34,7 @@ describe('query logic', () => {
expect(comp.viewChildren.last).toBeAnInstanceOf(SimpleCompB);
});
it('should return Component instance when Component is labelled and retrieved via Content query',
() => {
const template = `
<local-ref-query-component #q>
<simple-comp-a #contentQuery></simple-comp-a>
</local-ref-query-component>
`;
const fixture = initWithTemplate(AppComp, template);
const comp = fixture.debugElement.children[0].references['q'];
expect(comp.contentChild).toBeAnInstanceOf(SimpleCompA);
expect(comp.contentChildren.first).toBeAnInstanceOf(SimpleCompA);
});
onlyInIvy('multiple local refs are supported in Ivy')
.it('should return Component instances when Components are labelled and retrieved via Content query',
() => {
const template = `
<local-ref-query-component #q>
<simple-comp-a #contentQuery></simple-comp-a>
<simple-comp-b #contentQuery></simple-comp-b>
</local-ref-query-component>
`;
const fixture = initWithTemplate(AppComp, template);
const comp = fixture.debugElement.children[0].references['q'];
expect(comp.contentChild).toBeAnInstanceOf(SimpleCompA);
expect(comp.contentChildren.first).toBeAnInstanceOf(SimpleCompA);
expect(comp.contentChildren.last).toBeAnInstanceOf(SimpleCompB);
expect(comp.contentChildren.length).toBe(2);
});
it('should return ElementRef when HTML element is labelled and retrieved via View query', () => {
it('should return ElementRef when HTML element is labeled and retrieved', () => {
const template = `
<div #viewQuery></div>
`;
@ -71,8 +45,7 @@ describe('query logic', () => {
});
onlyInIvy('multiple local refs are supported in Ivy')
.it('should return ElementRefs when HTML elements are labelled and retrieved via View query',
() => {
.it('should return ElementRefs when HTML elements are labeled and retrieved', () => {
const template = `
<div #viewQuery #first>A</div>
<div #viewQuery #second>B</div>
@ -81,15 +54,14 @@ describe('query logic', () => {
const comp = fixture.componentInstance;
expect(comp.viewChild).toBeAnInstanceOf(ElementRef);
expect(comp.viewChild.nativeElement)
.toBe(fixture.debugElement.children[0].nativeElement);
expect(comp.viewChild.nativeElement).toBe(fixture.debugElement.children[0].nativeElement);
expect(comp.viewChildren.first).toBeAnInstanceOf(ElementRef);
expect(comp.viewChildren.last).toBeAnInstanceOf(ElementRef);
expect(comp.viewChildren.length).toBe(2);
});
it('should return TemplateRef when template is labelled and retrieved via View query', () => {
it('should return TemplateRef when template is labeled and retrieved', () => {
const template = `
<ng-template #viewQuery></ng-template>
`;
@ -99,8 +71,7 @@ describe('query logic', () => {
});
onlyInIvy('multiple local refs are supported in Ivy')
.it('should return TemplateRefs when templates are labelled and retrieved via View query',
() => {
.it('should return TemplateRefs when templates are labeled and retrieved', () => {
const template = `
<ng-template #viewQuery></ng-template>
<ng-template #viewQuery></ng-template>
@ -116,8 +87,82 @@ describe('query logic', () => {
expect(comp.viewChildren.length).toBe(2);
});
it('should return ElementRef when HTML element is labelled and retrieved via Content query',
fixmeIvy('Must support static view queries in Ivy')
.it('should set static view child queries in creation mode (and just in creation mode)',
() => {
const fixture = TestBed.createComponent(StaticViewQueryComp);
const component = fixture.componentInstance;
// static ViewChild query should be set in creation mode, before CD runs
expect(component.textDir).toBeAnInstanceOf(TextDirective);
expect(component.textDir.text).toEqual('');
expect(component.setEvents).toEqual(['textDir set']);
// dynamic ViewChild query should not have been resolved yet
expect(component.foo).not.toBeDefined();
const span = fixture.nativeElement.querySelector('span');
fixture.detectChanges();
expect(component.textDir.text).toEqual('some text');
expect(component.foo.nativeElement).toBe(span);
expect(component.setEvents).toEqual(['textDir set', 'foo set']);
});
fixmeIvy('Must support static view queries in Ivy')
.it('should support static view child queries inherited from superclasses', () => {
const fixture = TestBed.createComponent(SubclassStaticViewQueryComp);
const component = fixture.componentInstance;
const divs = fixture.nativeElement.querySelectorAll('div');
const spans = fixture.nativeElement.querySelectorAll('span');
// static ViewChild queries should be set in creation mode, before CD runs
expect(component.textDir).toBeAnInstanceOf(TextDirective);
expect(component.textDir.text).toEqual('');
expect(component.bar.nativeElement).toEqual(divs[1]);
// dynamic ViewChild queries should not have been resolved yet
expect(component.foo).not.toBeDefined();
expect(component.baz).not.toBeDefined();
fixture.detectChanges();
expect(component.textDir.text).toEqual('some text');
expect(component.foo.nativeElement).toBe(spans[0]);
expect(component.baz.nativeElement).toBe(spans[1]);
});
});
describe('content queries', () => {
it('should return Component instance when Component is labeled and retrieved', () => {
const template = `
<local-ref-query-component #q>
<simple-comp-a #contentQuery></simple-comp-a>
</local-ref-query-component>
`;
const fixture = initWithTemplate(AppComp, template);
const comp = fixture.debugElement.children[0].references['q'];
expect(comp.contentChild).toBeAnInstanceOf(SimpleCompA);
expect(comp.contentChildren.first).toBeAnInstanceOf(SimpleCompA);
});
onlyInIvy('multiple local refs are supported in Ivy')
.it('should return Component instances when Components are labeled and retrieved', () => {
const template = `
<local-ref-query-component #q>
<simple-comp-a #contentQuery></simple-comp-a>
<simple-comp-b #contentQuery></simple-comp-b>
</local-ref-query-component>
`;
const fixture = initWithTemplate(AppComp, template);
const comp = fixture.debugElement.children[0].references['q'];
expect(comp.contentChild).toBeAnInstanceOf(SimpleCompA);
expect(comp.contentChildren.first).toBeAnInstanceOf(SimpleCompA);
expect(comp.contentChildren.last).toBeAnInstanceOf(SimpleCompB);
expect(comp.contentChildren.length).toBe(2);
});
it('should return ElementRef when HTML element is labeled and retrieved', () => {
const template = `
<local-ref-query-component #q>
<div #contentQuery></div>
@ -129,8 +174,7 @@ describe('query logic', () => {
});
onlyInIvy('multiple local refs are supported in Ivy')
.it('should return ElementRefs when HTML elements are labelled and retrieved via Content query',
() => {
.it('should return ElementRefs when HTML elements are labeled and retrieved', () => {
const template = `
<local-ref-query-component #q>
<div #contentQuery></div>
@ -149,7 +193,7 @@ describe('query logic', () => {
expect(comp.contentChildren.length).toBe(2);
});
it('should return TemplateRef when template is labelled and retrieved via Content query', () => {
it('should return TemplateRef when template is labeled and retrieved', () => {
const template = `
<local-ref-query-component #q>
<ng-template #contentQuery></ng-template>
@ -161,8 +205,7 @@ describe('query logic', () => {
});
onlyInIvy('multiple local refs are supported in Ivy')
.it('should return TemplateRefs when templates are labelled and retrieved via Content query',
() => {
.it('should return TemplateRefs when templates are labeled and retrieved', () => {
const template = `
<local-ref-query-component #q>
<ng-template #contentQuery></ng-template>
@ -181,6 +224,72 @@ describe('query logic', () => {
expect(comp.contentChildren.last).toBeAnInstanceOf(TemplateRef);
expect(comp.contentChildren.length).toBe(2);
});
});
fixmeIvy('Must support static content queries in Ivy')
.it('should set static content child queries in creation mode (and just in creation mode)',
() => {
const template = `
<static-content-query-comp>
<div [text]="text"></div>
<span #foo></span>
</static-content-query-comp>
`;
TestBed.overrideComponent(AppComp, {set: new Component({template})});
const fixture = TestBed.createComponent(AppComp);
const component = fixture.debugElement.children[0].injector.get(StaticContentQueryComp);
// static ContentChild query should be set in creation mode, before CD runs
expect(component.textDir).toBeAnInstanceOf(TextDirective);
expect(component.textDir.text).toEqual('');
expect(component.setEvents).toEqual(['textDir set']);
// dynamic ContentChild query should not have been resolved yet
expect(component.foo).not.toBeDefined();
const span = fixture.nativeElement.querySelector('span');
(fixture.componentInstance as any).text = 'some text';
fixture.detectChanges();
expect(component.textDir.text).toEqual('some text');
expect(component.foo.nativeElement).toBe(span);
expect(component.setEvents).toEqual(['textDir set', 'foo set']);
});
fixmeIvy('Must support static content queries in Ivy')
.it('should support static content child queries inherited from superclasses', () => {
const template = `
<subclass-static-content-query-comp>
<div [text]="text"></div>
<span #foo></span>
<div #bar></div>
<span #baz></span>
</subclass-static-content-query-comp>
`;
TestBed.overrideComponent(AppComp, {set: new Component({template})});
const fixture = TestBed.createComponent(AppComp);
const component =
fixture.debugElement.children[0].injector.get(SubclassStaticContentQueryComp);
const divs = fixture.nativeElement.querySelectorAll('div');
const spans = fixture.nativeElement.querySelectorAll('span');
// static ContentChild queries should be set in creation mode, before CD runs
expect(component.textDir).toBeAnInstanceOf(TextDirective);
expect(component.textDir.text).toEqual('');
expect(component.bar.nativeElement).toEqual(divs[1]);
// dynamic ContentChild queries should not have been resolved yet
expect(component.foo).not.toBeDefined();
expect(component.baz).not.toBeDefined();
(fixture.componentInstance as any).text = 'some text';
fixture.detectChanges();
expect(component.textDir.text).toEqual('some text');
expect(component.foo.nativeElement).toBe(spans[0]);
expect(component.baz.nativeElement).toBe(spans[1]);
});
});
function initWithTemplate(compType: Type<any>, template: string) {
@ -210,3 +319,90 @@ class SimpleCompA {
@Component({selector: 'simple-comp-b', template: ''})
class SimpleCompB {
}
@Directive({selector: '[text]'})
class TextDirective {
@Input() text = '';
}
@Component({
selector: 'static-view-query-comp',
template: `
<div [text]="text"></div>
<span #foo></span>
`
})
class StaticViewQueryComp {
private _textDir !: TextDirective;
private _foo !: ElementRef;
setEvents: string[] = [];
@ViewChild(TextDirective, {static: true})
get textDir(): TextDirective { return this._textDir; }
set textDir(value: TextDirective) {
this.setEvents.push('textDir set');
this._textDir = value;
}
@ViewChild('foo', {static: false})
get foo(): ElementRef { return this._foo; }
set foo(value: ElementRef) {
this.setEvents.push('foo set');
this._foo = value;
}
text = 'some text';
}
@Component({
selector: 'subclass-static-view-query-comp',
template: `
<div [text]="text"></div>
<span #foo></span>
<div #bar></div>
<span #baz></span>
`
})
class SubclassStaticViewQueryComp extends StaticViewQueryComp {
@ViewChild('bar', {static: true})
bar !: ElementRef;
@ViewChild('baz', {static: false})
baz !: ElementRef;
}
@Component({selector: 'static-content-query-comp', template: `<ng-content></ng-content>`})
class StaticContentQueryComp {
private _textDir !: TextDirective;
private _foo !: ElementRef;
setEvents: string[] = [];
@ContentChild(TextDirective, {static: true})
get textDir(): TextDirective { return this._textDir; }
set textDir(value: TextDirective) {
this.setEvents.push('textDir set');
this._textDir = value;
}
@ContentChild('foo', {static: false})
get foo(): ElementRef { return this._foo; }
set foo(value: ElementRef) {
this.setEvents.push('foo set');
this._foo = value;
}
}
@Component({selector: 'subclass-static-content-query-comp', template: `<ng-content></ng-content>`})
class SubclassStaticContentQueryComp extends StaticContentQueryComp {
@ContentChild('bar', {static: true})
bar !: ElementRef;
@ContentChild('baz', {static: false})
baz !: ElementRef;
}

View File

@ -166,9 +166,11 @@ export declare type ContentChild = Query;
export interface ContentChildDecorator {
(selector: Type<any> | Function | string, opts?: {
read?: any;
static?: boolean;
}): any;
new (selector: Type<any> | Function | string, opts?: {
read?: any;
static?: boolean;
}): ContentChild;
}
@ -670,6 +672,7 @@ export interface Query {
isViewQuery: boolean;
read: any;
selector: any;
static?: boolean;
}
export declare abstract class Query {
@ -947,9 +950,11 @@ export declare type ViewChild = Query;
export interface ViewChildDecorator {
(selector: Type<any> | Function | string, opts?: {
read?: any;
static?: boolean;
}): any;
new (selector: Type<any> | Function | string, opts?: {
read?: any;
static?: boolean;
}): ViewChild;
}