From 3750ea9dff591477c6fb27ed2b526f634ba729e8 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Wed, 20 Dec 2017 17:35:03 +0100 Subject: [PATCH] feat(core): final adjustements to ngIvy read option for queries (#21187) PR Close #21187 --- packages/core/src/render3/instructions.ts | 3 +- packages/core/src/render3/interfaces.ts | 4 +- packages/core/src/render3/query.ts | 100 +++++++++++++--------- packages/core/test/render3/query_spec.ts | 82 +++++++++++++++++- 4 files changed, 146 insertions(+), 43 deletions(-) diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index a2610ccb8f..15862a9f2a 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -1780,7 +1780,8 @@ function valueInData(data: any[], index: number, value?: T): T { } export function query( - predicate: Type| string[], descend?: boolean, read?: QueryReadType): QueryList { + predicate: Type| string[], descend?: boolean, + read?: QueryReadType | Type): QueryList { ngDevMode && assertPreviousIsParent(); const queryList = new QueryList(); const query = currentQuery || (currentQuery = new QueryState_()); diff --git a/packages/core/src/render3/interfaces.ts b/packages/core/src/render3/interfaces.ts index cbab9e4418..4533909d0f 100644 --- a/packages/core/src/render3/interfaces.ts +++ b/packages/core/src/render3/interfaces.ts @@ -518,8 +518,8 @@ export interface QueryState { * @param read Indicates which token should be read from DI for this query. */ track( - queryList: QueryList, predicate: Type|string[], descend?: boolean, - read?: QueryReadType): void; + queryList: QueryList, predicate: Type|string[], descend?: boolean, + read?: QueryReadType|Type): void; } /** diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index d3340c0576..f399f77897 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -13,7 +13,6 @@ import {Observable} from 'rxjs/Observable'; import {ElementRef as viewEngine_ElementRef} from '../linker/element_ref'; import {QueryList as viewEngine_QueryList} from '../linker/query_list'; import {TemplateRef as viewEngine_TemplateRef} from '../linker/template_ref'; -import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref'; import {Type} from '../type'; import {assertNotNull} from './assert'; @@ -108,33 +107,8 @@ export class QueryState_ implements QueryState { } } -function readDefaultInjectable(nodeInjector: LNodeInjector, node: LNode): viewEngine_ElementRef| - viewEngine_TemplateRef|undefined { - ngDevMode && assertNodeOfPossibleTypes(node, LNodeFlags.Container, LNodeFlags.Element); - if ((node.flags & LNodeFlags.TYPE_MASK) === LNodeFlags.Element) { - return getOrCreateElementRef(nodeInjector); - } else if ((node.flags & LNodeFlags.TYPE_MASK) === LNodeFlags.Container) { - return getOrCreateTemplateRef(nodeInjector); - } -} - -function readFromNodeInjector(nodeInjector: LNodeInjector, node: LNode, read: QueryReadType | null): - viewEngine_ElementRef|viewEngine_ViewContainerRef|viewEngine_TemplateRef|undefined { - if (read === QueryReadType.ElementRef) { - return getOrCreateElementRef(nodeInjector); - } else if (read === QueryReadType.ViewContainerRef) { - return getOrCreateContainerRef(nodeInjector); - } else if (read === QueryReadType.TemplateRef) { - return getOrCreateTemplateRef(nodeInjector); - } - - if (ngDevMode) { - throw new Error(`Unrecognised read type for queries: ${read}`); - } -} - /** - * Goes over local names for a given node and returns directive index + * Iterates over local names for a given node and returns directive index * (or -1 if a local name points to an element). * * @param staticData static data of a node to check @@ -153,23 +127,68 @@ function getIdxOfMatchingSelector(staticData: LNodeStatic, selector: string): nu return null; } +/** + * Iterates over all the directives for a node and returns index of a directive for a given type. + * + * @param node Node on which directives are present. + * @param type Type of a directive to look for. + * @returns Index of a found directive or null when none found. + */ +function geIdxOfMatchingDirective(node: LNode, type: Type): number|null { + const ngStaticData = node.view.ngStaticData; + const flags = node.flags; + for (let i = flags >> LNodeFlags.INDX_SHIFT, + ii = i + ((flags & LNodeFlags.SIZE_MASK) >> LNodeFlags.SIZE_SHIFT); + i < ii; i++) { + const def = ngStaticData[i] as DirectiveDef; + if (def.diPublic && def.type === type) { + return i; + } + } + return null; +} + +function readDefaultInjectable(nodeInjector: LNodeInjector, node: LNode): viewEngine_ElementRef| + viewEngine_TemplateRef|undefined { + ngDevMode && assertNodeOfPossibleTypes(node, LNodeFlags.Container, LNodeFlags.Element); + if ((node.flags & LNodeFlags.TYPE_MASK) === LNodeFlags.Element) { + return getOrCreateElementRef(nodeInjector); + } else if ((node.flags & LNodeFlags.TYPE_MASK) === LNodeFlags.Container) { + return getOrCreateTemplateRef(nodeInjector); + } +} + +function readFromNodeInjector( + nodeInjector: LNodeInjector, node: LNode, read: QueryReadType | Type): any { + if (read === QueryReadType.ElementRef) { + return getOrCreateElementRef(nodeInjector); + } else if (read === QueryReadType.ViewContainerRef) { + return getOrCreateContainerRef(nodeInjector); + } else if (read === QueryReadType.TemplateRef) { + return getOrCreateTemplateRef(nodeInjector); + } else { + const matchingIdx = geIdxOfMatchingDirective(node, read); + if (matchingIdx !== null) { + return node.view.data[matchingIdx]; + } + } + return null; +} + function add(predicate: QueryPredicate| null, node: LNode) { const nodeInjector = getOrCreateNodeInjectorForNode(node as LElement | LContainer); while (predicate) { const type = predicate.type; if (type) { - const ngStaticData = node.view.ngStaticData; - const flags = node.flags; - for (let i = flags >> LNodeFlags.INDX_SHIFT, - ii = i + ((flags & LNodeFlags.SIZE_MASK) >> LNodeFlags.SIZE_SHIFT); - i < ii; i++) { - const def = ngStaticData[i] as DirectiveDef; - if (def.diPublic && def.type === type) { - if (predicate.read !== null) { - predicate.values.push(readFromNodeInjector(nodeInjector, node, predicate.read)); - } else { - predicate.values.push(node.view.data[i]); + const directiveIdx = geIdxOfMatchingDirective(node, type); + if (directiveIdx !== null) { + if (predicate.read !== null) { + const requestedRead = readFromNodeInjector(nodeInjector, node, predicate.read); + if (requestedRead !== null) { + predicate.values.push(requestedRead); } + } else { + predicate.values.push(node.view.data[directiveIdx]); } } } else { @@ -180,7 +199,10 @@ function add(predicate: QueryPredicate| null, node: LNode) { // is anything on a node matching a selector? if (directiveIdx !== null) { if (predicate.read !== null) { - predicate.values.push(readFromNodeInjector(nodeInjector, node, predicate.read)); + const requestedRead = readFromNodeInjector(nodeInjector, node, predicate.read); + if (requestedRead !== null) { + predicate.values.push(requestedRead); + } } else { // is local name pointing to a directive? if (directiveIdx > -1) { diff --git a/packages/core/test/render3/query_spec.ts b/packages/core/test/render3/query_spec.ts index 1df506fcc0..3e991f22a0 100644 --- a/packages/core/test/render3/query_spec.ts +++ b/packages/core/test/render3/query_spec.ts @@ -79,7 +79,7 @@ describe('query', () => { describe('types predicate', () => { - it('should query using type predicate and read specified token', () => { + it('should query using type predicate and read a specified token', () => { const Child = createDirective(); let elToQuery; /** @@ -106,6 +106,61 @@ describe('query', () => { expect(query.first.nativeElement).toEqual(elToQuery); }); + + it('should query using type predicate and read another directive type', () => { + const Child = createDirective(); + const OtherChild = createDirective(); + let otherChildInstance; + /** + *
+ * class Cmpt { + * @ViewChildren(Child, {read: OtherChild}) query; + * } + */ + const Cmpt = createComponent('cmpt', function(ctx: any, cm: boolean) { + let tmp: any; + if (cm) { + m(0, Q(Child, false, OtherChild)); + E(1, 'div'); + { + D(2, Child.ngDirectiveDef.n(), Child.ngDirectiveDef); + D(3, otherChildInstance = OtherChild.ngDirectiveDef.n(), OtherChild.ngDirectiveDef); + } + e(); + } + qR(tmp = m>(0)) && (ctx.query = tmp as QueryList); + }); + + const cmptInstance = renderComponent(Cmpt); + const query = (cmptInstance.query as QueryList); + expect(query.length).toBe(1); + expect(query.first).toBe(otherChildInstance); + }); + + it('should not add results to query if a requested token cant be read', () => { + const Child = createDirective(); + const OtherChild = createDirective(); + /** + *
+ * class Cmpt { + * @ViewChildren(Child, {read: OtherChild}) query; + * } + */ + const Cmpt = createComponent('cmpt', function(ctx: any, cm: boolean) { + let tmp: any; + if (cm) { + m(0, Q(Child, false, OtherChild)); + E(1, 'div'); + { D(2, Child.ngDirectiveDef.n(), Child.ngDirectiveDef); } + e(); + } + qR(tmp = m>(0)) && (ctx.query = tmp as QueryList); + }); + + const cmptInstance = renderComponent(Cmpt); + const query = (cmptInstance.query as QueryList); + expect(query.length).toBe(0); + }); }); describe('local names predicate', () => { @@ -461,5 +516,30 @@ describe('query', () => { expect(query.last).toBe(childInstance); }); + it('should not add results to query if a requested token cant be read', () => { + const Child = createDirective(); + + let childInstance, div; + /** + *
+ * class Cmpt { + * @ViewChildren('foo', {read: Child}) query; + * } + */ + const Cmpt = createComponent('cmpt', function(ctx: any, cm: boolean) { + let tmp: any; + if (cm) { + m(0, Q(['foo'], false, Child)); + div = E(1, 'div', [], 'foo'); + e(); + } + qR(tmp = m>(0)) && (ctx.query = tmp as QueryList); + }); + + const cmptInstance = renderComponent(Cmpt); + const query = (cmptInstance.query as QueryList); + expect(query.length).toBe(0); + }); + }); });