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
petebacondarwin committed Nov 10, 2021
1 parent 17bfc2f commit c0ad276
Show file tree
Hide file tree
Showing 21 changed files with 462 additions and 68 deletions.
10 changes: 5 additions & 5 deletions packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_component_linker_1.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {ChangeDetectionStrategy, compileComponentFromMetadata, ConstantPool, DeclarationListEmitMode, DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig, makeBindingParser, outputAst as o, parseTemplate, R3ComponentMetadata, R3DeclareComponentMetadata, R3DeclareUsedDirectiveMetadata, R3PartialDeclaration, R3UsedDirectiveMetadata, ViewEncapsulation} from '@angular/compiler';
import {ChangeDetectionStrategy, compileComponentFromMetadata, ConstantPool, DeclarationListEmitMode, DEFAULT_INTERPOLATION_CONFIG, ForwardRefHandling, InterpolationConfig, makeBindingParser, outputAst as o, parseTemplate, R3ComponentMetadata, R3DeclareComponentMetadata, R3DeclareUsedDirectiveMetadata, R3PartialDeclaration, R3UsedDirectiveMetadata, ViewEncapsulation} from '@angular/compiler';

import {AbsoluteFsPath} from '../../../../src/ngtsc/file_system';
import {Range} from '../../ast/ast_host';
Expand Down Expand Up @@ -69,8 +69,8 @@ export class PartialComponentLinkerVersion1<TStatement, TExpression> implements
const type = directiveExpr.getValue('type');
const selector = directiveExpr.getString('selector');

