Skip to content

Commit 7b5e12c

Browse files
petebacondarwinpetebacondarwin
committed
fix(compiler): ensure that partially compiled queries can handle forward references
When a partially compiled component or directive is "linked" in JIT mode, the body of its declaration is evaluated by the JavaScript runtime. If a class is referenced in a query (e.g. `ViewQuery` or `ContentQuery`) but its definition is later in the file, then the reference must be wrapped in a `forwardRef()` call. Previously, query predicates were not wrapped correctly in partial declarations causing the code to crash at runtime. In AOT mode, this code is never evaluated but instead transformed as part of the build, so this bug did not become apparent until Angular Material started running JIT mode tests on its distributable output. This change fixes this problem by noting when queries are wrapped in `forwardRef()` calls and ensuring that this gets passed through to partial compilation declarations and then suitably stripped during linking. See angular/components#23882 and angular/components#23907
1 parent 2c269c3 commit 7b5e12c

File tree

15 files changed

+382
-9
lines changed

15 files changed

+382
-9
lines changed

‎packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_directive_linker_1.ts

+6
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,12 @@ function toQueryMetadata<TExpression>(obj: AstObject<R3DeclareQueryMetadata, TEx
139139
const predicateExpr = obj.getValue('predicate');
140140
if (predicateExpr.isArray()) {
141141
predicate = predicateExpr.getArray().map(entry => entry.getString());
142+
} else if (
143+
obj.has('isForwardRef') && obj.getBoolean('isForwardRef') &&
144+
predicateExpr.isCallExpression()) {
145+
const forwardRefArg = predicateExpr.getArguments()[0] as AstValue<Function, TExpression>;
146+
predicate = forwardRefArg.getFunctionReturnValue().getOpaque();
147+
142148
} else {
143149
predicate = predicateExpr.getOpaque();
144150
}

‎packages/compiler-cli/src/ngtsc/annotations/src/directive.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,9 @@ export function extractQueryMetadata(
531531
ErrorCode.DECORATOR_ARITY_WRONG, exprNode, `@${name} must have arguments`);
532532
}
533533
const first = name === 'ViewChild' || name === 'ContentChild';
534-
const node = tryUnwrapForwardRef(args[0], reflector) ?? args[0];
534+
const forwardReferenceTarget = tryUnwrapForwardRef(args[0], reflector);
535+
const node = forwardReferenceTarget ?? args[0];
536+
535537
const arg = evaluator.evaluate(node);
536538

