Skip to content

Commit

Permalink
fix(core): queries not matching string injection tokens (#38321)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@crisbeto @AndrewKushnir
crisbeto authored and AndrewKushnir committed Aug 10, 2020
1 parent c9acb7b commit 32109dc
Show file tree
Hide file tree
Showing 3 changed files with 187 additions and 10 deletions.
7 changes: 4 additions & 3 deletions packages/core/src/render3/di.ts
View file
Expand Up @@ -495,8 +495,8 @@ function searchTokensOnInjector<T>(
* @returns Index of a found directive or provider, or null when none found.
*/
export function locateDirectiveOrProvider<T>(
tNode: TNode, tView: TView, token: Type<T>|InjectionToken<T>, canAccessViewProviders: boolean,
isHostSpecialCase: boolean|number): number|null {
tNode: TNode, tView: TView, token: Type<T>|InjectionToken<T>|string,
canAccessViewProviders: boolean, isHostSpecialCase: boolean|number): number|null {
const nodeProviderIndexes = tNode.providerIndexes;
const tInjectables = tView.data;

Expand All @@ -510,7 +510,8 @@ export function locateDirectiveOrProvider<T>(
// 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<any>| Type<any>| DirectiveDef<any>;
const providerTokenOrDef =
tInjectables[i] as InjectionToken<any>| Type<any>| DirectiveDef<any>| string;
if (i < directivesStart && token === providerTokenOrDef ||
i >= directivesStart && (providerTokenOrDef as DirectiveDef<any>).type === token) {
return i;
Expand Down
17 changes: 10 additions & 7 deletions packages/core/src/render3/query.ts
View file
Expand Up @@ -226,20 +226,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));
}
}
}
Expand Down
173 changes: 173 additions & 0 deletions packages/core/test/acceptance/query_spec.ts
View file
Expand Up @@ -1594,6 +1594,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: '<div text-token></div>'})
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: '<div text-token #Token></div>'})
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: '<div text-token></div>'})
class App {
@ViewChildren('Token') tokens!: QueryList<any>;
}

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: '<div text-token #Token></div>'})
class App {
@ViewChildren('Token') tokens!: QueryList<any>;
}

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: '<ng-content></ng-content>'})
class HasQuery {
@ContentChild('Token') token: any;
}

@Component({template: '<has-query><div text-token></div></has-query>'})
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: '<ng-content></ng-content>'})
class HasQuery {
@ContentChild('Token') token: any;
}

@Component({template: '<has-query><div text-token #Token></div></has-query>'})
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: '<ng-content></ng-content>'})
class HasQuery {
@ContentChildren('Token') tokens!: QueryList<any>;
}

@Component({template: '<has-query><div text-token></div></has-query>'})
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: '<ng-content></ng-content>'})
class HasQuery {
@ContentChildren('Token') tokens!: QueryList<any>;
}

@Component({template: '<has-query><div text-token #Token></div></has-query>'})
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: '<div text-token #Token></div>'})
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: '<ng-content></ng-content>'})
class HasQuery {
@ContentChild('Token', {read: 'Token'}) token: any;
}

@Component({template: '<has-query><div text-token #Token></div></has-query>'})
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<any>, template: string) {
Expand Down

0 comments on commit 32109dc

Please sign in to comment.