Skip to content

Commit

Permalink
fix(compiler): ensure that partially compiled queries can handle forw…
Browse files Browse the repository at this point in the history
…ard 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
  • Loading branch information
petebacondarwin committed Nov 9, 2021
1 parent 0c0698a commit d8751b1
Show file tree
Hide file tree
Showing 14 changed files with 366 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {AstObject, AstValue} from '../../ast/ast_value';
import {FatalLinkerError} from '../../fatal_linker_error';

import {PartialLinker} from './partial_linker';
import {wrapReference} from './util';
import {extractForwardRef, wrapReference} from './util';

/**
* A `PartialLinker` that is designed to process `ɵɵngDeclareDirective()` call expressions.
Expand Down Expand Up @@ -141,7 +141,7 @@ function toQueryMetadata<TExpression>(obj: AstObject<R3DeclareQueryMetadata, TEx
if (predicateExpr.isArray()) {
predicate = predicateExpr.getArray().map(entry => entry.getString());
} else {
predicate = predicateExpr.getOpaque();
predicate = extractForwardRef(predicateExpr);
}
return {
propertyName: obj.getString('propertyName'),
Expand Down
11 changes: 7 additions & 4 deletions packages/compiler-cli/src/ngtsc/annotations/src/directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {compileClassMetadata, compileDeclareClassMetadata, compileDeclareDirectiveFromMetadata, compileDirectiveFromMetadata, ConstantPool, Expression, ExternalExpr, FactoryTarget, getSafePropertyAccessString, makeBindingParser, ParsedHostBindings, ParseError, parseHostBindings, R3ClassMetadata, R3DirectiveMetadata, R3FactoryMetadata, R3QueryMetadata, Statement, verifyHostBindings, WrappedNodeExpr} from '@angular/compiler';
import {compileClassMetadata, compileDeclareClassMetadata, compileDeclareDirectiveFromMetadata, compileDirectiveFromMetadata, ConstantPool, createMayBeForwardRefExpression, Expression, ExternalExpr, FactoryTarget, getSafePropertyAccessString, makeBindingParser, MaybeForwardRefExpression, ParsedHostBindings, ParseError, parseHostBindings, R3ClassMetadata, R3DirectiveMetadata, R3QueryMetadata, verifyHostBindings, WrappedNodeExpr} from '@angular/compiler';
import {emitDistinctChangesOnlyDefaultValue} from '@angular/compiler/src/core';
import * as ts from 'typescript';

Expand Down Expand Up @@ -532,17 +532,20 @@ export function extractQueryMetadata(
ErrorCode.DECORATOR_ARITY_WRONG, exprNode, `@${name} must have arguments`);
}
const first = name === 'ViewChild' || name === 'ContentChild';
const node = tryUnwrapForwardRef(args[0], reflector) ?? args[0];
const forwardReferenceTarget = tryUnwrapForwardRef(args[0], reflector);
const node = forwardReferenceTarget ?? args[0];

const arg = evaluator.evaluate(node);

/** Whether or not this query should collect only static results (see view/api.ts) */
let isStatic: boolean = false;

// Extract the predicate
let predicate: Expression|string[]|null = null;
let predicate: MaybeForwardRefExpression|string[]|null = null;
if (arg instanceof Reference || arg instanceof DynamicValue) {
// References and predicates that could not be evaluated statically are emitted as is.
predicate = new WrappedNodeExpr(node);
predicate =
createMayBeForwardRefExpression(new WrappedNodeExpr(node), forwardReferenceTarget !== null);
} else if (typeof arg === 'string') {
predicate = [arg];
} else if (isStringArrayOrDie(arg, `@${name} predicate`, node)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,92 @@ export declare class MyModule {
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
}

/****************************************************************************************************
* PARTIAL FILE: view_query_forward_ref.js
****************************************************************************************************/
import { Component, Directive, forwardRef, NgModule, ViewChild, ViewChildren } from '@angular/core';
import * as i0 from "@angular/core";
export class ViewQueryComponent {
}
ViewQueryComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ViewQueryComponent, deps: [], target: i0.ɵɵFactoryTarget.Component });
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; }), descendants: true }, { propertyName: "someDirList", predicate: i0.forwardRef(function () { return SomeDirective; }), descendants: true }], ngImport: i0, template: `
<div someDir></div>
`, isInline: true, directives: [{ type: i0.forwardRef(function () { return SomeDirective; }), selector: "[someDir]" }] });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ViewQueryComponent, decorators: [{
type: Component,
args: [{
selector: 'view-query-component',
template: `
<div someDir></div>
`
}]
}], propDecorators: { someDir: [{
type: ViewChild,
args: [forwardRef(() => SomeDirective)]
}], someDirList: [{
type: ViewChildren,
args: [forwardRef(() => SomeDirective)]
}] } });
export class MyApp {
}
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "my-app", ngImport: i0, template: `
<view-query-component></view-query-component>
`, isInline: true, components: [{ type: ViewQueryComponent, selector: "view-query-component" }] });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
type: Component,
args: [{
selector: 'my-app',
template: `
<view-query-component></view-query-component>
`
}]
}] });
export class SomeDirective {
}
SomeDirective.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, deps: [], target: i0.ɵɵFactoryTarget.Directive });
SomeDirective.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: SomeDirective, selector: "[someDir]", ngImport: i0 });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, decorators: [{
type: Directive,
args: [{
selector: '[someDir]',
}]
}] });
export class MyModule {
}
MyModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule });
MyModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, declarations: [SomeDirective, ViewQueryComponent, MyApp] });
MyModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, decorators: [{
type: NgModule,
args: [{ declarations: [SomeDirective, ViewQueryComponent, MyApp] }]
}] });