537539
/** Whether or not this query should collect only static results (see view/api.ts) */
@@ -606,6 +608,7 @@ export function extractQueryMetadata(
606608
return {
607609
propertyName,
608610
predicate,
611+
isForwardRef: forwardReferenceTarget !== null,
609612
first,
610613
descendants,
611614
read,

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/GOLDEN_PARTIAL.js

+176
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,92 @@ export declare class MyModule {
7979
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
8080
}
8181

82+
/****************************************************************************************************
83+
* PARTIAL FILE: view_query_forward_ref.js
84+
****************************************************************************************************/
85+
import { Component, Directive, forwardRef, NgModule, ViewChild, ViewChildren } from '@angular/core';
86+
import * as i0 from "@angular/core";
87+
export class ViewQueryComponent {
88+
}
89+
ViewQueryComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ViewQueryComponent, deps: [], target: i0.ɵɵFactoryTarget.Component });
90+
ViewQueryComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: ViewQueryComponent, selector: "view-query-component", viewQueries: [{ propertyName: "someDir", first: true, predicate: i0.forwardRef(function () { return SomeDirective; }), isForwardRef: true, descendants: true }, { propertyName: "someDirList", predicate: i0.forwardRef(function () { return SomeDirective; }), isForwardRef: true, descendants: true }], ngImport: i0, template: `
91+
<div someDir></div>
92+
`, isInline: true, directives: [{ type: i0.forwardRef(function () { return SomeDirective; }), selector: "[someDir]" }] });
93+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ViewQueryComponent, decorators: [{
94+
type: Component,
95+
args: [{
96+
selector: 'view-query-component',
97+
template: `
98+
<div someDir></div>
99+
`
100+
}]
101+
}], propDecorators: { someDir: [{
102+
type: ViewChild,
103+
args: [forwardRef(() => SomeDirective)]
104+
}], someDirList: [{
105+
type: ViewChildren,
106+
args: [forwardRef(() => SomeDirective)]
107+
}] } });
108+
export class MyApp {
109+
}
110+
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
111+
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "my-app", ngImport: i0, template: `
112+
<view-query-component></view-query-component>
113+
`, isInline: true, components: [{ type: ViewQueryComponent, selector: "view-query-component" }] });
114+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
115+
type: Component,
116+
args: [{
117+
selector: 'my-app',
118+
template: `
119+
<view-query-component></view-query-component>
120+
`
121+
}]
122+
}] });
123+
export class SomeDirective {
124+
}
125+
SomeDirective.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, deps: [], target: i0.ɵɵFactoryTarget.Directive });
126+
SomeDirective.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: SomeDirective, selector: "[someDir]", ngImport: i0 });
127+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, decorators: [{
128+
type: Directive,
129+
args: [{
130+
selector: '[someDir]',
131+
}]
132+
}] });
133+
export class MyModule {
134+
}
135+
MyModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule });
136+
MyModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, declarations: [SomeDirective, ViewQueryComponent, MyApp] });
137+
MyModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule });
138+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, decorators: [{
139+
type: NgModule,
140+
args: [{ declarations: [SomeDirective, ViewQueryComponent, MyApp] }]
141+
}] });
142+
143+
/****************************************************************************************************
144+
* PARTIAL FILE: view_query_forward_ref.d.ts
145+
****************************************************************************************************/
146+
import { QueryList } from '@angular/core';
147+
import * as i0 from "@angular/core";
148+
export declare class ViewQueryComponent {
149+
someDir: SomeDirective;
150+
someDirList: QueryList<SomeDirective>;
151+
static ɵfac: i0.ɵɵFactoryDeclaration<ViewQueryComponent, never>;
152+
static ɵcmp: i0.ɵɵComponentDeclaration<ViewQueryComponent, "view-query-component", never, {}, {}, never, never>;
153+
}
154+
export declare class MyApp {
155+
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
156+
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "my-app", never, {}, {}, never, never>;
157+
}
158+
export declare class SomeDirective {
159+
static ɵfac: i0.ɵɵFactoryDeclaration<SomeDirective, never>;
160+
static ɵdir: i0.ɵɵDirectiveDeclaration<SomeDirective, "[someDir]", never, {}, {}, never>;
161+
}
162+
export declare class MyModule {
163+
static ɵfac: i0.ɵɵFactoryDeclaration<MyModule, never>;
164+
static ɵmod: i0.ɵɵNgModuleDeclaration<MyModule, [typeof SomeDirective, typeof ViewQueryComponent, typeof MyApp], never, never>;
165+
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
166+
}
167+
82168
/****************************************************************************************************
83169
* PARTIAL FILE: view_query_for_local_ref.js
84170
****************************************************************************************************/
@@ -410,6 +496,96 @@ export declare class MyModule {
410496
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
411497
}
412498

