From 39d0311e4eed3ff7d805574aa46b1131d345df6d Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Sat, 2 Feb 2019 11:20:33 -0800 Subject: [PATCH] refactor(ivy): combine contentQueries and contentQueriesRefresh functions (#28503) Prior to this update we had separate contentQueries and contentQueriesRefresh functions to handle creation and update phases. This approach was inconsistent with View Queries, Host Bindings and Template functions that we generate for Component/Directive defs. Now the mentioned 2 functions are combines into one (contentQueries), creation and update logic is separated with RenderFlags (similar to what we have in other generated functions). PR Close #28503 --- .../compliance/r3_compiler_compliance_spec.ts | 45 +++--- .../compiler/src/render3/view/compiler.ts | 84 ++++------ packages/core/src/render3/definition.ts | 15 +- .../features/inherit_definition_feature.ts | 21 +-- packages/core/src/render3/instructions.ts | 26 +-- .../core/src/render3/interfaces/definition.ts | 23 +-- packages/core/src/render3/interfaces/view.ts | 4 +- .../core/test/render3/host_binding_spec.ts | 13 +- .../inherit_definition_feature_spec.ts | 67 +++----- packages/core/test/render3/ivy/jit_spec.ts | 4 - packages/core/test/render3/query_spec.ts | 148 ++++++++++-------- 11 files changed, 206 insertions(+), 244 deletions(-) diff --git a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts index 3015e4bbac..497d59d143 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -1553,15 +1553,16 @@ describe('compiler compliance', () => { factory: function ContentQueryComponent_Factory(t) { return new (t || ContentQueryComponent)(); }, - contentQueries: function ContentQueryComponent_ContentQueries(dirIndex) { + contentQueries: function ContentQueryComponent_ContentQueries(rf, ctx, dirIndex) { + if (rf & 1) { $r3$.ɵcontentQuery(dirIndex, SomeDirective, true); $r3$.ɵcontentQuery(dirIndex, SomeDirective, false); - }, - contentQueriesRefresh: function ContentQueryComponent_ContentQueriesRefresh(dirIndex) { - const instance = $r3$.ɵload(dirIndex); + } + if (rf & 2) { var $tmp$; - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && ($instance$.someDir = $tmp$.first)); - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && ($instance$.someDirList = $tmp$)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (ctx.someDir = $tmp$.first)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (ctx.someDirList = $tmp$)); + } }, ngContentSelectors: _c0, consts: 2, @@ -1612,15 +1613,16 @@ describe('compiler compliance', () => { … ContentQueryComponent.ngComponentDef = $r3$.ɵdefineComponent({ … - contentQueries: function ContentQueryComponent_ContentQueries(dirIndex) { + contentQueries: function ContentQueryComponent_ContentQueries(rf, ctx, dirIndex) { + if (rf & 1) { $r3$.ɵcontentQuery(dirIndex, $e0_attrs$, true); $r3$.ɵcontentQuery(dirIndex, $e1_attrs$, false); - }, - contentQueriesRefresh: function ContentQueryComponent_ContentQueriesRefresh(dirIndex) { - const instance = $r3$.ɵload(dirIndex); + } + if (rf & 2) { var $tmp$; - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (instance.myRef = $tmp$.first)); - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (instance.myRefs = $tmp$)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (ctx.myRef = $tmp$.first)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (ctx.myRefs = $tmp$)); + } }, … });`; @@ -1665,19 +1667,20 @@ describe('compiler compliance', () => { … ContentQueryComponent.ngComponentDef = $r3$.ɵdefineComponent({ … - contentQueries: function ContentQueryComponent_ContentQueries(dirIndex) { - $r3$.ɵcontentQuery(dirIndex, $e0_attrs$ , true, TemplateRef); + contentQueries: function ContentQueryComponent_ContentQueries(rf, ctx, dirIndex) { + if (rf & 1) { + $r3$.ɵcontentQuery(dirIndex, $e0_attrs$, true, TemplateRef); $r3$.ɵcontentQuery(dirIndex, SomeDirective, true, ElementRef); $r3$.ɵcontentQuery(dirIndex, $e1_attrs$, false, ElementRef); $r3$.ɵcontentQuery(dirIndex, SomeDirective, false, TemplateRef); - }, - contentQueriesRefresh: function ContentQueryComponent_ContentQueriesRefresh(dirIndex) { - const instance = $r3$.ɵload(dirIndex); + } + if (rf & 2) { var $tmp$; - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (instance.myRef = $tmp$.first)); - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (instance.someDir = $tmp$.first)); - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (instance.myRefs = $tmp$)); - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (instance.someDirs = $tmp$)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (ctx.myRef = $tmp$.first)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (ctx.someDir = $tmp$.first)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (ctx.myRefs = $tmp$)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (ctx.someDirs = $tmp$)); + } }, … });`; diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index bfc2b7aeca..a79cb4186a 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -64,9 +64,10 @@ function baseDirectiveFields( }); definitionMap.set('factory', result.factory); - definitionMap.set('contentQueries', createContentQueriesFunction(meta, constantPool)); - - definitionMap.set('contentQueriesRefresh', createContentQueriesRefreshFunction(meta)); + if (meta.queries.length > 0) { + // e.g. `contentQueries: (rf, ctx, dirIndex) => { ... } + definitionMap.set('contentQueries', createContentQueriesFunction(meta, constantPool)); + } // Initialize hostVarsCount to number of bound host properties (interpolations illegal), // except 'style' and 'class' properties, since they should *not* allocate host var slots @@ -510,56 +511,39 @@ function convertAttributesToExpressions(attributes: any): o.Expression[] { return values; } -// Return a contentQueries function or null if one is not necessary. +// Define and update any content queries function createContentQueriesFunction( - meta: R3DirectiveMetadata, constantPool: ConstantPool): o.Expression|null { - if (meta.queries.length) { - const statements: o.Statement[] = meta.queries.map((query: R3QueryMetadata) => { - const args = [o.variable('dirIndex'), ...prepareQueryParams(query, constantPool) as any]; - return o.importExpr(R3.contentQuery).callFn(args).toStmt(); - }); - const typeName = meta.name; - const parameters = [new o.FnParam('dirIndex', o.NUMBER_TYPE)]; - return o.fn( - parameters, statements, o.INFERRED_TYPE, null, - typeName ? `${typeName}_ContentQueries` : null); + meta: R3DirectiveMetadata, constantPool: ConstantPool): o.Expression { + const createStatements: o.Statement[] = []; + const updateStatements: o.Statement[] = []; + const tempAllocator = temporaryAllocator(updateStatements, TEMPORARY_NAME); + + for (const query of meta.queries) { + // creation, e.g. r3.contentQuery(dirIndex, somePredicate, true); + const args = [o.variable('dirIndex'), ...prepareQueryParams(query, constantPool) as any]; + createStatements.push(o.importExpr(R3.contentQuery).callFn(args).toStmt()); + + // update, e.g. (r3.queryRefresh(tmp = r3.loadContentQuery()) && (ctx.someDir = tmp)); + const temporary = tempAllocator(); + const getQueryList = o.importExpr(R3.loadContentQuery).callFn([]); + const refresh = o.importExpr(R3.queryRefresh).callFn([temporary.set(getQueryList)]); + const updateDirective = o.variable(CONTEXT_NAME) + .prop(query.propertyName) + .set(query.first ? temporary.prop('first') : temporary); + updateStatements.push(refresh.and(updateDirective).toStmt()); } - return null; -} - -// Return a contentQueriesRefresh function or null if one is not necessary. -function createContentQueriesRefreshFunction(meta: R3DirectiveMetadata): o.Expression|null { - if (meta.queries.length > 0) { - const statements: o.Statement[] = []; - const typeName = meta.name; - const parameters = [new o.FnParam('dirIndex', o.NUMBER_TYPE)]; - const directiveInstanceVar = o.variable('instance'); - // var $tmp$: any; - const temporary = temporaryAllocator(statements, TEMPORARY_NAME); - - // const $instance$ = $r3$.ɵload(dirIndex); - statements.push(directiveInstanceVar.set(o.importExpr(R3.load).callFn([o.variable('dirIndex')])) - .toDeclStmt(o.INFERRED_TYPE, [o.StmtModifier.Final])); - - meta.queries.forEach((query: R3QueryMetadata) => { - const getQueryList = o.importExpr(R3.loadContentQuery).callFn([]); - const assignToTemporary = temporary().set(getQueryList); - const callQueryRefresh = o.importExpr(R3.queryRefresh).callFn([assignToTemporary]); - - const updateDirective = directiveInstanceVar.prop(query.propertyName) - .set(query.first ? temporary().prop('first') : temporary()); - const refreshQueryAndUpdateDirective = callQueryRefresh.and(updateDirective); - - statements.push(refreshQueryAndUpdateDirective.toStmt()); - }); - - return o.fn( - parameters, statements, o.INFERRED_TYPE, null, - typeName ? `${typeName}_ContentQueriesRefresh` : null); - } - - return null; + const contentQueriesFnName = meta.name ? `${meta.name}_ContentQueries` : null; + return o.fn( + [ + new o.FnParam(RENDER_FLAGS, o.NUMBER_TYPE), new o.FnParam(CONTEXT_NAME, null), + new o.FnParam('dirIndex', null) + ], + [ + renderFlagCheckIfStmt(core.RenderFlags.Create, createStatements), + renderFlagCheckIfStmt(core.RenderFlags.Update, updateStatements) + ], + o.INFERRED_TYPE, null, contentQueriesFnName); } function stringAsType(str: string): o.Type { diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts index 17fd7f5544..8a035b0c97 100644 --- a/packages/core/src/render3/definition.ts +++ b/packages/core/src/render3/definition.ts @@ -17,7 +17,7 @@ import {stringify} from '../util/stringify'; import {EMPTY_ARRAY, EMPTY_OBJ} from './empty'; import {NG_COMPONENT_DEF, NG_DIRECTIVE_DEF, NG_MODULE_DEF, NG_PIPE_DEF} from './fields'; -import {BaseDef, ComponentDef, ComponentDefFeature, ComponentQuery, ComponentTemplate, ComponentType, DirectiveDef, DirectiveDefFeature, DirectiveType, DirectiveTypesOrFactory, FactoryFn, HostBindingsFunction, PipeDef, PipeType, PipeTypesOrFactory} from './interfaces/definition'; +import {BaseDef, ComponentDef, ComponentDefFeature, ComponentTemplate, ComponentType, ContentQueriesFunction, DirectiveDef, DirectiveDefFeature, DirectiveType, DirectiveTypesOrFactory, FactoryFn, HostBindingsFunction, PipeDef, PipeType, PipeTypesOrFactory, ViewQueriesFunction} from './interfaces/definition'; import {CssSelectorList} from './interfaces/projection'; let _renderCompCount = 0; @@ -133,10 +133,7 @@ export function defineComponent(componentDefinition: { /** * Function to create instances of content queries associated with a given directive. */ - contentQueries?: ((dirIndex: number) => void); - - /** Refreshes content queries associated with directives in a given view */ - contentQueriesRefresh?: ((directiveIndex: number) => void); + contentQueries?: ContentQueriesFunction; /** * Defines the name that can be used in the template to assign this directive to a variable. @@ -189,7 +186,7 @@ export function defineComponent(componentDefinition: { * execution is different as compared to all other instructions (after change detection hooks but * before view hooks). */ - viewQuery?: ComponentQuery| null; + viewQuery?: ViewQueriesFunction| null; /** * A list of optional features to apply. @@ -251,7 +248,6 @@ export function defineComponent(componentDefinition: { ngContentSelectors: componentDefinition.ngContentSelectors, hostBindings: componentDefinition.hostBindings || null, contentQueries: componentDefinition.contentQueries || null, - contentQueriesRefresh: componentDefinition.contentQueriesRefresh || null, declaredInputs: declaredInputs, inputs: null !, // assigned in noSideEffects outputs: null !, // assigned in noSideEffects @@ -589,10 +585,7 @@ export const defineDirective = defineComponent as any as(directiveDefinition: /** * Function to create instances of content queries associated with a given directive. */ - contentQueries?: ((directiveIndex: number) => void); - - /** Refreshes content queries associated with directives in a given view */ - contentQueriesRefresh?: ((directiveIndex: number, queryIndex: number) => void); + contentQueries?: ContentQueriesFunction; /** * Defines the name that can be used in the template to assign this directive to a variable. diff --git a/packages/core/src/render3/features/inherit_definition_feature.ts b/packages/core/src/render3/features/inherit_definition_feature.ts index fabd73f4ef..7305c619f2 100644 --- a/packages/core/src/render3/features/inherit_definition_feature.ts +++ b/packages/core/src/render3/features/inherit_definition_feature.ts @@ -104,30 +104,15 @@ export function InheritDefinitionFeature(definition: DirectiveDef| Componen const superContentQueries = superDef.contentQueries; if (superContentQueries) { if (prevContentQueries) { - definition.contentQueries = (directiveIndex: number) => { - superContentQueries(directiveIndex); - prevContentQueries(directiveIndex); + definition.contentQueries = (rf: RenderFlags, ctx: T, directiveIndex: number) => { + superContentQueries(rf, ctx, directiveIndex); + prevContentQueries(rf, ctx, directiveIndex); }; } else { definition.contentQueries = superContentQueries; } } - // Merge Content Queries Refresh - const prevContentQueriesRefresh = definition.contentQueriesRefresh; - const superContentQueriesRefresh = superDef.contentQueriesRefresh; - if (superContentQueriesRefresh) { - if (prevContentQueriesRefresh) { - definition.contentQueriesRefresh = (directiveIndex: number) => { - superContentQueriesRefresh(directiveIndex); - prevContentQueriesRefresh(directiveIndex); - }; - } else { - definition.contentQueriesRefresh = superContentQueriesRefresh; - } - } - - // Merge inputs and outputs fillProperties(definition.inputs, superDef.inputs); fillProperties(definition.declaredInputs, superDef.declaredInputs); diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 721b658c46..4e84daea32 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -24,7 +24,7 @@ import {diPublicInInjector, getNodeInjectable, getOrCreateInjectable, getOrCreat import {throwMultipleComponentError} from './errors'; import {executeHooks, executeInitHooks, registerPostOrderHooks, registerPreOrderHooks} from './hooks'; import {ACTIVE_INDEX, LContainer, VIEWS} from './interfaces/container'; -import {ComponentDef, ComponentQuery, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, PipeDefListOrFactory, RenderFlags} from './interfaces/definition'; +import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from './interfaces/definition'; import {INJECTOR_BLOOM_PARENT_SIZE, NodeInjectorFactory} from './interfaces/injector'; import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, PropertyAliasValue, PropertyAliases, TAttributes, TContainerNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TProjectionNode, TViewNode} from './interfaces/node'; import {PlayerFactory} from './interfaces/player'; @@ -80,7 +80,7 @@ export function refreshDescendantViews(lView: LView) { refreshDynamicEmbeddedViews(lView); // Content query results must be refreshed before content hooks are called. - refreshContentQueries(tView); + refreshContentQueries(tView, lView); executeHooks( lView, tView.contentHooks, tView.contentCheckHooks, checkNoChangesMode, @@ -134,13 +134,15 @@ export function setHostBindings(tView: TView, viewData: LView): void { } /** Refreshes content queries for all directives in the given view. */ -function refreshContentQueries(tView: TView): void { +function refreshContentQueries(tView: TView, lView: LView): void { if (tView.contentQueries != null) { setCurrentQueryIndex(0); for (let i = 0; i < tView.contentQueries.length; i++) { const directiveDefIdx = tView.contentQueries[i]; const directiveDef = tView.data[directiveDefIdx] as DirectiveDef; - directiveDef.contentQueriesRefresh !(directiveDefIdx - HEADER_OFFSET); + ngDevMode && + assertDefined(directiveDef.contentQueries, 'contentQueries function should be defined'); + directiveDef.contentQueries !(RenderFlags.Update, lView[directiveDefIdx], directiveDefIdx); } } } @@ -534,17 +536,17 @@ export function elementContainerStart( currentQueries.addNode(tNode); lView[QUERIES] = currentQueries.clone(); } - executeContentQueries(tView, tNode); + executeContentQueries(tView, tNode, lView); } -function executeContentQueries(tView: TView, tNode: TNode) { +function executeContentQueries(tView: TView, tNode: TNode, lView: LView) { if (isContentQueryHost(tNode)) { const start = tNode.directiveStart; const end = tNode.directiveEnd; - for (let i = start; i < end; i++) { - const def = tView.data[i] as DirectiveDef; + for (let directiveIndex = start; directiveIndex < end; directiveIndex++) { + const def = tView.data[directiveIndex] as DirectiveDef; if (def.contentQueries) { - def.contentQueries(i); + def.contentQueries(RenderFlags.Create, lView[directiveIndex], directiveIndex); } } } @@ -650,7 +652,7 @@ export function elementStart( currentQueries.addNode(tNode); lView[QUERIES] = currentQueries.clone(); } - executeContentQueries(tView, tNode); + executeContentQueries(tView, tNode, lView); } /** @@ -731,7 +733,7 @@ function saveResolvedLocalsInData( export function getOrCreateTView( templateFn: ComponentTemplate, consts: number, vars: number, directives: DirectiveDefListOrFactory | null, pipes: PipeDefListOrFactory | null, - viewQuery: ComponentQuery| null): TView { + viewQuery: ViewQueriesFunction| null): TView { // TODO(misko): reading `ngPrivateData` here is problematic for two reasons // 1. It is a megamorphic call on each invocation. // 2. For nested embedded views (ngFor inside ngFor) the template instance is per @@ -756,7 +758,7 @@ export function getOrCreateTView( export function createTView( viewIndex: number, templateFn: ComponentTemplate| null, consts: number, vars: number, directives: DirectiveDefListOrFactory | null, pipes: PipeDefListOrFactory | null, - viewQuery: ComponentQuery| null): TView { + viewQuery: ViewQueriesFunction| null): TView { ngDevMode && ngDevMode.tView++; const bindingStartIndex = HEADER_OFFSET + consts; // This length does not yet contain host bindings from child directives because at this point, diff --git a/packages/core/src/render3/interfaces/definition.ts b/packages/core/src/render3/interfaces/definition.ts index 638b2b0157..1228249cef 100644 --- a/packages/core/src/render3/interfaces/definition.ts +++ b/packages/core/src/render3/interfaces/definition.ts @@ -22,9 +22,15 @@ export type ComponentTemplate = { }; /** - * Definition of what a query function should look like. + * Definition of what a view queries function should look like. */ -export type ComponentQuery = ComponentTemplate; +export type ViewQueriesFunction = (rf: RenderFlags, ctx: U) => void; + +/** + * Definition of what a content queries function should look like. + */ +export type ContentQueriesFunction = + (rf: RenderFlags, ctx: U, directiveIndex: number) => void; /** * Definition of what a factory function should look like. @@ -148,14 +154,13 @@ export interface DirectiveDef extends BaseDef { factory: FactoryFn; /** - * Function to create instances of content queries associated with a given directive. + * Function to create and refresh content queries associated with a given directive. */ - contentQueries: ((directiveIndex: number) => void)|null; + contentQueries: ContentQueriesFunction|null; - /** Refreshes content queries associated with directives in a given view */ - contentQueriesRefresh: ((directiveIndex: number) => void)|null; - - /** Refreshes host bindings on the associated directive. */ + /** + * Refreshes host bindings on the associated directive. + */ hostBindings: HostBindingsFunction|null; /* The following are lifecycle hooks for this component */ @@ -236,7 +241,7 @@ export interface ComponentDef extends DirectiveDef { /** * Query-related instructions for a component. */ - viewQuery: ComponentQuery|null; + viewQuery: ViewQueriesFunction|null; /** * The view encapsulation type, which determines how styles are applied to diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index dff84dfe0d..66c1121bc2 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -13,7 +13,7 @@ import {QueryList} from '../../linker'; import {Sanitizer} from '../../sanitization/security'; import {LContainer} from './container'; -import {ComponentDef, ComponentQuery, ComponentTemplate, DirectiveDef, DirectiveDefList, HostBindingsFunction, PipeDef, PipeDefList} from './definition'; +import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefList, HostBindingsFunction, PipeDef, PipeDefList, ViewQueriesFunction} from './definition'; import {I18nUpdateOpCodes, TI18n} from './i18n'; import {TElementNode, TNode, TViewNode} from './node'; import {PlayerHandler} from './player'; @@ -323,7 +323,7 @@ export interface TView { /** * A function containing query-related instructions. */ - viewQuery: ComponentQuery<{}>|null; + viewQuery: ViewQueriesFunction<{}>|null; /** * Pointer to the `TNode` that represents the root of the view. diff --git a/packages/core/test/render3/host_binding_spec.ts b/packages/core/test/render3/host_binding_spec.ts index a80eb9373b..2185df3317 100644 --- a/packages/core/test/render3/host_binding_spec.ts +++ b/packages/core/test/render3/host_binding_spec.ts @@ -1009,11 +1009,14 @@ describe('host bindings', () => { elementProperty(elIndex, 'id', bind(ctx.foos.length), null, true); } }, - contentQueries: (dirIndex: number) => { contentQuery(dirIndex, ['foo']); }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - const instance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (instance.foos = tmp); + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, ['foo']); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.foos = tmp); + } }, template: (rf: RenderFlags, cmp: HostBindingWithContentChildren) => {} }); diff --git a/packages/core/test/render3/inherit_definition_feature_spec.ts b/packages/core/test/render3/inherit_definition_feature_spec.ts index efdef34f30..7fa9769039 100644 --- a/packages/core/test/render3/inherit_definition_feature_spec.ts +++ b/packages/core/test/render3/inherit_definition_feature_spec.ts @@ -520,7 +520,7 @@ describe('InheritDefinitionFeature', () => { }); - it('should compose contentQueries', () => { + it('should compose contentQueries (basic mechanics check)', () => { const log: string[] = []; class SuperDirective { @@ -544,45 +544,12 @@ describe('InheritDefinitionFeature', () => { const subDef = SubDirective.ngDirectiveDef as DirectiveDef; - subDef.contentQueries !(0); + subDef.contentQueries !(RenderFlags.Create, {}, 0); expect(log).toEqual(['super', 'sub']); }); - it('should compose contentQueriesRefresh', () => { - const log: Array<[string, number]> = []; - - class SuperDirective { - static ngDirectiveDef = defineDirective({ - type: SuperDirective, - selectors: [['', 'superDir', '']], - contentQueriesRefresh: (directiveIndex: number) => { - log.push(['super', directiveIndex]); - }, - factory: () => new SuperDirective(), - }); - } - - class SubDirective extends SuperDirective { - static ngDirectiveDef = defineDirective({ - type: SubDirective, - selectors: [['', 'subDir', '']], - contentQueriesRefresh: (directiveIndex: number) => { - log.push(['sub', directiveIndex]); - }, - factory: () => new SubDirective(), - features: [InheritDefinitionFeature] - }); - } - - const subDef = SubDirective.ngDirectiveDef as DirectiveDef; - - subDef.contentQueriesRefresh !(1); - - expect(log).toEqual([['super', 1], ['sub', 1]]); - }); - - it('should compose contentQueries and contentQueriesRefresh', () => { + it('should compose contentQueries (verify query sets)', () => { let dirInstance: SubDirective; class SuperDirective { // @ContentChildren('foo') @@ -592,11 +559,14 @@ describe('InheritDefinitionFeature', () => { type: SuperDirective, selectors: [['', 'super-dir', '']], factory: () => new SuperDirective(), - contentQueries: (dirIndex: number) => { contentQuery(dirIndex, ['foo'], true); }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - const instance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (instance.foos = tmp); + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, ['foo'], true); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.foos = tmp); + } } }); } @@ -608,12 +578,15 @@ describe('InheritDefinitionFeature', () => { static ngDirectiveDef = defineDirective({ type: SubDirective, selectors: [['', 'sub-dir', '']], - factory: () => new SubDirective(), - contentQueries: (dirIndex: number) => { contentQuery(dirIndex, ['bar'], true); }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - dirInstance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (dirInstance.bars = tmp); + factory: () => dirInstance = new SubDirective(), + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, ['bar'], true); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.bars = tmp); + } }, features: [InheritDefinitionFeature] }); diff --git a/packages/core/test/render3/ivy/jit_spec.ts b/packages/core/test/render3/ivy/jit_spec.ts index acebef453d..0e8e3c399c 100644 --- a/packages/core/test/render3/ivy/jit_spec.ts +++ b/packages/core/test/render3/ivy/jit_spec.ts @@ -313,7 +313,6 @@ ivyEnabled && describe('render3 jit', () => { } expect((TestDirective as any).ngDirectiveDef.contentQueries).not.toBeNull(); - expect((TestDirective as any).ngDirectiveDef.contentQueriesRefresh).not.toBeNull(); }); it('should compile ContentChild query with string predicate on a directive', () => { @@ -323,7 +322,6 @@ ivyEnabled && describe('render3 jit', () => { } expect((TestDirective as any).ngDirectiveDef.contentQueries).not.toBeNull(); - expect((TestDirective as any).ngDirectiveDef.contentQueriesRefresh).not.toBeNull(); }); it('should compile ContentChildren query with type predicate on a directive', () => { @@ -335,7 +333,6 @@ ivyEnabled && describe('render3 jit', () => { } expect((TestDirective as any).ngDirectiveDef.contentQueries).not.toBeNull(); - expect((TestDirective as any).ngDirectiveDef.contentQueriesRefresh).not.toBeNull(); }); it('should compile ContentChild query with type predicate on a directive', () => { @@ -347,7 +344,6 @@ ivyEnabled && describe('render3 jit', () => { } expect((TestDirective as any).ngDirectiveDef.contentQueries).not.toBeNull(); - expect((TestDirective as any).ngDirectiveDef.contentQueriesRefresh).not.toBeNull(); }); it('should not pick up view queries from directives', () => { diff --git a/packages/core/test/render3/query_spec.ts b/packages/core/test/render3/query_spec.ts index 9a7c334dc8..8f769b4b3f 100644 --- a/packages/core/test/render3/query_spec.ts +++ b/packages/core/test/render3/query_spec.ts @@ -2281,12 +2281,15 @@ describe('query', () => { static ngDirectiveDef = defineDirective({ type: WithContentDirective, selectors: [['', 'with-content', '']], - factory: () => new WithContentDirective(), - contentQueries: (dirIndex: number) => { contentQuery(dirIndex, ['foo'], true); }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - withContentInstance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (withContentInstance.foos = tmp); + factory: () => withContentInstance = new WithContentDirective(), + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, ['foo'], true); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.foos = tmp); + } } }); } @@ -2298,15 +2301,18 @@ describe('query', () => { static ngComponentDef = defineComponent({ type: ShallowComp, selectors: [['shallow-comp']], - factory: () => new ShallowComp(), + factory: () => shallowCompInstance = new ShallowComp(), template: function(rf: RenderFlags, ctx: any) {}, consts: 0, vars: 0, - contentQueries: (dirIndex: number) => { contentQuery(dirIndex, ['foo'], false); }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - shallowCompInstance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (shallowCompInstance.foos = tmp); + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, ['foo'], false); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.foos = tmp); + } } }); } @@ -2525,16 +2531,17 @@ describe('query', () => { selectors: [['', 'query', '']], exportAs: ['query'], factory: () => new QueryDirective(), - contentQueries: (dirIndex: number) => { - // @ContentChildren('foo, bar, baz', {descendants: true}) fooBars: - // QueryList; - contentQuery(dirIndex, ['foo', 'bar', 'baz'], true); - }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - const instance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (instance.fooBars = tmp); - }, + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + // @ContentChildren('foo, bar, baz', {descendants: true}) + // fooBars: QueryList; + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, ['foo', 'bar', 'baz'], true); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.fooBars = tmp); + } + } }); } @@ -2588,16 +2595,17 @@ describe('query', () => { selectors: [['', 'query', '']], exportAs: ['query'], factory: () => new QueryDirective(), - contentQueries: (dirIndex: number) => { - // @ContentChildren('foo, bar, baz', {descendants: true}) fooBars: - // QueryList; - contentQuery(dirIndex, ['foo'], false); - }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - const instance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (instance.fooBars = tmp); - }, + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + // @ContentChildren('foo', {descendants: true}) + // fooBars: QueryList; + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, ['foo'], false); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.fooBars = tmp); + } + } }); } @@ -2643,16 +2651,17 @@ describe('query', () => { selectors: [['', 'query', '']], exportAs: ['query'], factory: () => new QueryDirective(), - contentQueries: (dirIndex: number) => { - // @ContentChildren('foo', {descendants: true}) fooBars: - // QueryList; - contentQuery(dirIndex, ['foo'], false); - }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - const instance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (instance.fooBars = tmp); - }, + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + // @ContentChildren('foo', {descendants: true}) + // fooBars: QueryList; + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, ['foo'], false); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.fooBars = tmp); + } + } }); } @@ -2702,15 +2711,17 @@ describe('query', () => { selectors: [['', 'shallow-query', '']], exportAs: ['shallow-query'], factory: () => new ShallowQueryDirective(), - contentQueries: (dirIndex: number) => { - // @ContentChildren('foo', {descendants: false}) foos: QueryList; - contentQuery(dirIndex, ['foo'], false); - }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - const instance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (instance.foos = tmp); - }, + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + // @ContentChildren('foo', {descendants: false}) + // foos: QueryList; + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, ['foo'], false); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.foos = tmp); + } + } }); } @@ -2721,15 +2732,17 @@ describe('query', () => { selectors: [['', 'deep-query', '']], exportAs: ['deep-query'], factory: () => new DeepQueryDirective(), - contentQueries: (dirIndex: number) => { - // @ContentChildren('foo', {descendants: false}) foos: QueryList; - contentQuery(dirIndex, ['foo'], true); - }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - const instance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (instance.foos = tmp); - }, + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + // @ContentChildren('foo', {descendants: true}) + // foos: QueryList; + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, ['foo'], true); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.foos = tmp); + } + } }); } @@ -2795,11 +2808,16 @@ describe('query', () => { type: ContentQueryDirective, selectors: [['', 'content-query', '']], factory: () => contentQueryDirective = new ContentQueryDirective(), - contentQueries: (dirIndex: number) => { contentQuery(dirIndex, TextDirective, true); }, - contentQueriesRefresh: (dirIndex: number) => { - let tmp: any; - const instance = load(dirIndex); - queryRefresh(tmp = loadContentQuery()) && (instance.texts = tmp); + contentQueries: (rf: RenderFlags, ctx: any, dirIndex: number) => { + // @ContentChildren(TextDirective, {descendants: true}) + // texts: QueryList; + if (rf & RenderFlags.Create) { + contentQuery(dirIndex, TextDirective, true); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = loadContentQuery()) && (ctx.texts = tmp); + } } }); }