From 5dc8d287aac1824032aa4bd18a17ba5fdc67a6c3 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 1 Aug 2020 11:57:29 +0200 Subject: [PATCH] fix(core): queries not matching string injection tokens (#38321) Queries weren't matching directives that provide themselves via string injection tokens, because the assumption was that any string passed to a query decorator refers to a template reference. These changes make it so we match both template references and providers while giving precedence to the template references. Fixes #38313. Fixes #38315. PR Close #38321 --- packages/core/src/render3/di.ts | 7 +- packages/core/src/render3/query.ts | 17 +- packages/core/test/acceptance/query_spec.ts | 173 ++++++++++++++++++++ 3 files changed, 187 insertions(+), 10 deletions(-) diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 5aee59cdd6..0c799cbcf2 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -495,8 +495,8 @@ function searchTokensOnInjector( * @returns Index of a found directive or provider, or null when none found. */ export function locateDirectiveOrProvider( - tNode: TNode, tView: TView, token: Type|InjectionToken, canAccessViewProviders: boolean, - isHostSpecialCase: boolean|number): number|null { + tNode: TNode, tView: TView, token: Type|InjectionToken|string, + canAccessViewProviders: boolean, isHostSpecialCase: boolean|number): number|null { const nodeProviderIndexes = tNode.providerIndexes; const tInjectables = tView.data; @@ -510,7 +510,8 @@ export function locateDirectiveOrProvider( // When the host special case applies, only the viewProviders and the component are visible const endIndex = isHostSpecialCase ? injectablesStart + cptViewProvidersCount : directiveEnd; for (let i = startingIndex; i < endIndex; i++) { - const providerTokenOrDef = tInjectables[i] as InjectionToken| Type| DirectiveDef; + const providerTokenOrDef = + tInjectables[i] as InjectionToken| Type| DirectiveDef| string; if (i < directivesStart && token === providerTokenOrDef || i >= directivesStart && (providerTokenOrDef as DirectiveDef).type === token) { return i; diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index 58e587703e..3f135d8633 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -227,20 +227,23 @@ class TQuery_ implements TQuery { } private matchTNode(tView: TView, tNode: TNode): void { - if (Array.isArray(this.metadata.predicate)) { - const localNames = this.metadata.predicate; - for (let i = 0; i < localNames.length; i++) { - this.matchTNodeWithReadOption(tView, tNode, getIdxOfMatchingSelector(tNode, localNames[i])); + const predicate = this.metadata.predicate; + if (Array.isArray(predicate)) { + for (let i = 0; i < predicate.length; i++) { + const name = predicate[i]; + this.matchTNodeWithReadOption(tView, tNode, getIdxOfMatchingSelector(tNode, name)); + // Also try matching the name to a provider since strings can be used as DI tokens too. + this.matchTNodeWithReadOption( + tView, tNode, locateDirectiveOrProvider(tNode, tView, name, false, false)); } } else { - const typePredicate = this.metadata.predicate as any; - if (typePredicate === ViewEngine_TemplateRef) { + if ((predicate as any) === ViewEngine_TemplateRef) { if (tNode.type === TNodeType.Container) { this.matchTNodeWithReadOption(tView, tNode, -1); } } else { this.matchTNodeWithReadOption( - tView, tNode, locateDirectiveOrProvider(tNode, tView, typePredicate, false, false)); + tView, tNode, locateDirectiveOrProvider(tNode, tView, predicate, false, false)); } } } diff --git a/packages/core/test/acceptance/query_spec.ts b/packages/core/test/acceptance/query_spec.ts index 3ee127eed3..341f4f45c9 100644 --- a/packages/core/test/acceptance/query_spec.ts +++ b/packages/core/test/acceptance/query_spec.ts @@ -1631,6 +1631,179 @@ describe('query logic', () => { expect(groups[2]).toBeUndefined(); }); }); + + describe('querying for string token providers', () => { + @Directive({ + selector: '[text-token]', + providers: [{provide: 'Token', useExisting: TextTokenDirective}], + }) + class TextTokenDirective { + } + + it('should match string injection token in a ViewChild query', () => { + @Component({template: '
'}) + class App { + @ViewChild('Token') token: any; + } + + TestBed.configureTestingModule({declarations: [App, TextTokenDirective]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.componentInstance.token).toBeAnInstanceOf(TextTokenDirective); + }); + + it('should give precedence to local reference if both a reference and a string injection token provider match a ViewChild query', + () => { + @Component({template: '
'}) + class App { + @ViewChild('Token') token: any; + } + + TestBed.configureTestingModule({declarations: [App, TextTokenDirective]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.componentInstance.token).toBeAnInstanceOf(ElementRef); + }); + + it('should match string injection token in a ViewChildren query', () => { + @Component({template: '
'}) + class App { + @ViewChildren('Token') tokens!: QueryList; + } + + TestBed.configureTestingModule({declarations: [App, TextTokenDirective]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + const tokens = fixture.componentInstance.tokens; + expect(tokens.length).toBe(1); + expect(tokens.first).toBeAnInstanceOf(TextTokenDirective); + }); + + it('should match both string injection token and local reference inside a ViewChildren query', + () => { + @Component({template: '
'}) + class App { + @ViewChildren('Token') tokens!: QueryList; + } + + TestBed.configureTestingModule({declarations: [App, TextTokenDirective]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.tokens.toArray()).toEqual([ + jasmine.any(ElementRef), jasmine.any(TextTokenDirective) + ]); + }); + + it('should match string injection token in a ContentChild query', () => { + @Component({selector: 'has-query', template: ''}) + class HasQuery { + @ContentChild('Token') token: any; + } + + @Component({template: '
'}) + class App { + @ViewChild(HasQuery) queryComp!: HasQuery; + } + + TestBed.configureTestingModule({declarations: [App, HasQuery, TextTokenDirective]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.queryComp.token).toBeAnInstanceOf(TextTokenDirective); + }); + + it('should give precedence to local reference if both a reference and a string injection token provider match a ContentChild query', + () => { + @Component({selector: 'has-query', template: ''}) + class HasQuery { + @ContentChild('Token') token: any; + } + + @Component({template: '
'}) + class App { + @ViewChild(HasQuery) queryComp!: HasQuery; + } + + TestBed.configureTestingModule({declarations: [App, HasQuery, TextTokenDirective]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.queryComp.token).toBeAnInstanceOf(ElementRef); + }); + + it('should match string injection token in a ContentChildren query', () => { + @Component({selector: 'has-query', template: ''}) + class HasQuery { + @ContentChildren('Token') tokens!: QueryList; + } + + @Component({template: '
'}) + class App { + @ViewChild(HasQuery) queryComp!: HasQuery; + } + + TestBed.configureTestingModule({declarations: [App, HasQuery, TextTokenDirective]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + const tokens = fixture.componentInstance.queryComp.tokens; + expect(tokens.length).toBe(1); + expect(tokens.first).toBeAnInstanceOf(TextTokenDirective); + }); + + it('should match both string injection token and local reference inside a ContentChildren query', + () => { + @Component({selector: 'has-query', template: ''}) + class HasQuery { + @ContentChildren('Token') tokens!: QueryList; + } + + @Component({template: '
'}) + class App { + @ViewChild(HasQuery) queryComp!: HasQuery; + } + + TestBed.configureTestingModule({declarations: [App, HasQuery, TextTokenDirective]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.queryComp.tokens.toArray()).toEqual([ + jasmine.any(ElementRef), jasmine.any(TextTokenDirective) + ]); + }); + + it('should match string token specified through the `read` option of a view query', () => { + @Component({template: '
'}) + class App { + @ViewChild('Token', {read: 'Token'}) token: any; + } + + TestBed.configureTestingModule({declarations: [App, TextTokenDirective]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.componentInstance.token).toBeAnInstanceOf(TextTokenDirective); + }); + + it('should match string token specified through the `read` option of a content query', () => { + @Component({selector: 'has-query', template: ''}) + class HasQuery { + @ContentChild('Token', {read: 'Token'}) token: any; + } + + @Component({template: '
'}) + class App { + @ViewChild(HasQuery) queryComp!: HasQuery; + } + + TestBed.configureTestingModule({declarations: [App, HasQuery, TextTokenDirective]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.queryComp.token).toBeAnInstanceOf(TextTokenDirective); + }); + }); }); function initWithTemplate(compType: Type, template: string) {