499+
/****************************************************************************************************
500+
* PARTIAL FILE: content_query_forward_ref.js
501+
****************************************************************************************************/
502+
import { Component, ContentChild, ContentChildren, Directive, forwardRef, NgModule } from '@angular/core';
503+
import * as i0 from "@angular/core";
504+
export class ContentQueryComponent {
505+
}
506+
ContentQueryComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ContentQueryComponent, deps: [], target: i0.ɵɵFactoryTarget.Component });
507+
ContentQueryComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: ContentQueryComponent, selector: "content-query-component", queries: [{ propertyName: "someDir", first: true, predicate: i0.forwardRef(function () { return SomeDirective; }), isForwardRef: true, descendants: true }, { propertyName: "someDirList", predicate: i0.forwardRef(function () { return SomeDirective; }), isForwardRef: true }], ngImport: i0, template: `
508+
<div><ng-content></ng-content></div>
509+
`, isInline: true });
510+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ContentQueryComponent, decorators: [{
511+
type: Component,
512+
args: [{
513+
selector: 'content-query-component',
514+
template: `
515+
<div><ng-content></ng-content></div>
516+
`
517+
}]
518+
}], propDecorators: { someDir: [{
519+
type: ContentChild,
520+
args: [forwardRef(() => SomeDirective)]
521+
}], someDirList: [{
522+
type: ContentChildren,
523+
args: [forwardRef(() => SomeDirective)]
524+
}] } });
525+
export class MyApp {
526+
}
527+
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
528+
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "my-app", ngImport: i0, template: `
529+
<content-query-component>
530+
<div someDir></div>
531+
</content-query-component>
532+
`, isInline: true, components: [{ type: i0.forwardRef(function () { return ContentQueryComponent; }), selector: "content-query-component" }], directives: [{ type: i0.forwardRef(function () { return SomeDirective; }), selector: "[someDir]" }] });
533+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
534+
type: Component,
535+
args: [{
536+
selector: 'my-app',
537+
template: `
538+
<content-query-component>
539+
<div someDir></div>
540+
</content-query-component>
541+
`
542+
}]
543+
}] });
544+
export class SomeDirective {
545+
}
546+
SomeDirective.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, deps: [], target: i0.ɵɵFactoryTarget.Directive });
547+
SomeDirective.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: SomeDirective, selector: "[someDir]", ngImport: i0 });
548+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, decorators: [{
549+
type: Directive,
550+
args: [{
551+
selector: '[someDir]',
552+
}]
553+
}] });
554+
export class MyModule {
555+
}
556+
MyModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule });
557+
MyModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, declarations: [SomeDirective, ContentQueryComponent, MyApp] });
558+
MyModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule });
559+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, decorators: [{
560+
type: NgModule,
561+
args: [{ declarations: [SomeDirective, ContentQueryComponent, MyApp] }]
562+
}] });
563+
564+
/****************************************************************************************************
565+
* PARTIAL FILE: content_query_forward_ref.d.ts
566+
****************************************************************************************************/
567+
import { QueryList } from '@angular/core';
568+
import * as i0 from "@angular/core";
569+
export declare class ContentQueryComponent {
570+
someDir: SomeDirective;
571+
someDirList: QueryList<SomeDirective>;
572+
static ɵfac: i0.ɵɵFactoryDeclaration<ContentQueryComponent, never>;
573+
static ɵcmp: i0.ɵɵComponentDeclaration<ContentQueryComponent, "content-query-component", never, {}, {}, ["someDir", "someDirList"], ["*"]>;
574+
}
575+
export declare class MyApp {
576+
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
577+
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "my-app", never, {}, {}, never, never>;
578+
}
579+
export declare class SomeDirective {
580+
static ɵfac: i0.ɵɵFactoryDeclaration<SomeDirective, never>;
581+
static ɵdir: i0.ɵɵDirectiveDeclaration<SomeDirective, "[someDir]", never, {}, {}, never>;
582+
}
583+
export declare class MyModule {
584+
static ɵfac: i0.ɵɵFactoryDeclaration<MyModule, never>;
585+
static ɵmod: i0.ɵɵNgModuleDeclaration<MyModule, [typeof SomeDirective, typeof ContentQueryComponent, typeof MyApp], never, never>;
586+
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
587+
}
588+
413589
/****************************************************************************************************
414590
* PARTIAL FILE: content_query_for_local_ref.js
415591
****************************************************************************************************/

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/TEST_CASES.json

