From d767e0b2c06fe9ba25a72302c59c3f7638dbff2f Mon Sep 17 00:00:00 2001 From: JoostK Date: Sun, 18 Nov 2018 21:04:05 +0100 Subject: [PATCH] fix(ivy): consider providers for view/content queries (#27156) In ViewEngine it is possible to query for a token that is provided by a directive that is in scope. See StackBlitz for example: https://stackblitz.com/edit/ng-viewengine-viewchild-providers Material uses this pattern with its `MatFormFieldControl` setup, see https://bit.ly/2zgCUxD for documentation. PR Close #27156 --- packages/core/src/render3/di.ts | 35 +++++- packages/core/src/render3/query.ts | 40 ++---- .../bundle.golden_symbols.json | 3 + .../hello_world_r2/bundle.golden_symbols.json | 3 + .../bundling/todo/bundle.golden_symbols.json | 3 + .../todo_r2/bundle.golden_symbols.json | 3 + packages/core/test/render3/providers_spec.ts | 8 +- packages/core/test/render3/query_spec.ts | 119 +++++++++++++++++- 8 files changed, 170 insertions(+), 44 deletions(-) diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index cf605d17b3..22cb779edb 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -396,9 +396,6 @@ function searchTokensOnInjector( previousTView: TView | null) { const currentTView = injectorView[TVIEW]; const tNode = currentTView.data[injectorIndex + TNODE] as TNode; - const nodeFlags = tNode.flags; - const nodeProviderIndexes = tNode.providerIndexes; - const tInjectables = currentTView.data; // First, we step through providers let canAccessViewProviders = false; // We need to determine if view providers can be accessed by the starting element. @@ -412,9 +409,35 @@ function searchTokensOnInjector( // and check the host node of the current view to identify component views. if (previousTView == null && isComponent(tNode) && includeViewProviders || previousTView != null && previousTView != currentTView && - (currentTView.node == null || currentTView.node !.type === TNodeType.Element)) { + (currentTView.node == null || currentTView.node.type === TNodeType.Element)) { canAccessViewProviders = true; } + const injectableIdx = + locateDirectiveOrProvider(tNode, injectorView, token, canAccessViewProviders); + if (injectableIdx !== null) { + return getNodeInjectable(currentTView.data, injectorView, injectableIdx, tNode as TElementNode); + } else { + return NOT_FOUND; + } +} + +/** + * Searches for the given token among the node's directives and providers. + * + * @param tNode TNode on which directives are present. + * @param view The view we are currently processing + * @param token Provider token or type of a directive to look for. + * @param canAccessViewProviders Whether view providers should be considered. + * @returns Index of a found directive or provider, or null when none found. + */ +export function locateDirectiveOrProvider( + tNode: TNode, view: LViewData, token: Type| InjectionToken, + canAccessViewProviders: boolean): number|null { + const tView = view[TVIEW]; + const nodeFlags = tNode.flags; + const nodeProviderIndexes = tNode.providerIndexes; + const tInjectables = tView.data; + const startInjectables = nodeProviderIndexes & TNodeProviderIndexes.ProvidersStartIndexMask; const startDirectives = nodeFlags >> TNodeFlags.DirectiveStartingIndexShift; const cptViewProvidersCount = @@ -426,10 +449,10 @@ function searchTokensOnInjector( const providerTokenOrDef = tInjectables[i] as InjectionToken| Type| DirectiveDef; if (i < startDirectives && token === providerTokenOrDef || i >= startDirectives && (providerTokenOrDef as DirectiveDef).type === token) { - return getNodeInjectable(tInjectables, injectorView, i, tNode as TElementNode); + return i; } } - return NOT_FOUND; + return null; } /** diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index 54e90eb411..cdb0907025 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -18,11 +18,12 @@ import {Type} from '../type'; import {getSymbolIterator} from '../util'; import {assertDefined, assertEqual} from './assert'; +import {getNodeInjectable, locateDirectiveOrProvider} from './di'; import {NG_ELEMENT_ID} from './fields'; import {store, storeCleanupWithContext} from './instructions'; -import {DirectiveDef, unusedValueExportToPlacateAjd as unused1} from './interfaces/definition'; +import {unusedValueExportToPlacateAjd as unused1} from './interfaces/definition'; import {unusedValueExportToPlacateAjd as unused2} from './interfaces/injector'; -import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeFlags, TNodeType, unusedValueExportToPlacateAjd as unused3} from './interfaces/node'; +import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, unusedValueExportToPlacateAjd as unused3} from './interfaces/node'; import {LQueries, unusedValueExportToPlacateAjd as unused4} from './interfaces/query'; import {LViewData, TVIEW} from './interfaces/view'; import {assertPreviousIsParent, getOrCreateCurrentQueries, getViewData} from './state'; @@ -242,31 +243,6 @@ function getIdxOfMatchingSelector(tNode: TNode, selector: string): number|null { return null; } -/** - * Iterates over all the directives for a node and returns index of a directive for a given type. - * - * @param tNode TNode on which directives are present. - * @param currentView The view we are currently processing - * @param type Type of a directive to look for. - * @returns Index of a found directive or null when none found. - */ -function getIdxOfMatchingDirective(tNode: TNode, currentView: LViewData, type: Type): number| - null { - const defs = currentView[TVIEW].data; - if (defs) { - const flags = tNode.flags; - const count = flags & TNodeFlags.DirectiveCountMask; - const start = flags >> TNodeFlags.DirectiveStartingIndexShift; - const end = start + count; - for (let i = start; i < end; i++) { - const def = defs[i] as DirectiveDef; - if (def.type === type) { - return i; - } - } - } - return null; -} // TODO: "read" should be an AbstractType (FW-486) function queryByReadToken(read: any, tNode: TNode, currentView: LViewData): any { @@ -274,9 +250,10 @@ function queryByReadToken(read: any, tNode: TNode, currentView: LViewData): any if (typeof factoryFn === 'function') { return factoryFn(); } else { - const matchingIdx = getIdxOfMatchingDirective(tNode, currentView, read as Type); + const matchingIdx = locateDirectiveOrProvider(tNode, currentView, read as Type, false); if (matchingIdx !== null) { - return currentView[matchingIdx]; + return getNodeInjectable( + currentView[TVIEW].data, currentView, matchingIdx, tNode as TElementNode); } } return null; @@ -307,7 +284,8 @@ function queryRead(tNode: TNode, currentView: LViewData, read: any, matchingIdx: return queryByReadToken(read, tNode, currentView); } if (matchingIdx > -1) { - return currentView[matchingIdx]; + return getNodeInjectable( + currentView[TVIEW].data, currentView, matchingIdx, tNode as TElementNode); } // if read token and / or strategy is not specified, // detect it using appropriate tNode type @@ -326,7 +304,7 @@ function add( if (type === ViewEngine_TemplateRef) { result = queryByTemplateRef(type, tNode, currentView, predicate.read); } else { - const matchingIdx = getIdxOfMatchingDirective(tNode, currentView, type); + const matchingIdx = locateDirectiveOrProvider(tNode, currentView, type, false); if (matchingIdx !== null) { result = queryRead(tNode, currentView, predicate.read, matchingIdx); } diff --git a/packages/core/test/bundling/animation_world/bundle.golden_symbols.json b/packages/core/test/bundling/animation_world/bundle.golden_symbols.json index 8007265b47..f211e5957d 100644 --- a/packages/core/test/bundling/animation_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animation_world/bundle.golden_symbols.json @@ -929,6 +929,9 @@ { "name": "loadContext" }, + { + "name": "locateDirectiveOrProvider" + }, { "name": "locateHostElement" }, diff --git a/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json index 84bb286881..e34ca82a65 100644 --- a/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json @@ -1100,6 +1100,9 @@ { "name": "leaveView" }, + { + "name": "locateDirectiveOrProvider" + }, { "name": "locateHostElement" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 427f0c9959..2b08727a48 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -947,6 +947,9 @@ { "name": "loadInternal" }, + { + "name": "locateDirectiveOrProvider" + }, { "name": "locateHostElement" }, diff --git a/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json b/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json index 6e0a529556..0fa4d1bac2 100644 --- a/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json @@ -2198,6 +2198,9 @@ { "name": "localeEn" }, + { + "name": "locateDirectiveOrProvider" + }, { "name": "locateHostElement" }, diff --git a/packages/core/test/render3/providers_spec.ts b/packages/core/test/render3/providers_spec.ts index 67ad097d3c..b5cedd99c4 100644 --- a/packages/core/test/render3/providers_spec.ts +++ b/packages/core/test/render3/providers_spec.ts @@ -1298,7 +1298,7 @@ function expectProvidersScenario(defs: { } class ViewChildDirective { - static ngComponentDef = defineDirective({ + static ngDirectiveDef = defineDirective({ type: ViewChildDirective, selectors: [['view-child']], factory: () => testDirectiveInjection(defs.viewChild, new ViewChildDirective()), @@ -1325,7 +1325,7 @@ function expectProvidersScenario(defs: { } class ContentChildDirective { - static ngComponentDef = defineDirective({ + static ngDirectiveDef = defineDirective({ type: ContentChildDirective, selectors: [['content-child']], factory: () => testDirectiveInjection(defs.contentChild, new ContentChildDirective()), @@ -1353,7 +1353,7 @@ function expectProvidersScenario(defs: { } class ParentDirective { - static ngComponentDef = defineDirective({ + static ngDirectiveDef = defineDirective({ type: ParentDirective, selectors: [['parent']], factory: () => testDirectiveInjection(defs.parent, new ParentDirective()), @@ -1362,7 +1362,7 @@ function expectProvidersScenario(defs: { } class ParentDirective2 { - static ngComponentDef = defineDirective({ + static ngDirectiveDef = defineDirective({ type: ParentDirective2, selectors: [['parent']], factory: () => testDirectiveInjection(defs.parent, new ParentDirective2()), diff --git a/packages/core/test/render3/query_spec.ts b/packages/core/test/render3/query_spec.ts index 3e819a7a0b..20537d38fb 100644 --- a/packages/core/test/render3/query_spec.ts +++ b/packages/core/test/render3/query_spec.ts @@ -10,9 +10,9 @@ import {NgForOfContext} from '@angular/common'; import {ElementRef, TemplateRef, ViewContainerRef} from '@angular/core'; import {EventEmitter} from '../..'; - -import {AttributeMarker, QueryList, defineComponent, defineDirective, detectChanges} from '../../src/render3/index'; +import {AttributeMarker, ProvidersFeature, QueryList, 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 {RenderFlags} from '../../src/render3/interfaces/definition'; import {query, queryRefresh} from '../../src/render3/query'; @@ -205,6 +205,119 @@ describe('query', () => { }); }); + describe('providers', () => { + + class Service {} + class Alias {} + + let directive: MyDirective|null = null; + + class MyDirective { + constructor(public service: Service) {} + + static ngDirectiveDef = defineDirective({ + type: MyDirective, + selectors: [['', 'myDir', '']], + factory: function MyDirective_Factory() { + return directive = new MyDirective(directiveInject(Service)); + }, + features: [ProvidersFeature([Service, {provide: Alias, useExisting: Service}])], + }); + } + + beforeEach(() => directive = null); + + // https://stackblitz.com/edit/ng-viewengine-viewchild-providers?file=src%2Fapp%2Fapp.component.ts + it('should query for providers that are present on a directive', () => { + + /** + *
+ * class App { + * @ViewChild(MyDirective) directive: MyDirective; + * @ViewChild(Service) service: Service; + * @ViewChild(Alias) alias: Alias; + * } + */ + class App { + directive?: MyDirective; + service?: Service; + alias?: Alias; + + static ngComponentDef = defineComponent({ + type: App, + selectors: [['app']], + consts: 4, + vars: 0, + factory: function App_Factory() { return new App(); }, + template: function App_Template(rf: RenderFlags, ctx: App) { + if (rf & RenderFlags.Create) { + element(3, 'div', ['myDir']); + } + }, + viewQuery: function(rf: RenderFlags, ctx: App) { + let tmp: any; + if (rf & RenderFlags.Create) { + query(0, MyDirective, false); + query(1, Service, false); + query(2, Alias, false); + } + if (rf & RenderFlags.Update) { + queryRefresh(tmp = load>(0)) && (ctx.directive = tmp.first); + queryRefresh(tmp = load>(1)) && (ctx.service = tmp.first); + queryRefresh(tmp = load>(2)) && (ctx.alias = tmp.first); + } + }, + directives: [MyDirective] + }); + } + + const componentFixture = new ComponentFixture(App); + expect(componentFixture.component.directive).toBe(directive !); + expect(componentFixture.component.service).toBe(directive !.service); + expect(componentFixture.component.alias).toBe(directive !.service); + }); + + it('should resolve a provider if given as read token', () => { + + /** + *
+ * class App { + * @ViewChild(MyDirective, {read: Alias}}) service: Service; + * } + */ + class App { + service?: Service; + + static ngComponentDef = defineComponent({ + type: App, + selectors: [['app']], + consts: 2, + vars: 0, + factory: function App_Factory() { return new App(); }, + template: function App_Template(rf: RenderFlags, ctx: App) { + if (rf & RenderFlags.Create) { + element(1, 'div', ['myDir']); + } + }, + viewQuery: function(rf: RenderFlags, ctx: App) { + let tmp: any; + if (rf & RenderFlags.Create) { + query(0, MyDirective, false, Alias); + } + if (rf & RenderFlags.Update) { + queryRefresh(tmp = load>(0)) && (ctx.service = tmp.first); + } + }, + directives: [MyDirective] + }); + } + + const componentFixture = new ComponentFixture(App); + expect(componentFixture.component.service).toBe(directive !.service); + }); + + }); + describe('local names', () => { it('should query for a single element and read ElementRef by default', () => { @@ -2122,7 +2235,7 @@ describe('query', () => { this.contentCheckedQuerySnapshot = this.foos ? this.foos.length : 0; } - static ngComponentDef = defineDirective({ + static ngDirectiveDef = defineDirective({ type: WithContentDirective, selectors: [['', 'with-content', '']], factory: () => new WithContentDirective(),