Skip to content

Commit

Permalink
feat(core): allow users to define timing of ViewChild/ContentChild qu…
Browse files Browse the repository at this point in the history
…eries (#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
  • Loading branch information
kara authored and IgorMinar committed Feb 19, 2019
1 parent 5e68e35 commit 19afb79
Show file tree
Hide file tree
Showing 7 changed files with 363 additions and 136 deletions.
1 change: 1 addition & 0 deletions packages/compiler/src/compile_metadata.ts
Expand Up @@ -164,6 +164,7 @@ export interface CompileQueryMetadata {
first: boolean;
propertyName: string;
read: CompileTokenMetadata;
static?: boolean;
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/compiler/src/core.ts
Expand Up @@ -29,6 +29,7 @@ export interface Query {
read: any;
isViewQuery: boolean;
selector: any;
static?: boolean;
}

export const createContentChildren = makeMetadataFactory<Query>(
Expand Down
3 changes: 2 additions & 1 deletion packages/compiler/src/metadata_resolver.ts
Expand Up @@ -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
};
}

Expand Down
20 changes: 15 additions & 5 deletions packages/compiler/src/view_compiler/view_compiler.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1081,18 +1081,28 @@ 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;
}
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;
}
21 changes: 17 additions & 4 deletions packages/core/src/metadata/di.ts
Expand Up @@ -102,6 +102,7 @@ export interface Query {
read: any;
isViewQuery: boolean;
selector: any;
static?: boolean;
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

/**
Expand Down

0 comments on commit 19afb79

Please sign in to comment.