/****************************************************************************************************
* PARTIAL FILE: view_query_forward_ref.d.ts
****************************************************************************************************/
import { QueryList } from '@angular/core';
import * as i0 from "@angular/core";
export declare class ViewQueryComponent {
someDir: SomeDirective;
someDirList: QueryList<SomeDirective>;
static ɵfac: i0.ɵɵFactoryDeclaration<ViewQueryComponent, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<ViewQueryComponent, "view-query-component", never, {}, {}, never, never>;
}
export declare class MyApp {
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "my-app", never, {}, {}, never, never>;
}
export declare class SomeDirective {
static ɵfac: i0.ɵɵFactoryDeclaration<SomeDirective, never>;
static ɵdir: i0.ɵɵDirectiveDeclaration<SomeDirective, "[someDir]", never, {}, {}, never>;
}
export declare class MyModule {
static ɵfac: i0.ɵɵFactoryDeclaration<MyModule, never>;
static ɵmod: i0.ɵɵNgModuleDeclaration<MyModule, [typeof SomeDirective, typeof ViewQueryComponent, typeof MyApp], never, never>;
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
}

/****************************************************************************************************
* PARTIAL FILE: view_query_for_local_ref.js
****************************************************************************************************/
Expand Down Expand Up @@ -410,6 +496,96 @@ export declare class MyModule {
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
}

/****************************************************************************************************
* PARTIAL FILE: content_query_forward_ref.js
****************************************************************************************************/
import { Component, ContentChild, ContentChildren, Directive, forwardRef, NgModule } from '@angular/core';
import * as i0 from "@angular/core";
export class ContentQueryComponent {
}
ContentQueryComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ContentQueryComponent, deps: [], target: i0.ɵɵFactoryTarget.Component });
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; }), descendants: true }, { propertyName: "someDirList", predicate: i0.forwardRef(function () { return SomeDirective; }) }], ngImport: i0, template: `
<div><ng-content></ng-content></div>
`, isInline: true });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ContentQueryComponent, decorators: [{
type: Component,
args: [{
selector: 'content-query-component',
template: `
<div><ng-content></ng-content></div>
`
}]
}], propDecorators: { someDir: [{
type: ContentChild,
args: [forwardRef(() => SomeDirective)]
}], someDirList: [{
type: ContentChildren,
args: [forwardRef(() => SomeDirective)]
}] } });
export class MyApp {
}
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "my-app", ngImport: i0, template: `
<content-query-component>
<div someDir></div>
</content-query-component>
`, isInline: true, components: [{ type: i0.forwardRef(function () { return ContentQueryComponent; }), selector: "content-query-component" }], directives: [{ type: i0.forwardRef(function () { return SomeDirective; }), selector: "[someDir]" }] });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
type: Component,
args: [{
selector: 'my-app',
template: `
<content-query-component>
<div someDir></div>
</content-query-component>
`
}]
}] });
export class SomeDirective {
}
SomeDirective.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, deps: [], target: i0.ɵɵFactoryTarget.Directive });
SomeDirective.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: SomeDirective, selector: "[someDir]", ngImport: i0 });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, decorators: [{
type: Directive,
args: [{
selector: '[someDir]',
}]
}] });
export class MyModule {
}
MyModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule });
MyModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, declarations: [SomeDirective, ContentQueryComponent, MyApp] });
MyModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, decorators: [{
type: NgModule,
args: [{ declarations: [SomeDirective, ContentQueryComponent, MyApp] }]
}] });