+28
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,20 @@
1616
}
1717
]
1818
},
19+
{
20+
"description": "should support view queries with forwardRefs",
21+
"inputFiles": [
22+
"view_query_forward_ref.ts"
23+
],
24+
"expectations": [
25+
{
26+
"failureMessage": "Invalid viewQuery declaration",
27+
"files": [
28+
"view_query_forward_ref.js"
29+
]
30+
}
31+
]
32+
},
1933
{
2034
"description": "should support view queries with local refs",
2135
"inputFiles": [
@@ -75,6 +89,20 @@
7589
}
7690
]
7791
},
92+
{
93+
"description": "should support content queries with forwardRefs",
94+
"inputFiles": [
95+
"content_query_forward_ref.ts"
96+
],
97+
"expectations": [
98+
{
99+
"failureMessage": "Invalid contentQuery declaration",
100+
"files": [
101+
"content_query_forward_ref.js"
102+
]
103+
}
104+
]
105+
},
78106
{
79107
"description": "should support content queries with local refs",
80108
"inputFiles": [

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.js

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
ContentQueryComponent.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({
2+
type: ContentQueryComponent,
3+
selectors: [["content-query-component"]],
4+
contentQueries: function ContentQueryComponent_ContentQueries(rf, ctx, dirIndex) {
5+
if (rf & 1) {
6+
$r3$.ɵɵcontentQuery(dirIndex, SomeDirective, 5);
7+
$r3$.ɵɵcontentQuery(dirIndex, SomeDirective, 4);
8+
}
9+
if (rf & 2) {
10+
let $tmp$;
11+
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first);
12+
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirList = $tmp$);
13+
}
14+
},
15+
ngContentSelectors: _c0,
16+
decls: 2,
17+
vars: 0,
18+
template: function ContentQueryComponent_Template(rf, ctx) {
19+
if (rf & 1) {
20+
$r3$.ɵɵprojectionDef();
21+
$r3$.ɵɵelementStart(0, "div");
22+
$r3$.ɵɵprojection(1);
23+
$r3$.ɵɵelementEnd();
24+
}
25+
},
26+
encapsulation: 2
27+
});

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.ts

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import {Component, ContentChild, ContentChildren, Directive, forwardRef, NgModule, QueryList} from '@angular/core';
2+
3+
@Component({
4+
selector: 'content-query-component',
5+
template: `
6+
<div><ng-content></ng-content></div>
7+
`
8+
})
9+
export class ContentQueryComponent {
10+
@ContentChild(forwardRef(() => SomeDirective)) someDir!: SomeDirective;
11+
@ContentChildren(forwardRef(() => SomeDirective)) someDirList!: QueryList<SomeDirective>;
12+
}
13+
14+
@Component({
15+
selector: 'my-app',
16+
template: `
17+
<content-query-component>
18+
<div someDir></div>
19+
</content-query-component>
20+
`
21+
})
22+
export class MyApp {
23+
}
24+
25+
26+
@Directive({
27+
selector: '[someDir]',
28+
})
29+
export class SomeDirective {
30+
}
31+
32+
@NgModule({declarations: [SomeDirective, ContentQueryComponent, MyApp]})
33+
export class MyModule {
34+
}

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.js

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
ViewQueryComponent.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({
2+
type: ViewQueryComponent,
3+
selectors: [["view-query-component"]],
4+
viewQuery: function ViewQueryComponent_Query(rf, ctx) {
5+
if (rf & 1) {
6+
$r3$.ɵɵviewQuery(SomeDirective, 5);
7+
$r3$.ɵɵviewQuery(SomeDirective, 5);
8+
}
9+
if (rf & 2) {
10+
let $tmp$;
11+
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first);
12+
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirList = $tmp$);
13+
}
14+
},
15+
// ...
16+
});

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.ts

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import {Component, Directive, forwardRef, NgModule, QueryList, ViewChild, ViewChildren} from '@angular/core';
2+
3+
@Component({
4+
selector: 'view-query-component',
5+
template: `
6+
<div someDir></div>
7+
`
8+
})
9+
export class ViewQueryComponent {
10+
@ViewChild(forwardRef(() => SomeDirective)) someDir!: SomeDirective;
11+
@ViewChildren(forwardRef(() => SomeDirective)) someDirList!: QueryList<SomeDirective>;
12+
}
13+
14+
@Component({
15+
selector: 'my-app',
16+
template: `
17+
<view-query-component></view-query-component>
18+
`
19+
})
20+
export class MyApp {
21+
}
22+
23+
24+
@Directive({
25+
selector: '[someDir]',
26+
})
27+
export class SomeDirective {
28+
}
29+
30+
@NgModule({declarations: [SomeDirective, ViewQueryComponent, MyApp]})
31+
export class MyModule {
32+
}

‎packages/compiler/src/compiler_facade_interface.ts

+1
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ export interface R3DeclareQueryMetadataFacade {
278278
propertyName: string;
279279
first?: boolean;
280280
predicate: OpaqueValue|string[];
281+
isForwardRef?: boolean;
281282
descendants?: boolean;
282283
read?: OpaqueValue;
283284
static?: boolean;

0 commit comments

Comments
 (0)