const {expression: typeExpr, isForwardRef} = extractForwardRef(type);
if (isForwardRef) {
const {expression: typeExpr, forwardRef} = extractForwardRef(type);
if (forwardRef === ForwardRefHandling.Unwrapped) {
declarationListEmitMode = DeclarationListEmitMode.Closure;
}

Expand Down Expand Up @@ -101,8 +101,8 @@ export class PartialComponentLinkerVersion1<TStatement, TExpression> implements
let pipes = new Map<string, o.Expression>();
if (metaObj.has('pipes')) {
pipes = metaObj.getObject('pipes').toMap(pipe => {
const {expression: pipeType, isForwardRef} = extractForwardRef(pipe);
if (isForwardRef) {
const {expression: pipeType, forwardRef} = extractForwardRef(pipe);
if (forwardRef === ForwardRefHandling.Unwrapped) {
declarationListEmitMode = DeclarationListEmitMode.Closure;
}
return pipeType;
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_directive_linker_1.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,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 @@ -140,7 +140,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
7 changes: 4 additions & 3 deletions packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_injectable_linker_1.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {compileInjectable, ConstantPool, createMayBeForwardRefExpression, outputAst as o, R3DeclareInjectableMetadata, R3InjectableMetadata, R3PartialDeclaration} from '@angular/compiler';
import {compileInjectable, ConstantPool, createMayBeForwardRefExpression, ForwardRefHandling, outputAst as o, R3DeclareInjectableMetadata, R3InjectableMetadata, R3PartialDeclaration} from '@angular/compiler';

import {AstObject} from '../../ast/ast_value';
import {FatalLinkerError} from '../../fatal_linker_error';
Expand Down Expand Up @@ -43,8 +43,9 @@ export function toR3InjectableMeta<TExpression>(
type: wrapReference(typeExpr.getOpaque()),
internalType: typeExpr.getOpaque(),
typeArgumentCount: 0,
providedIn: metaObj.has('providedIn') ? extractForwardRef(metaObj.getValue('providedIn')) :
createMayBeForwardRefExpression(o.literal(null), false),
providedIn: metaObj.has('providedIn') ?
extractForwardRef(metaObj.getValue('providedIn')) :
createMayBeForwardRefExpression(o.literal(null), ForwardRefHandling.None),
};

if (metaObj.has('useClass')) {
Expand Down
7 changes: 4 additions & 3 deletions packages/compiler-cli/linker/src/file_linker/partial_linkers/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {createMayBeForwardRefExpression, MaybeForwardRefExpression, outputAst as o, R3DeclareDependencyMetadata, R3DependencyMetadata, R3Reference} from '@angular/compiler';
import {createMayBeForwardRefExpression, ForwardRefHandling, MaybeForwardRefExpression, outputAst as o, R3DeclareDependencyMetadata, R3DependencyMetadata, R3Reference} from '@angular/compiler';

import {AstObject, AstValue} from '../../ast/ast_value';
import {FatalLinkerError} from '../../fatal_linker_error';
Expand Down Expand Up @@ -68,7 +68,7 @@ export function getDependency<TExpression>(
export function extractForwardRef<TExpression>(expr: AstValue<unknown, TExpression>):
MaybeForwardRefExpression<o.WrappedNodeExpr<TExpression>> {
if (!expr.isCallExpression()) {
return createMayBeForwardRefExpression(expr.getOpaque(), /* isForwardRef */ false);
return createMayBeForwardRefExpression(expr.getOpaque(), ForwardRefHandling.None);
}

const callee = expr.getCallee();
Expand All @@ -90,5 +90,6 @@ export function extractForwardRef<TExpression>(expr: AstValue<unknown, TExpressi
wrapperFn, 'Unsupported `forwardRef(fn)` call, expected its argument to be a function');
}

return createMayBeForwardRefExpression(wrapperFn.getFunctionReturnValue().getOpaque(), true);
return createMayBeForwardRefExpression(
wrapperFn.getFunctionReturnValue().getOpaque(), ForwardRefHandling.Unwrapped);
}
12 changes: 8 additions & 4 deletions packages/compiler-cli/src/ngtsc/annotations/src/directive.ts
View file
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, emitDistinctChangesOnlyDefaultValue, 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, emitDistinctChangesOnlyDefaultValue, Expression, ExternalExpr, FactoryTarget, ForwardRefHandling, getSafePropertyAccessString, makeBindingParser, MaybeForwardRefExpression, ParsedHostBindings, ParseError, parseHostBindings, R3ClassMetadata, R3DirectiveMetadata, R3FactoryMetadata, R3QueryMetadata, Statement, verifyHostBindings, WrappedNodeExpr} from '@angular/compiler';
import ts from 'typescript';

import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
Expand Down Expand Up @@ -531,17 +531,21 @@ 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 ? ForwardRefHandling.Unwrapped : ForwardRefHandling.None);
} else if (typeof arg === 'string') {
predicate = [arg];
} else if (isStringArrayOrDie(arg, `@${name} predicate`, node)) {
Expand Down
9 changes: 5 additions & 4 deletions packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts
View file
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, CompileClassMetadataFn, compileDeclareClassMetadata, compileDeclareInjectableFromMetadata, compileInjectable, createMayBeForwardRefExpression, FactoryTarget, LiteralExpr, MaybeForwardRefExpression, R3ClassMetadata, R3CompiledExpression, R3DependencyMetadata, R3InjectableMetadata, WrappedNodeExpr} from '@angular/compiler';
import {compileClassMetadata, CompileClassMetadataFn, compileDeclareClassMetadata, compileDeclareInjectableFromMetadata, compileInjectable, createMayBeForwardRefExpression, FactoryTarget, ForwardRefHandling, LiteralExpr, MaybeForwardRefExpression, R3ClassMetadata, R3CompiledExpression, R3DependencyMetadata, R3InjectableMetadata, WrappedNodeExpr} from '@angular/compiler';
import ts from 'typescript';

import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
Expand Down Expand Up @@ -162,7 +162,7 @@ function extractInjectableMetadata(
type,
typeArgumentCount,
internalType,
providedIn: createMayBeForwardRefExpression(new LiteralExpr(null), false),
providedIn: createMayBeForwardRefExpression(new LiteralExpr(null), ForwardRefHandling.None),
};
} else if (decorator.args.length === 1) {
const metaNode = decorator.args[0];
Expand All @@ -180,7 +180,7 @@ function extractInjectableMetadata(

const providedIn = meta.has('providedIn') ?
getProviderExpression(meta.get('providedIn')!, reflector) :
createMayBeForwardRefExpression(new LiteralExpr(null), false);
createMayBeForwardRefExpression(new LiteralExpr(null), ForwardRefHandling.None);

let deps: R3DependencyMetadata[]|undefined = undefined;
if ((meta.has('useClass') || meta.has('useFactory')) && meta.has('deps')) {
Expand Down Expand Up @@ -223,7 +223,8 @@ function getProviderExpression(
expression: ts.Expression, reflector: ReflectionHost): MaybeForwardRefExpression {
const forwardRefValue = tryUnwrapForwardRef(expression, reflector);
return createMayBeForwardRefExpression(
new WrappedNodeExpr(forwardRefValue ?? expression), forwardRefValue !== null);
new WrappedNodeExpr(forwardRefValue ?? expression),
forwardRefValue !== null ? ForwardRefHandling.Unwrapped : ForwardRefHandling.None);
}

function extractInjectableCtorDeps(
Expand Down
Loading

0 comments on commit c0ad276

Please sign in to comment.