/****************************************************************************************************
* PARTIAL FILE: content_query_forward_ref.d.ts
****************************************************************************************************/
import { QueryList } from '@angular/core';
import * as i0 from "@angular/core";
export declare class ContentQueryComponent {
someDir: SomeDirective;
someDirList: QueryList<SomeDirective>;
static ɵfac: i0.ɵɵFactoryDeclaration<ContentQueryComponent, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<ContentQueryComponent, "content-query-component", never, {}, {}, ["someDir", "someDirList"], ["*"]>;
}
export declare class MyApp {
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "my-app", never, {}, {}, never, never>;
}
export declare class SomeDirective {
static ɵfac: i0.ɵɵFactoryDeclaration<SomeDirective, never>;
static ɵdir: i0.ɵɵDirectiveDeclaration<SomeDirective, "[someDir]", never, {}, {}, never>;
}
export declare class MyModule {
static ɵfac: i0.ɵɵFactoryDeclaration<MyModule, never>;
static ɵmod: i0.ɵɵNgModuleDeclaration<MyModule, [typeof SomeDirective, typeof ContentQueryComponent, typeof MyApp], never, never>;
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
}

/****************************************************************************************************
* PARTIAL FILE: content_query_for_local_ref.js
****************************************************************************************************/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,20 @@
}
]
},
{
"description": "should support view queries with forwardRefs",
"inputFiles": [
"view_query_forward_ref.ts"
],
"expectations": [
{
"failureMessage": "Invalid ViewQuery declaration",
"files": [
"view_query_forward_ref.js"
]
}
]
},
{
"description": "should support view queries with local refs",
"inputFiles": [
Expand Down Expand Up @@ -75,6 +89,20 @@
}
]
},
{
"description": "should support content queries with forwardRefs",
"inputFiles": [
"content_query_forward_ref.ts"
],
"expectations": [
{
"failureMessage": "Invalid ContentQuery declaration",
"files": [
"content_query_forward_ref.js"
]
}
]
},
{
"description": "should support content queries with local refs",
"inputFiles": [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
ContentQueryComponent.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({
type: ContentQueryComponent,
selectors: [["content-query-component"]],
contentQueries: function ContentQueryComponent_ContentQueries(rf, ctx, dirIndex) {
if (rf & 1) {
$r3$.ɵɵcontentQuery(dirIndex, SomeDirective, __QueryFlags.descendants__|__QueryFlags.emitDistinctChangesOnly__);
$r3$.ɵɵcontentQuery(dirIndex, SomeDirective, __QueryFlags.emitDistinctChangesOnly__);
}
if (rf & 2) {
let $tmp$;
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirList = $tmp$);
}
},
ngContentSelectors: _c0,
decls: 2,
vars: 0,
template: function ContentQueryComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵprojectionDef();
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵprojection(1);
$r3$.ɵɵelementEnd();
}
},
encapsulation: 2
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import {Component, ContentChild, ContentChildren, Directive, forwardRef, NgModule, QueryList} from '@angular/core';

@Component({
selector: 'content-query-component',
template: `
<div><ng-content></ng-content></div>
`
})
export class ContentQueryComponent {
@ContentChild(forwardRef(() => SomeDirective)) someDir!: SomeDirective;
@ContentChildren(forwardRef(() => SomeDirective)) someDirList!: QueryList<SomeDirective>;
}

@Component({
selector: 'my-app',
template: `
<content-query-component>
<div someDir></div>
</content-query-component>
`
})
export class MyApp {
}


@Directive({
selector: '[someDir]',
})
export class SomeDirective {
}

@NgModule({declarations: [SomeDirective, ContentQueryComponent, MyApp]})
export class MyModule {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
ViewQueryComponent.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({
type: ViewQueryComponent,
selectors: [["view-query-component"]],
viewQuery: function ViewQueryComponent_Query(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵviewQuery(SomeDirective, __QueryFlags.descendants__|__QueryFlags.emitDistinctChangesOnly__);
$r3$.ɵɵviewQuery(SomeDirective, __QueryFlags.descendants__|__QueryFlags.emitDistinctChangesOnly__);
}
if (rf & 2) {
let $tmp$;
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirList = $tmp$);
}
},
// ...
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import {Component, Directive, forwardRef, NgModule, QueryList, ViewChild, ViewChildren} from '@angular/core';

@Component({
selector: 'view-query-component',
template: `
<div someDir></div>
`
})
export class ViewQueryComponent {
@ViewChild(forwardRef(() => SomeDirective)) someDir!: SomeDirective;
@ViewChildren(forwardRef(() => SomeDirective)) someDirList!: QueryList<SomeDirective>;
}

@Component({
selector: 'my-app',
template: `
<view-query-component></view-query-component>
`
})
export class MyApp {
}


@Directive({
selector: '[someDir]',
})
export class SomeDirective {
}

@NgModule({declarations: [SomeDirective, ViewQueryComponent, MyApp]})
export class MyModule {
}
Loading

0 comments on commit d8751b1

Please sign in to comment.