Skip to content

Commit 5dc8d28

Browse files
crisbetoAndrewKushnir
authored andcommitted
fix(core): queries not matching string injection tokens (angular#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 angular#38313. Fixes angular#38315. PR Close angular#38321
1 parent 6da9e58 commit 5dc8d28

File tree

3 files changed

+187
-10
lines changed

3 files changed

+187
-10
lines changed

packages/core/src/render3/di.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -495,8 +495,8 @@ function searchTokensOnInjector<T>(
495495
* @returns Index of a found directive or provider, or null when none found.
496496
*/
497497
export function locateDirectiveOrProvider<T>(
498-
tNode: TNode, tView: TView, token: Type<T>|InjectionToken<T>, canAccessViewProviders: boolean,
499-
isHostSpecialCase: boolean|number): number|null {
498+
tNode: TNode, tView: TView, token: Type<T>|InjectionToken<T>|string,
499+
canAccessViewProviders: boolean, isHostSpecialCase: boolean|number): number|null {
500500
const nodeProviderIndexes = tNode.providerIndexes;
501501
const tInjectables = tView.data;
502502

@@ -510,7 +510,8 @@ export function locateDirectiveOrProvider<T>(
510510
// When the host special case applies, only the viewProviders and the component are visible
511511
const endIndex = isHostSpecialCase ? injectablesStart + cptViewProvidersCount : directiveEnd;
512512
for (let i = startingIndex; i < endIndex; i++) {
513-
const providerTokenOrDef = tInjectables[i] as InjectionToken<any>| Type<any>| DirectiveDef<any>;
513+
const providerTokenOrDef =
514+
tInjectables[i] as InjectionToken<any>| Type<any>| DirectiveDef<any>| string;
514515
if (i < directivesStart && token === providerTokenOrDef ||
515516
i >= directivesStart && (providerTokenOrDef as DirectiveDef<any>).type === token) {
516517
return i;

packages/core/src/render3/query.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -227,20 +227,23 @@ class TQuery_ implements TQuery {
227227
}
228228

229229
private matchTNode(tView: TView, tNode: TNode): void {
230-
if (Array.isArray(this.metadata.predicate)) {
231-
const localNames = this.metadata.predicate;
232-
for (let i = 0; i < localNames.length; i++) {
233-
this.matchTNodeWithReadOption(tView, tNode, getIdxOfMatchingSelector(tNode, localNames[i]));
230+
const predicate = this.metadata.predicate;
231+
if (Array.isArray(predicate)) {
232+
for (let i = 0; i < predicate.length; i++) {
233+
const name = predicate[i];
234+
this.matchTNodeWithReadOption(tView, tNode, getIdxOfMatchingSelector(tNode, name));
235+
// Also try matching the name to a provider since strings can be used as DI tokens too.
236+
this.matchTNodeWithReadOption(
237+
tView, tNode, locateDirectiveOrProvider(tNode, tView, name, false, false));
234238
}
235239
} else {
236-
const typePredicate = this.metadata.predicate as any;
237-
if (typePredicate === ViewEngine_TemplateRef) {
240+
if ((predicate as any) === ViewEngine_TemplateRef) {
238241
if (tNode.type === TNodeType.Container) {
239242
this.matchTNodeWithReadOption(tView, tNode, -1);
240243
}
241244
} else {
242245
this.matchTNodeWithReadOption(
243-
tView, tNode, locateDirectiveOrProvider(tNode, tView, typePredicate, false, false));
246+
tView, tNode, locateDirectiveOrProvider(tNode, tView, predicate, false, false));
244247
}
245248
}
246249
}

packages/core/test/acceptance/query_spec.ts

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1631,6 +1631,179 @@ describe('query logic', () => {
16311631
expect(groups[2]).toBeUndefined();
16321632
});
16331633
});
1634+
1635+
describe('querying for string token providers', () => {
1636+
@Directive({
1637+
selector: '[text-token]',
1638+
providers: [{provide: 'Token', useExisting: TextTokenDirective}],
1639+
})
1640+
class TextTokenDirective {
1641+
}
1642+
1643+
it('should match string injection token in a ViewChild query', () => {
1644+
@Component({template: '<div text-token></div>'})
1645+
class App {
1646+
@ViewChild('Token') token: any;
1647+
}
1648+
1649+
TestBed.configureTestingModule({declarations: [App, TextTokenDirective]});
1650+
const fixture = TestBed.createComponent(App);
1651+
fixture.detectChanges();
1652+
expect(fixture.componentInstance.token).toBeAnInstanceOf(TextTokenDirective);
1653+
});
1654+
1655+
it('should give precedence to local reference if both a reference and a string injection token provider match a ViewChild query',
1656+
() => {
1657+
@Component({template: '<div text-token #Token></div>'})
1658+
class App {
1659+
@ViewChild('Token') token: any;
1660+
}
1661+
1662+
TestBed.configureTestingModule({declarations: [App, TextTokenDirective]});
1663+
const fixture = TestBed.createComponent(App);
1664+
fixture.detectChanges();
1665+
expect(fixture.componentInstance.token).toBeAnInstanceOf(ElementRef);
1666+
});
1667+
1668+
it('should match string injection token in a ViewChildren query', () => {
1669+
@Component({template: '<div text-token></div>'})
1670+
class App {
1671+
@ViewChildren('Token') tokens!: QueryList<any>;
1672+
}
1673+
1674+
TestBed.configureTestingModule({declarations: [App, TextTokenDirective]});
1675+
const fixture = TestBed.createComponent(App);
1676+
fixture.detectChanges();
1677+
1678+
const tokens = fixture.componentInstance.tokens;
1679+
expect(tokens.length).toBe(1);
1680+
expect(tokens.first).toBeAnInstanceOf(TextTokenDirective);
1681+
});
1682+
1683+
it('should match both string injection token and local reference inside a ViewChildren query',
1684+
() => {
1685+
@Component({template: '<div text-token #Token></div>'})
1686+
class App {
1687+
@ViewChildren('Token') tokens!: QueryList<any>;
1688+
}
1689+
1690+
TestBed.configureTestingModule({declarations: [App, TextTokenDirective]});
1691+
const fixture = TestBed.createComponent(App);
1692+
fixture.detectChanges();
1693+
1694+
expect(fixture.componentInstance.tokens.toArray()).toEqual([
1695+
jasmine.any(ElementRef), jasmine.any(TextTokenDirective)
1696+
]);
1697+
});
1698+
1699+
it('should match string injection token in a ContentChild query', () => {
1700+
@Component({selector: 'has-query', template: '<ng-content></ng-content>'})
1701+
class HasQuery {
1702+
@ContentChild('Token') token: any;
1703+
}
1704+
1705+
@Component({template: '<has-query><div text-token></div></has-query>'})
1706+
class App {
1707+
@ViewChild(HasQuery) queryComp!: HasQuery;
1708+
}
1709+
1710+
TestBed.configureTestingModule({declarations: [App, HasQuery, TextTokenDirective]});
1711+
const fixture = TestBed.createComponent(App);
1712+
fixture.detectChanges();
1713+
1714+
expect(fixture.componentInstance.queryComp.token).toBeAnInstanceOf(TextTokenDirective);
1715+
});
1716+
1717+
it('should give precedence to local reference if both a reference and a string injection token provider match a ContentChild query',
1718+
() => {
1719+
@Component({selector: 'has-query', template: '<ng-content></ng-content>'})
1720+
class HasQuery {
1721+
@ContentChild('Token') token: any;
1722+
}
1723+
1724+
@Component({template: '<has-query><div text-token #Token></div></has-query>'})
1725+
class App {
1726+
@ViewChild(HasQuery) queryComp!: HasQuery;
1727+
}
1728+
1729+
TestBed.configureTestingModule({declarations: [App, HasQuery, TextTokenDirective]});
1730+
const fixture = TestBed.createComponent(App);
1731+
fixture.detectChanges();
1732+
1733+
expect(fixture.componentInstance.queryComp.token).toBeAnInstanceOf(ElementRef);
1734+
});
1735+
1736+
it('should match string injection token in a ContentChildren query', () => {
1737+
@Component({selector: 'has-query', template: '<ng-content></ng-content>'})
1738+
class HasQuery {
1739+
@ContentChildren('Token') tokens!: QueryList<any>;
1740+
}
1741+
1742+
@Component({template: '<has-query><div text-token></div></has-query>'})
1743+
class App {
1744+
@ViewChild(HasQuery) queryComp!: HasQuery;
1745+
}
1746+
1747+
TestBed.configureTestingModule({declarations: [App, HasQuery, TextTokenDirective]});
1748+
const fixture = TestBed.createComponent(App);
1749+
fixture.detectChanges();
1750+
1751+
const tokens = fixture.componentInstance.queryComp.tokens;
1752+
expect(tokens.length).toBe(1);
1753+
expect(tokens.first).toBeAnInstanceOf(TextTokenDirective);
1754+
});
1755+
1756+
it('should match both string injection token and local reference inside a ContentChildren query',
1757+
() => {
1758+
@Component({selector: 'has-query', template: '<ng-content></ng-content>'})
1759+
class HasQuery {
1760+
@ContentChildren('Token') tokens!: QueryList<any>;
1761+
}
1762+
1763+
@Component({template: '<has-query><div text-token #Token></div></has-query>'})
1764+
class App {
1765+
@ViewChild(HasQuery) queryComp!: HasQuery;
1766+
}
1767+
1768+
TestBed.configureTestingModule({declarations: [App, HasQuery, TextTokenDirective]});
1769+
const fixture = TestBed.createComponent(App);
1770+
fixture.detectChanges();
1771+
1772+
expect(fixture.componentInstance.queryComp.tokens.toArray()).toEqual([
1773+
jasmine.any(ElementRef), jasmine.any(TextTokenDirective)
1774+
]);
1775+
});
1776+
1777+
it('should match string token specified through the `read` option of a view query', () => {
1778+
@Component({template: '<div text-token #Token></div>'})
1779+
class App {
1780+
@ViewChild('Token', {read: 'Token'}) token: any;
1781+
}
1782+
1783+
TestBed.configureTestingModule({declarations: [App, TextTokenDirective]});
1784+
const fixture = TestBed.createComponent(App);
1785+
fixture.detectChanges();
1786+
expect(fixture.componentInstance.token).toBeAnInstanceOf(TextTokenDirective);
1787+
});
1788+
1789+
it('should match string token specified through the `read` option of a content query', () => {
1790+
@Component({selector: 'has-query', template: '<ng-content></ng-content>'})
1791+
class HasQuery {
1792+
@ContentChild('Token', {read: 'Token'}) token: any;
1793+
}
1794+
1795+
@Component({template: '<has-query><div text-token #Token></div></has-query>'})
1796+
class App {
1797+
@ViewChild(HasQuery) queryComp!: HasQuery;
1798+
}
1799+
1800+
TestBed.configureTestingModule({declarations: [App, HasQuery, TextTokenDirective]});
1801+
const fixture = TestBed.createComponent(App);
1802+
fixture.detectChanges();
1803+
1804+
expect(fixture.componentInstance.queryComp.token).toBeAnInstanceOf(TextTokenDirective);
1805+
});
1806+
});
16341807
});
16351808

16361809
function initWithTemplate(compType: Type<any>, template: string) {

0 commit comments

Comments
 (0)