From bb94434d854bfbc1b04efe96dcda338aa2c9a826 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Wed, 23 Jan 2019 11:54:43 -0800 Subject: [PATCH] fix(ivy): Content Queries inheritance fix (#28324) Prior to this change contentQueriesRefresh functions that represent refresh logic for @ContentQuery list were not composable, which caused problems in case one Directive inherits another one and both of them contain Content Queries. Due to the fact that we used indices to reference queries in refresh function, results were placed into wrong Queries. In order to avoid that we no longer use indices to reference queries and instead maintain current content query index while iterating through them. This allows us to compose contentQueriesRefresh functions and make inheritance feature work with Content Queries. PR Close #28324 --- .../compliance/r3_compiler_compliance_spec.ts | 38 ++++----- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 47 +++++------ .../compiler/src/render3/r3_identifiers.ts | 6 +- .../compiler/src/render3/view/compiler.ts | 22 ++---- .../core/src/core_render3_private_export.ts | 5 +- packages/core/src/render3/definition.ts | 2 +- .../features/inherit_definition_feature.ts | 12 +-- packages/core/src/render3/index.ts | 7 +- packages/core/src/render3/instructions.ts | 41 ++-------- .../core/src/render3/interfaces/definition.ts | 2 +- packages/core/src/render3/interfaces/view.ts | 3 - packages/core/src/render3/jit/environment.ts | 5 +- packages/core/src/render3/query.ts | 56 ++++++++++++-- packages/core/src/render3/state.ts | 14 ++-- .../hello_world/bundle.golden_symbols.json | 2 +- .../bundling/todo/bundle.golden_symbols.json | 2 +- .../core/test/render3/host_binding_spec.ts | 10 +-- .../inherit_definition_feature_spec.ts | 77 ++++++++++++++++--- packages/core/test/render3/query_spec.ts | 63 +++++++-------- 19 files changed, 224 insertions(+), 190 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 d6126df4193..836f6f11a31 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -1554,14 +1554,14 @@ describe('compiler compliance', () => { return new (t || ContentQueryComponent)(); }, contentQueries: function ContentQueryComponent_ContentQueries(dirIndex) { - $r3$.ɵregisterContentQuery($r3$.ɵquery(SomeDirective, true), dirIndex); - $r3$.ɵregisterContentQuery($r3$.ɵquery(SomeDirective, false), dirIndex); + $r3$.ɵcontentQuery(dirIndex, SomeDirective, true); + $r3$.ɵcontentQuery(dirIndex, SomeDirective, false); }, - contentQueriesRefresh: function ContentQueryComponent_ContentQueriesRefresh(dirIndex, queryStartIndex) { + contentQueriesRefresh: function ContentQueryComponent_ContentQueriesRefresh(dirIndex) { const instance = $r3$.ɵload(dirIndex); var $tmp$; - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadQueryList(queryStartIndex))) && ($instance$.someDir = $tmp$.first)); - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadQueryList((queryStartIndex + 1)))) && ($instance$.someDirList = $tmp$)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && ($instance$.someDir = $tmp$.first)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && ($instance$.someDirList = $tmp$)); }, ngContentSelectors: _c0, consts: 2, @@ -1613,14 +1613,14 @@ describe('compiler compliance', () => { ContentQueryComponent.ngComponentDef = $r3$.ɵdefineComponent({ … contentQueries: function ContentQueryComponent_ContentQueries(dirIndex) { - $r3$.ɵregisterContentQuery($r3$.ɵquery($e0_attrs$, true), dirIndex); - $r3$.ɵregisterContentQuery($r3$.ɵquery($e1_attrs$, false), dirIndex); + $r3$.ɵcontentQuery(dirIndex, $e0_attrs$, true); + $r3$.ɵcontentQuery(dirIndex, $e1_attrs$, false); }, - contentQueriesRefresh: function ContentQueryComponent_ContentQueriesRefresh(dirIndex, queryStartIndex) { + contentQueriesRefresh: function ContentQueryComponent_ContentQueriesRefresh(dirIndex) { const instance = $r3$.ɵload(dirIndex); var $tmp$; - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadQueryList(queryStartIndex))) && (instance.myRef = $tmp$.first)); - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadQueryList((queryStartIndex + 1)))) && (instance.myRefs = $tmp$)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (instance.myRef = $tmp$.first)); + ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadContentQuery())) && (instance.myRefs = $tmp$)); }, … });`; @@ -1666,18 +1666,18 @@ describe('compiler compliance', () => { ContentQueryComponent.ngComponentDef = $r3$.ɵdefineComponent({ … contentQueries: function ContentQueryComponent_ContentQueries(dirIndex) { - $r3$.ɵregisterContentQuery($r3$.ɵquery($e0_attrs$ , true, TemplateRef), dirIndex); - $r3$.ɵregisterContentQuery($r3$.ɵquery(SomeDirective, true, ElementRef), dirIndex); - $r3$.ɵregisterContentQuery($r3$.ɵquery($e1_attrs$, false, ElementRef), dirIndex); - $r3$.ɵregisterContentQuery($r3$.ɵquery(SomeDirective, false, TemplateRef), dirIndex); + $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, queryStartIndex) { + contentQueriesRefresh: function ContentQueryComponent_ContentQueriesRefresh(dirIndex) { const instance = $r3$.ɵload(dirIndex); var $tmp$; - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadQueryList(queryStartIndex))) && (instance.myRef = $tmp$.first)); - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadQueryList((queryStartIndex + 1)))) && (instance.someDir = $tmp$.first)); - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadQueryList((queryStartIndex + 2)))) && (instance.myRefs = $tmp$)); - ($r3$.ɵqueryRefresh(($tmp$ = $r3$.ɵloadQueryList((queryStartIndex + 3)))) && (instance.someDirs = $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$)); }, … });`; diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index b0b7e793e55..77305e15953 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -12,6 +12,18 @@ import {NgtscTestEnvironment} from './env'; const trim = (input: string): string => input.replace(/\s+/g, ' ').trim(); +const varRegExp = (name: string): RegExp => new RegExp(`var \\w+ = \\[\"${name}\"\\];`); + +const viewQueryRegExp = (descend: boolean, ref?: string): RegExp => { + const maybeRef = ref ? `, ${ref}` : ``; + return new RegExp(`i0\\.ɵviewQuery\\(\\w+, ${descend}${maybeRef}\\)`); +}; + +const contentQueryRegExp = (predicate: string, descend: boolean, ref?: string): RegExp => { + const maybeRef = ref ? `, ${ref}` : ``; + return new RegExp(`i0\\.ɵcontentQuery\\(dirIndex, ${predicate}, ${descend}${maybeRef}\\)`); +}; + describe('ngtsc behavioral tests', () => { if (!NgtscTestEnvironment.supported) { // These tests should be excluded from the non-Bazel build. @@ -706,14 +718,6 @@ describe('ngtsc behavioral tests', () => { }); it('should generate queries for components', () => { - - // Helper functions to construct RegExps for output validation - const varRegExp = (name: string): RegExp => new RegExp(`var \\w+ = \\[\"${name}\"\\];`); - const queryRegExp = (fnName: string, descend: boolean, ref?: string | null): RegExp => { - const maybeRef = ref ? `, ${ref}` : ``; - return new RegExp(`i0\\.ɵ${fnName}\\(\\w+, ${descend}${maybeRef}\\)`); - }; - env.tsconfig(); env.write(`test.ts`, ` import {Component, ContentChild, ContentChildren, TemplateRef, ViewChild} from '@angular/core'; @@ -740,23 +744,10 @@ describe('ngtsc behavioral tests', () => { expect(jsContents).toMatch(varRegExp('test1')); expect(jsContents).toMatch(varRegExp('test2')); expect(jsContents).toMatch(varRegExp('accessor')); - expect(jsContents).toContain(`i0.ɵquery(TemplateRef, false)`); - expect(jsContents) - .toMatch( - // match `i0.ɵquery(_c0, true, TemplateRef)` - queryRegExp('query', true, 'TemplateRef')); - expect(jsContents) - .toMatch( - // match `i0.ɵquery(_c0, true)` - queryRegExp('query', true)); - expect(jsContents) - .toMatch( - // match `i0.ɵviewQuery(_c0, true)` - queryRegExp('viewQuery', true)); - expect(jsContents) - .toMatch( - // match `i0.ɵviewQuery(_c0, true)` - queryRegExp('viewQuery', true)); + // match `i0.ɵcontentQuery(dirIndex, _c1, true, TemplateRef)` + expect(jsContents).toMatch(contentQueryRegExp('\\w+', true, 'TemplateRef')); + // match `i0.ɵviewQuery(_c2, true)` + expect(jsContents).toMatch(viewQueryRegExp(true)); }); it('should handle queries that use forwardRef', () => { @@ -777,8 +768,10 @@ describe('ngtsc behavioral tests', () => { env.driveMain(); const jsContents = env.getContents('test.js'); - expect(jsContents).toContain(`i0.ɵquery(TemplateRef, true)`); - expect(jsContents).toContain(`i0.ɵquery(ViewContainerRef, true)`); + // match `i0.ɵcontentQuery(dirIndex, TemplateRef, true)` + expect(jsContents).toMatch(contentQueryRegExp('TemplateRef', true)); + // match `i0.ɵcontentQuery(dirIndex, ViewContainerRef, true)` + expect(jsContents).toMatch(contentQueryRegExp('ViewContainerRef', true)); }); it('should generate host listeners for components', () => { diff --git a/packages/compiler/src/render3/r3_identifiers.ts b/packages/compiler/src/render3/r3_identifiers.ts index 2a52f5619eb..2b156e1354e 100644 --- a/packages/compiler/src/render3/r3_identifiers.ts +++ b/packages/compiler/src/render3/r3_identifiers.ts @@ -116,7 +116,6 @@ export class Identifiers { static i18nPostprocess: o.ExternalReference = {name: 'ɵi18nPostprocess', moduleName: CORE}; static load: o.ExternalReference = {name: 'ɵload', moduleName: CORE}; - static loadQueryList: o.ExternalReference = {name: 'ɵloadQueryList', moduleName: CORE}; static pipe: o.ExternalReference = {name: 'ɵpipe', moduleName: CORE}; @@ -185,12 +184,11 @@ export class Identifiers { static definePipe: o.ExternalReference = {name: 'ɵdefinePipe', moduleName: CORE}; - static query: o.ExternalReference = {name: 'ɵquery', moduleName: CORE}; static queryRefresh: o.ExternalReference = {name: 'ɵqueryRefresh', moduleName: CORE}; static viewQuery: o.ExternalReference = {name: 'ɵviewQuery', moduleName: CORE}; static loadViewQuery: o.ExternalReference = {name: 'ɵloadViewQuery', moduleName: CORE}; - static registerContentQuery: - o.ExternalReference = {name: 'ɵregisterContentQuery', moduleName: CORE}; + static contentQuery: o.ExternalReference = {name: 'ɵcontentQuery', moduleName: CORE}; + static loadContentQuery: o.ExternalReference = {name: 'ɵloadContentQuery', moduleName: CORE}; static NgOnChangesFeature: o.ExternalReference = {name: 'ɵNgOnChangesFeature', moduleName: CORE}; diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index df84da23ed8..7801a3ff614 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -496,10 +496,6 @@ function prepareQueryParams(query: R3QueryMetadata, constantPool: ConstantPool): return parameters; } -function createQueryDefinition(query: R3QueryMetadata, constantPool: ConstantPool): o.Expression { - return o.importExpr(R3.query).callFn(prepareQueryParams(query, constantPool)); -} - // Turn a directive selector into an R3-compatible selector for directive def function createDirectiveSelector(selector: string | null): o.Expression { return asLiteral(core.parseSelectorToR3Selector(selector)); @@ -519,10 +515,8 @@ function createContentQueriesFunction( meta: R3DirectiveMetadata, constantPool: ConstantPool): o.Expression|null { if (meta.queries.length) { const statements: o.Statement[] = meta.queries.map((query: R3QueryMetadata) => { - const queryDefinition = createQueryDefinition(query, constantPool); - return o.importExpr(R3.registerContentQuery) - .callFn([queryDefinition, o.variable('dirIndex')]) - .toStmt(); + 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)]; @@ -539,10 +533,7 @@ function createContentQueriesRefreshFunction(meta: R3DirectiveMetadata): o.Expre if (meta.queries.length > 0) { const statements: o.Statement[] = []; const typeName = meta.name; - const parameters = [ - new o.FnParam('dirIndex', o.NUMBER_TYPE), - new o.FnParam('queryStartIndex', o.NUMBER_TYPE), - ]; + const parameters = [new o.FnParam('dirIndex', o.NUMBER_TYPE)]; const directiveInstanceVar = o.variable('instance'); // var $tmp$: any; const temporary = temporaryAllocator(statements, TEMPORARY_NAME); @@ -551,11 +542,8 @@ function createContentQueriesRefreshFunction(meta: R3DirectiveMetadata): o.Expre statements.push(directiveInstanceVar.set(o.importExpr(R3.load).callFn([o.variable('dirIndex')])) .toDeclStmt(o.INFERRED_TYPE, [o.StmtModifier.Final])); - meta.queries.forEach((query: R3QueryMetadata, idx: number) => { - const loadQLArg = o.variable('queryStartIndex'); - const getQueryList = o.importExpr(R3.loadQueryList).callFn([ - idx > 0 ? loadQLArg.plus(o.literal(idx)) : loadQLArg - ]); + 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]); diff --git a/packages/core/src/core_render3_private_export.ts b/packages/core/src/core_render3_private_export.ts index ecbc038637b..dc0e5e97b37 100644 --- a/packages/core/src/core_render3_private_export.ts +++ b/packages/core/src/core_render3_private_export.ts @@ -47,8 +47,6 @@ export { listener as ɵlistener, text as ɵtext, embeddedViewStart as ɵembeddedViewStart, - query as ɵquery, - registerContentQuery as ɵregisterContentQuery, projection as ɵprojection, bind as ɵbind, interpolation1 as ɵinterpolation1, @@ -84,7 +82,8 @@ export { queryRefresh as ɵqueryRefresh, viewQuery as ɵviewQuery, loadViewQuery as ɵloadViewQuery, - loadQueryList as ɵloadQueryList, + contentQuery as ɵcontentQuery, + loadContentQuery as ɵloadContentQuery, elementEnd as ɵelementEnd, elementProperty as ɵelementProperty, componentHostSyntheticProperty as ɵcomponentHostSyntheticProperty, diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts index c5adb32876a..2e9ca3bb8d1 100644 --- a/packages/core/src/render3/definition.ts +++ b/packages/core/src/render3/definition.ts @@ -136,7 +136,7 @@ export function defineComponent(componentDefinition: { contentQueries?: ((dirIndex: number) => void); /** Refreshes content queries associated with directives in a given view */ - contentQueriesRefresh?: ((directiveIndex: number, queryIndex: number) => void); + contentQueriesRefresh?: ((directiveIndex: number) => void); /** * 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 0f03ddb3a6e..a1af15752f6 100644 --- a/packages/core/src/render3/features/inherit_definition_feature.ts +++ b/packages/core/src/render3/features/inherit_definition_feature.ts @@ -103,9 +103,9 @@ export function InheritDefinitionFeature(definition: DirectiveDef| Componen const superContentQueries = superDef.contentQueries; if (superContentQueries) { if (prevContentQueries) { - definition.contentQueries = (dirIndex: number) => { - superContentQueries(dirIndex); - prevContentQueries(dirIndex); + definition.contentQueries = (directiveIndex: number) => { + superContentQueries(directiveIndex); + prevContentQueries(directiveIndex); }; } else { definition.contentQueries = superContentQueries; @@ -117,9 +117,9 @@ export function InheritDefinitionFeature(definition: DirectiveDef| Componen const superContentQueriesRefresh = superDef.contentQueriesRefresh; if (superContentQueriesRefresh) { if (prevContentQueriesRefresh) { - definition.contentQueriesRefresh = (directiveIndex: number, queryIndex: number) => { - superContentQueriesRefresh(directiveIndex, queryIndex); - prevContentQueriesRefresh(directiveIndex, queryIndex); + definition.contentQueriesRefresh = (directiveIndex: number) => { + superContentQueriesRefresh(directiveIndex); + prevContentQueriesRefresh(directiveIndex); }; } else { definition.contentQueriesRefresh = superContentQueriesRefresh; diff --git a/packages/core/src/render3/index.ts b/packages/core/src/render3/index.ts index 58eab2fe86d..447538a3b33 100644 --- a/packages/core/src/render3/index.ts +++ b/packages/core/src/render3/index.ts @@ -122,15 +122,12 @@ export { } from './pipe'; export { - query, queryRefresh, viewQuery, loadViewQuery, + contentQuery, + loadContentQuery, } from './query'; -export { - registerContentQuery, - loadQueryList, -} from './instructions'; export { pureFunction0, diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 932e896f588..b5562d9909e 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -36,7 +36,7 @@ import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTENT_QUERIES, CONTEXT, DECLA import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert'; import {appendChild, appendProjectedNode, createTextNode, getLViewChild, insertView, removeView} from './node_manipulation'; import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher'; -import {decreaseElementDepthCount, enterView, getBindingsEnabled, getCheckNoChangesMode, getContextLView, getCurrentDirectiveDef, getCurrentViewQueryIndex, getElementDepthCount, getFirstTemplatePass, getIsParent, getLView, getPreviousOrParentTNode, increaseElementDepthCount, isCreationMode, leaveView, nextContextImpl, resetComponentState, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentViewQueryIndex, setFirstTemplatePass, setIsParent, setPreviousOrParentTNode} from './state'; +import {decreaseElementDepthCount, enterView, getBindingsEnabled, getCheckNoChangesMode, getContextLView, getCurrentDirectiveDef, getCurrentQueryIndex, getElementDepthCount, getFirstTemplatePass, getIsParent, getLView, getPreviousOrParentTNode, increaseElementDepthCount, isCreationMode, leaveView, nextContextImpl, resetComponentState, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setFirstTemplatePass, setIsParent, setPreviousOrParentTNode} from './state'; import {getInitialClassNameValue, initializeStaticContext as initializeStaticStylingContext, patchContextWithStaticAttrs, renderInitialStylesAndClasses, renderStyling, updateClassProp as updateElementClassProp, updateContextWithBindings, updateStyleProp as updateElementStyleProp, updateStylingMap} from './styling/class_and_style_bindings'; import {BoundPlayerFactory} from './styling/player_factory'; import {createEmptyStylingContext, getStylingContext, hasClassInput, hasStyling, isAnimationProp} from './styling/util'; @@ -137,12 +137,11 @@ export function setHostBindings(tView: TView, viewData: LView): void { /** Refreshes content queries for all directives in the given view. */ function refreshContentQueries(tView: TView): void { if (tView.contentQueries != null) { - for (let i = 0; i < tView.contentQueries.length; i += 2) { + 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, tView.contentQueries[i + 1]); + directiveDef.contentQueriesRefresh !(directiveDefIdx - HEADER_OFFSET); } } } @@ -2762,7 +2761,7 @@ export function checkView(hostView: LView, component: T) { function executeViewQueryFn(lView: LView, tView: TView, component: T): void { const viewQuery = tView.viewQuery; if (viewQuery) { - setCurrentViewQueryIndex(tView.viewQueryStartIndex); + setCurrentQueryIndex(tView.viewQueryStartIndex); viewQuery(getRenderFlags(lView), component); } } @@ -3111,16 +3110,6 @@ export function reference(index: number) { return loadInternal(contextLView, index); } -export function loadQueryList(queryListIdx: number): QueryList { - const lView = getLView(); - ngDevMode && - assertDefined( - lView[CONTENT_QUERIES], 'Content QueryList array should be defined if reading a query.'); - ngDevMode && assertDataInRange(lView[CONTENT_QUERIES] !, queryListIdx); - - return lView[CONTENT_QUERIES] ![queryListIdx]; -} - /** Retrieves a value from current `viewData`. */ export function load(index: number): T { return loadInternal(getLView(), index); @@ -3170,26 +3159,6 @@ export function injectAttribute(attrNameToInject: string): string|null { return injectAttributeImpl(getPreviousOrParentTNode(), attrNameToInject); } -/** - * Registers a QueryList, associated with a content query, for later refresh (part of a view - * refresh). - */ -export function registerContentQuery( - queryList: QueryList, currentDirectiveIndex: number): void { - const viewData = getLView(); - const tView = viewData[TVIEW]; - const savedContentQueriesLength = - (viewData[CONTENT_QUERIES] || (viewData[CONTENT_QUERIES] = [])).push(queryList); - if (getFirstTemplatePass()) { - const tViewContentQueries = tView.contentQueries || (tView.contentQueries = []); - const lastSavedDirectiveIndex = - tView.contentQueries.length ? tView.contentQueries[tView.contentQueries.length - 2] : -1; - if (currentDirectiveIndex !== lastSavedDirectiveIndex) { - tViewContentQueries.push(currentDirectiveIndex, savedContentQueriesLength - 1); - } - } -} - export const CLEAN_PROMISE = _CLEAN_PROMISE; function initializeTNodeInputs(tNode: TNode | null): PropertyAliases|null { diff --git a/packages/core/src/render3/interfaces/definition.ts b/packages/core/src/render3/interfaces/definition.ts index b4a1614e141..4e7eac276b5 100644 --- a/packages/core/src/render3/interfaces/definition.ts +++ b/packages/core/src/render3/interfaces/definition.ts @@ -134,7 +134,7 @@ export interface DirectiveDef extends BaseDef { contentQueries: ((directiveIndex: number) => void)|null; /** Refreshes content queries associated with directives in a given view */ - contentQueriesRefresh: ((directiveIndex: number, queryIndex: number) => void)|null; + contentQueriesRefresh: ((directiveIndex: number) => void)|null; /** Refreshes host bindings on the associated directive. */ hostBindings: HostBindingsFunction|null; diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index b501bd508d2..32e93e38f04 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -509,9 +509,6 @@ export interface TView { /** * A list of indices for child directives that have content queries. - * - * Even indices: Directive indices - * Odd indices: Starting index of content queries (stored in CONTENT_QUERIES) for this directive */ contentQueries: number[]|null; } diff --git a/packages/core/src/render3/jit/environment.ts b/packages/core/src/render3/jit/environment.ts index a5aa778f9a7..81b440d3c2e 100644 --- a/packages/core/src/render3/jit/environment.ts +++ b/packages/core/src/render3/jit/environment.ts @@ -40,7 +40,6 @@ export const angularCoreEnv: {[name: string]: Function} = { 'ɵnextContext': r3.nextContext, 'ɵcontainerRefreshStart': r3.containerRefreshStart, 'ɵcontainerRefreshEnd': r3.containerRefreshEnd, - 'ɵloadQueryList': r3.loadQueryList, 'ɵnamespaceHTML': r3.namespaceHTML, 'ɵnamespaceMathML': r3.namespaceMathML, 'ɵnamespaceSVG': r3.namespaceSVG, @@ -87,11 +86,11 @@ export const angularCoreEnv: {[name: string]: Function} = { 'ɵpipeBindV': r3.pipeBindV, 'ɵprojectionDef': r3.projectionDef, 'ɵpipe': r3.pipe, - 'ɵquery': r3.query, 'ɵqueryRefresh': r3.queryRefresh, 'ɵviewQuery': r3.viewQuery, 'ɵloadViewQuery': r3.loadViewQuery, - 'ɵregisterContentQuery': r3.registerContentQuery, + 'ɵcontentQuery': r3.contentQuery, + 'ɵloadContentQuery': r3.loadContentQuery, 'ɵreference': r3.reference, 'ɵelementStyling': r3.elementStyling, 'ɵelementHostAttrs': r3.elementHostAttrs, diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index 45dad1b18ef..4e729c6d025 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -13,7 +13,7 @@ import {Type} from '../interface/type'; import {ElementRef as ViewEngine_ElementRef} from '../linker/element_ref'; import {QueryList} from '../linker/query_list'; import {TemplateRef as ViewEngine_TemplateRef} from '../linker/template_ref'; -import {assertDefined, assertEqual} from '../util/assert'; +import {assertDataInRange, assertDefined, assertEqual} from '../util/assert'; import {assertPreviousIsParent} from './assert'; import {getNodeInjectable, locateDirectiveOrProvider} from './di'; @@ -23,8 +23,8 @@ 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 {HEADER_OFFSET, LView, TVIEW} from './interfaces/view'; -import {getCurrentViewQueryIndex, getIsParent, getLView, getOrCreateCurrentQueries, setCurrentViewQueryIndex} from './state'; +import {CONTENT_QUERIES, HEADER_OFFSET, LView, TVIEW} from './interfaces/view'; +import {getCurrentQueryIndex, getFirstTemplatePass, getIsParent, getLView, getOrCreateCurrentQueries, setCurrentQueryIndex} from './state'; import {isContentQueryHost} from './util'; import {createElementRef, createTemplateRef} from './view_engine_compatibility'; @@ -405,10 +405,10 @@ export function viewQuery( if (tView.firstTemplatePass) { tView.expandoStartIndex++; } - const index = getCurrentViewQueryIndex(); + const index = getCurrentQueryIndex(); const viewQuery: QueryList = query(predicate, descend, read); store(index - HEADER_OFFSET, viewQuery); - setCurrentViewQueryIndex(index + 1); + setCurrentQueryIndex(index + 1); return viewQuery; } @@ -416,7 +416,49 @@ export function viewQuery( * Loads current View Query and moves the pointer/index to the next View Query in LView. */ export function loadViewQuery(): T { - const index = getCurrentViewQueryIndex(); - setCurrentViewQueryIndex(index + 1); + const index = getCurrentQueryIndex(); + setCurrentQueryIndex(index + 1); return load(index - HEADER_OFFSET); } + +/** + * Registers a QueryList, associated with a content query, for later refresh (part of a view + * refresh). + * + * @param directiveIndex Current directive index + * @param predicate The type for which the query will search + * @param descend Whether or not to descend into children + * @param read What to save in the query + * @returns QueryList + */ +export function contentQuery( + directiveIndex: number, predicate: Type| string[], descend?: boolean, + // TODO: "read" should be an AbstractType (FW-486) + read?: any): QueryList { + const lView = getLView(); + const tView = lView[TVIEW]; + const contentQuery: QueryList = query(predicate, descend, read); + (lView[CONTENT_QUERIES] || (lView[CONTENT_QUERIES] = [])).push(contentQuery); + if (getFirstTemplatePass()) { + const tViewContentQueries = tView.contentQueries || (tView.contentQueries = []); + const lastSavedDirectiveIndex = + tView.contentQueries.length ? tView.contentQueries[tView.contentQueries.length - 1] : -1; + if (directiveIndex !== lastSavedDirectiveIndex) { + tViewContentQueries.push(directiveIndex); + } + } + return contentQuery; +} + +export function loadContentQuery(): QueryList { + const lView = getLView(); + ngDevMode && + assertDefined( + lView[CONTENT_QUERIES], 'Content QueryList array should be defined if reading a query.'); + + const index = getCurrentQueryIndex(); + ngDevMode && assertDataInRange(lView[CONTENT_QUERIES] !, index); + + setCurrentQueryIndex(index + 1); + return lView[CONTENT_QUERIES] ![index]; +} \ No newline at end of file diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index 55ca7c3fa5e..5b743d19680 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -257,18 +257,18 @@ export function setBindingRoot(value: number) { } /** - * Current index of a View Query which needs to be processed next. - * We iterate over the list of View Queries stored in LView and increment current query index. + * Current index of a View or Content Query which needs to be processed next. + * We iterate over the list of Queries and increment current query index at every step. */ -let viewQueryIndex: number = 0; +let currentQueryIndex: number = 0; -export function getCurrentViewQueryIndex(): number { +export function getCurrentQueryIndex(): number { // top level variables should not be exported for performance reasons (PERF_NOTES.md) - return viewQueryIndex; + return currentQueryIndex; } -export function setCurrentViewQueryIndex(value: number): void { - viewQueryIndex = value; +export function setCurrentQueryIndex(value: number): void { + currentQueryIndex = value; } /** diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 836307a50d6..2cb69b4040a 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -408,7 +408,7 @@ "name": "setCurrentDirectiveDef" }, { - "name": "setCurrentViewQueryIndex" + "name": "setCurrentQueryIndex" }, { "name": "setFirstTemplatePass" diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index d8f9477e9e0..4e06286c1f0 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -1134,7 +1134,7 @@ "name": "setCurrentDirectiveDef" }, { - "name": "setCurrentViewQueryIndex" + "name": "setCurrentQueryIndex" }, { "name": "setDirectiveDirty" diff --git a/packages/core/test/render3/host_binding_spec.ts b/packages/core/test/render3/host_binding_spec.ts index 34321f6d5a7..a80eb9373b2 100644 --- a/packages/core/test/render3/host_binding_spec.ts +++ b/packages/core/test/render3/host_binding_spec.ts @@ -9,8 +9,8 @@ import {ElementRef, QueryList, ViewContainerRef} from '@angular/core'; import {AttributeMarker, defineComponent, template, defineDirective, InheritDefinitionFeature, ProvidersFeature, NgOnChangesFeature} from '../../src/render3/index'; -import {allocHostVars, bind, directiveInject, element, elementAttribute, elementEnd, elementProperty, elementStyleProp, elementStyling, elementStylingApply, elementStart, listener, load, text, textBinding, loadQueryList, registerContentQuery, elementHostAttrs} from '../../src/render3/instructions'; -import {query, queryRefresh} from '../../src/render3/query'; +import {allocHostVars, bind, directiveInject, element, elementAttribute, elementEnd, elementProperty, elementStyleProp, elementStyling, elementStylingApply, elementStart, listener, load, text, textBinding, elementHostAttrs} from '../../src/render3/instructions'; +import {loadContentQuery, contentQuery, queryRefresh} from '../../src/render3/query'; import {RenderFlags} from '../../src/render3/interfaces/definition'; import {pureFunction1, pureFunction2} from '../../src/render3/pure_function'; import {sanitizeUrl, sanitizeUrlOrResourceUrl, sanitizeHtml} from '../../src/sanitization/sanitization'; @@ -1009,11 +1009,11 @@ describe('host bindings', () => { elementProperty(elIndex, 'id', bind(ctx.foos.length), null, true); } }, - contentQueries: (dirIndex) => { registerContentQuery(query(['foo']), dirIndex); }, - contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => { + contentQueries: (dirIndex: number) => { contentQuery(dirIndex, ['foo']); }, + contentQueriesRefresh: (dirIndex: number) => { let tmp: any; const instance = load(dirIndex); - queryRefresh(tmp = loadQueryList(queryStartIdx)) && (instance.foos = tmp); + queryRefresh(tmp = loadContentQuery()) && (instance.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 b86faf755c6..cafa58a4a45 100644 --- a/packages/core/test/render3/inherit_definition_feature_spec.ts +++ b/packages/core/test/render3/inherit_definition_feature_spec.ts @@ -6,8 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -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 {ElementRef, Inject, InjectionToken, QueryList, ɵAttributeMarker as AttributeMarker} from '../../src/core'; +import {ComponentDef, DirectiveDef, InheritDefinitionFeature, NgOnChangesFeature, ProvidersFeature, RenderFlags, allocHostVars, bind, contentQuery, defineBase, defineComponent, defineDirective, directiveInject, element, elementEnd, elementProperty, elementStart, load, loadContentQuery, loadViewQuery, queryRefresh, viewQuery} from '../../src/render3/index'; import {ComponentFixture, createComponent, getDirectiveOnNode} from './render_util'; @@ -549,14 +549,14 @@ describe('InheritDefinitionFeature', () => { }); it('should compose contentQueriesRefresh', () => { - const log: Array<[string, number, number]> = []; + const log: Array<[string, number]> = []; class SuperDirective { static ngDirectiveDef = defineDirective({ type: SuperDirective, selectors: [['', 'superDir', '']], - contentQueriesRefresh: (directiveIndex: number, queryIndex: number) => { - log.push(['super', directiveIndex, queryIndex]); + contentQueriesRefresh: (directiveIndex: number) => { + log.push(['super', directiveIndex]); }, factory: () => new SuperDirective(), }); @@ -566,8 +566,8 @@ describe('InheritDefinitionFeature', () => { static ngDirectiveDef = defineDirective({ type: SubDirective, selectors: [['', 'subDir', '']], - contentQueriesRefresh: (directiveIndex: number, queryIndex: number) => { - log.push(['sub', directiveIndex, queryIndex]); + contentQueriesRefresh: (directiveIndex: number) => { + log.push(['sub', directiveIndex]); }, factory: () => new SubDirective(), features: [InheritDefinitionFeature] @@ -576,9 +576,68 @@ describe('InheritDefinitionFeature', () => { const subDef = SubDirective.ngDirectiveDef as DirectiveDef; - subDef.contentQueriesRefresh !(1, 2); + subDef.contentQueriesRefresh !(1); - expect(log).toEqual([['super', 1, 2], ['sub', 1, 2]]); + expect(log).toEqual([['super', 1], ['sub', 1]]); + }); + + it('should compose contentQueries and contentQueriesRefresh', () => { + let dirInstance: SubDirective; + class SuperDirective { + // @ContentChildren('foo') + foos !: QueryList; + + static ngDirectiveDef = defineDirective({ + 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); + } + }); + } + + class SubDirective extends SuperDirective { + // @ContentChildren('bar') + bars !: QueryList; + + 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); + }, + features: [InheritDefinitionFeature] + }); + } + + /** + *
+ * + * + *
+ */ + const AppComponent = createComponent('app-component', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div', [AttributeMarker.SelectOnly, 'sub-dir']); + { + element(1, 'span', null, ['foo', '']); + element(3, 'span', null, ['bar', '']); + } + elementEnd(); + } + }, 5, 0, [SubDirective]); + + const fixture = new ComponentFixture(AppComponent); + expect(dirInstance !.foos.length).toBe(1); + expect(dirInstance !.bars.length).toBe(1); }); it('should throw if inheriting a component from a directive', () => { diff --git a/packages/core/test/render3/query_spec.ts b/packages/core/test/render3/query_spec.ts index 3696efc4764..ec16aca8ec5 100644 --- a/packages/core/test/render3/query_spec.ts +++ b/packages/core/test/render3/query_spec.ts @@ -13,9 +13,9 @@ import {EventEmitter} from '../..'; import {AttributeMarker, ProvidersFeature, defineComponent, defineDirective, detectChanges} from '../../src/render3/index'; import {getNativeByIndex} from '../../src/render3/util'; -import {bind, container, containerRefreshEnd, containerRefreshStart, directiveInject, element, elementContainerEnd, elementContainerStart, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, load, loadQueryList, reference, registerContentQuery, template, text} from '../../src/render3/instructions'; +import {bind, container, containerRefreshEnd, containerRefreshStart, directiveInject, element, elementContainerEnd, elementContainerStart, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, load, reference, template, text} from '../../src/render3/instructions'; import {RenderFlags} from '../../src/render3/interfaces/definition'; -import {query, queryRefresh, viewQuery, loadViewQuery} from '../../src/render3/query'; +import {queryRefresh, viewQuery, loadViewQuery, contentQuery, loadContentQuery} from '../../src/render3/query'; import {getLView} from '../../src/render3/state'; import {templateRefExtractor} from '../../src/render3/view_engine_compatibility_prebound'; @@ -2282,12 +2282,11 @@ describe('query', () => { type: WithContentDirective, selectors: [['', 'with-content', '']], factory: () => new WithContentDirective(), - contentQueries: (dirIndex) => { registerContentQuery(query(['foo'], true), dirIndex); }, - contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => { + contentQueries: (dirIndex: number) => { contentQuery(dirIndex, ['foo'], true); }, + contentQueriesRefresh: (dirIndex: number) => { let tmp: any; withContentInstance = load(dirIndex); - queryRefresh(tmp = loadQueryList(queryStartIdx)) && - (withContentInstance.foos = tmp); + queryRefresh(tmp = loadContentQuery()) && (withContentInstance.foos = tmp); } }); } @@ -2303,12 +2302,11 @@ describe('query', () => { template: function(rf: RenderFlags, ctx: any) {}, consts: 0, vars: 0, - contentQueries: (dirIndex) => { registerContentQuery(query(['foo'], false), dirIndex); }, - contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => { + contentQueries: (dirIndex: number) => { contentQuery(dirIndex, ['foo'], false); }, + contentQueriesRefresh: (dirIndex: number) => { let tmp: any; shallowCompInstance = load(dirIndex); - queryRefresh(tmp = loadQueryList(queryStartIdx)) && - (shallowCompInstance.foos = tmp); + queryRefresh(tmp = loadContentQuery()) && (shallowCompInstance.foos = tmp); } }); } @@ -2527,16 +2525,15 @@ describe('query', () => { selectors: [['', 'query', '']], exportAs: ['query'], factory: () => new QueryDirective(), - contentQueries: (dirIndex) => { + contentQueries: (dirIndex: number) => { // @ContentChildren('foo, bar, baz', {descendants: true}) fooBars: // QueryList; - registerContentQuery(query(['foo', 'bar', 'baz'], true), dirIndex); + contentQuery(dirIndex, ['foo', 'bar', 'baz'], true); }, - contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => { + contentQueriesRefresh: (dirIndex: number) => { let tmp: any; const instance = load(dirIndex); - queryRefresh(tmp = loadQueryList(queryStartIdx)) && - (instance.fooBars = tmp); + queryRefresh(tmp = loadContentQuery()) && (instance.fooBars = tmp); }, }); } @@ -2591,16 +2588,15 @@ describe('query', () => { selectors: [['', 'query', '']], exportAs: ['query'], factory: () => new QueryDirective(), - contentQueries: (dirIndex) => { + contentQueries: (dirIndex: number) => { // @ContentChildren('foo, bar, baz', {descendants: true}) fooBars: // QueryList; - registerContentQuery(query(['foo'], false), dirIndex); + contentQuery(dirIndex, ['foo'], false); }, - contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => { + contentQueriesRefresh: (dirIndex: number) => { let tmp: any; const instance = load(dirIndex); - queryRefresh(tmp = loadQueryList(queryStartIdx)) && - (instance.fooBars = tmp); + queryRefresh(tmp = loadContentQuery()) && (instance.fooBars = tmp); }, }); } @@ -2647,16 +2643,15 @@ describe('query', () => { selectors: [['', 'query', '']], exportAs: ['query'], factory: () => new QueryDirective(), - contentQueries: (dirIndex) => { + contentQueries: (dirIndex: number) => { // @ContentChildren('foo', {descendants: true}) fooBars: // QueryList; - registerContentQuery(query(['foo'], false), dirIndex); + contentQuery(dirIndex, ['foo'], false); }, - contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => { + contentQueriesRefresh: (dirIndex: number) => { let tmp: any; const instance = load(dirIndex); - queryRefresh(tmp = loadQueryList(queryStartIdx)) && - (instance.fooBars = tmp); + queryRefresh(tmp = loadContentQuery()) && (instance.fooBars = tmp); }, }); } @@ -2703,15 +2698,14 @@ describe('query', () => { selectors: [['', 'shallow-query', '']], exportAs: ['shallow-query'], factory: () => new ShallowQueryDirective(), - contentQueries: (dirIndex) => { + contentQueries: (dirIndex: number) => { // @ContentChildren('foo', {descendants: false}) foos: QueryList; - registerContentQuery(query(['foo'], false), dirIndex); + contentQuery(dirIndex, ['foo'], false); }, - contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => { + contentQueriesRefresh: (dirIndex: number) => { let tmp: any; const instance = load(dirIndex); - queryRefresh(tmp = loadQueryList(queryStartIdx)) && - (instance.foos = tmp); + queryRefresh(tmp = loadContentQuery()) && (instance.foos = tmp); }, }); } @@ -2723,15 +2717,14 @@ describe('query', () => { selectors: [['', 'deep-query', '']], exportAs: ['deep-query'], factory: () => new DeepQueryDirective(), - contentQueries: (dirIndex) => { + contentQueries: (dirIndex: number) => { // @ContentChildren('foo', {descendants: false}) foos: QueryList; - registerContentQuery(query(['foo'], true), dirIndex); + contentQuery(dirIndex, ['foo'], true); }, - contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => { + contentQueriesRefresh: (dirIndex: number) => { let tmp: any; const instance = load(dirIndex); - queryRefresh(tmp = loadQueryList(queryStartIdx)) && - (instance.foos = tmp); + queryRefresh(tmp = loadContentQuery()) && (instance.foos = tmp); }, }); }