Skip to content

Commit

Permalink
feat(ivy): resolve forwardRef() for queries (angular#25080)
Browse files Browse the repository at this point in the history
@ContentChild[ren] and @ViewChild[ren] can contain a forwardRef() to a
type. This commit allows ngtsc to unwrap the forward reference and
deal with the node inside.

It includes two modes of support for forward reference resolution -
a foreign function resolver which understands deeply nested forward
references in expressions that are being statically evaluated, and
an unwrapForwardRef() function which deals only with top-level nodes.

Both will be useful in the future, but for now only unwrapForwardRef()
is used.

PR Close angular#25080
  • Loading branch information
alxhub authored and FrederikSchlemmer committed Jan 3, 2019
1 parent 129663e commit 92407a7
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 23 deletions.
7 changes: 4 additions & 3 deletions packages/compiler-cli/src/ngtsc/annotations/src/directive.ts
Expand Up @@ -14,7 +14,7 @@ import {Reference, filterToMembersWithDecorator, reflectObjectLiteral, staticall
import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform';

import {SelectorScopeRegistry} from './selector_scope';
import {getConstructorDependencies, isAngularCore, unwrapExpression} from './util';
import {getConstructorDependencies, isAngularCore, unwrapExpression, unwrapForwardRef} from './util';

const EMPTY_OBJECT: {[key: string]: string} = {};

Expand Down Expand Up @@ -156,12 +156,13 @@ export function extractQueryMetadata(
throw new Error(`@${name} must have arguments`);
}
const first = name === 'ViewChild' || name === 'ContentChild';
const arg = staticallyResolve(args[0], reflector, checker);
const node = unwrapForwardRef(args[0], reflector);
const arg = staticallyResolve(node, reflector, checker);

// Extract the predicate
let predicate: Expression|string[]|null = null;
if (arg instanceof Reference) {
predicate = new WrappedNodeExpr(args[0]);
predicate = new WrappedNodeExpr(node);
} else if (typeof arg === 'string') {
predicate = [arg];
} else if (isStringArrayOrDie(arg, '@' + name)) {
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts
Expand Up @@ -67,14 +67,14 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
if (ngModule.has('imports')) {
const importsMeta = staticallyResolve(
ngModule.get('imports') !, this.reflector, this.checker,
node => this._extractModuleFromModuleWithProvidersFn(node));
ref => this._extractModuleFromModuleWithProvidersFn(ref.node));
imports = resolveTypeList(importsMeta, 'imports');
}
let exports: Reference[] = [];
if (ngModule.has('exports')) {
const exportsMeta = staticallyResolve(
ngModule.get('exports') !, this.reflector, this.checker,
node => this._extractModuleFromModuleWithProvidersFn(node));
ref => this._extractModuleFromModuleWithProvidersFn(ref.node));
exports = resolveTypeList(exportsMeta, 'exports');
}

Expand Down
68 changes: 67 additions & 1 deletion packages/compiler-cli/src/ngtsc/annotations/src/util.ts
Expand Up @@ -10,7 +10,7 @@ import {Expression, R3DependencyMetadata, R3ResolvedDependencyType, WrappedNodeE
import * as ts from 'typescript';

import {Decorator, ReflectionHost} from '../../host';
import {Reference} from '../../metadata';
import {AbsoluteReference, Reference} from '../../metadata';

export function getConstructorDependencies(
clazz: ts.ClassDeclaration, reflector: ReflectionHost,
Expand Down Expand Up @@ -103,3 +103,69 @@ export function unwrapExpression(node: ts.Expression): ts.Expression {
}
return node;
}

function expandForwardRef(arg: ts.Expression): ts.Expression|null {
if (!ts.isArrowFunction(arg) && !ts.isFunctionExpression(arg)) {
return null;
}

const body = arg.body;
// Either the body is a ts.Expression directly, or a block with a single return statement.
if (ts.isBlock(body)) {
// Block body - look for a single return statement.
if (body.statements.length !== 1) {
return null;
}
const stmt = body.statements[0];
if (!ts.isReturnStatement(stmt) || stmt.expression === undefined) {
return null;
}
return stmt.expression;
} else {
// Shorthand body - return as an expression.
return body;
}
}

/**
* Possibly resolve a forwardRef() expression into the inner value.
*
* @param node the forwardRef() expression to resolve
* @param reflector a ReflectionHost
* @returns the resolved expression, if the original expression was a forwardRef(), or the original
* expression otherwise
*/
export function unwrapForwardRef(node: ts.Expression, reflector: ReflectionHost): ts.Expression {
if (!ts.isCallExpression(node) || !ts.isIdentifier(node.expression) ||
node.arguments.length !== 1) {
return node;
}
const expr = expandForwardRef(node.arguments[0]);
if (expr === null) {
return node;
}
const imp = reflector.getImportOfIdentifier(node.expression);
if (imp === null || imp.from !== '@angular/core' || imp.name !== 'forwardRef') {
return node;
} else {
return expr;
}
}

/**
* A foreign function resolver for `staticallyResolve` which unwraps forwardRef() expressions.
*
* @param ref a Reference to the declaration of the function being called (which might be
* forwardRef)
* @param args the arguments to the invocation of the forwardRef expression
* @returns an unwrapped argument if `ref` pointed to forwardRef, or null otherwise
*/
export function forwardRefResolver(
ref: Reference<ts.FunctionDeclaration|ts.MethodDeclaration>,
args: ts.Expression[]): ts.Expression|null {
if (!(ref instanceof AbsoluteReference) || ref.moduleName !== '@angular/core' ||
ref.symbolName !== 'forwardRef' || args.length !== 1) {
return null;
}
return expandForwardRef(args[0]);
}
36 changes: 20 additions & 16 deletions packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts
Expand Up @@ -84,8 +84,8 @@ type Scope = Map<ts.ParameterDeclaration, ResolvedValue>;
* For example, if an expression evaluates to a function or class definition, it will be returned
* as a `Reference` (assuming references are allowed in evaluation).
*/
export abstract class Reference {
constructor(readonly node: ts.Node) {}
export abstract class Reference<T extends ts.Node = ts.Node> {
constructor(readonly node: T) {}

/**
* Whether an `Expression` can be generated which references the node.
Expand All @@ -110,8 +110,8 @@ export abstract class Reference {
* This is used for returning references to things like method declarations, which are not directly
* referenceable.
*/
export class NodeReference extends Reference {
constructor(node: ts.Node, readonly moduleName: string|null) { super(node); }
export class NodeReference<T extends ts.Node = ts.Node> extends Reference<T> {
constructor(node: T, readonly moduleName: string|null) { super(node); }

toExpression(context: ts.SourceFile): null { return null; }

Expand All @@ -123,8 +123,8 @@ export class NodeReference extends Reference {
*
* Imports generated by `ResolvedReference`s are always relative.
*/
export class ResolvedReference extends Reference {
constructor(node: ts.Node, protected identifier: ts.Identifier) { super(node); }
export class ResolvedReference<T extends ts.Node = ts.Node> extends Reference<T> {
constructor(node: T, protected identifier: ts.Identifier) { super(node); }

readonly expressable = true;

Expand Down Expand Up @@ -169,7 +169,7 @@ export class ResolvedReference extends Reference {
export class AbsoluteReference extends Reference {
constructor(
node: ts.Node, private identifier: ts.Identifier, readonly moduleName: string,
private symbolName: string) {
readonly symbolName: string) {
super(node);
}

Expand Down Expand Up @@ -203,8 +203,9 @@ export class AbsoluteReference extends Reference {
*/
export function staticallyResolve(
node: ts.Expression, host: ReflectionHost, checker: ts.TypeChecker,
foreignFunctionResolver?: (node: ts.FunctionDeclaration | ts.MethodDeclaration) =>
ts.Expression | null): ResolvedValue {
foreignFunctionResolver?:
(node: Reference<ts.FunctionDeclaration|ts.MethodDeclaration>, args: ts.Expression[]) =>
ts.Expression | null): ResolvedValue {
return new StaticInterpreter(host, checker).visit(node, {
absoluteModuleName: null,
scope: new Map<ts.ParameterDeclaration, ResolvedValue>(), foreignFunctionResolver,
Expand Down Expand Up @@ -253,7 +254,9 @@ const UNARY_OPERATORS = new Map<ts.SyntaxKind, (a: any) => any>([
interface Context {
absoluteModuleName: string|null;
scope: Scope;
foreignFunctionResolver?(node: ts.FunctionDeclaration|ts.MethodDeclaration): ts.Expression|null;
foreignFunctionResolver?
(ref: Reference<ts.FunctionDeclaration|ts.MethodDeclaration>,
args: ReadonlyArray<ts.Expression>): ts.Expression|null;
}

class StaticInterpreter {
Expand Down Expand Up @@ -516,7 +519,7 @@ class StaticInterpreter {
const lhs = this.visitExpression(node.expression, context);
if (!(lhs instanceof Reference)) {
throw new Error(`attempting to call something that is not a function: ${lhs}`);
} else if (!isFunctionOrMethodDeclaration(lhs.node)) {
} else if (!isFunctionOrMethodReference(lhs)) {
throw new Error(
`calling something that is not a function declaration? ${ts.SyntaxKind[lhs.node.kind]}`);
}
Expand All @@ -528,10 +531,11 @@ class StaticInterpreter {
if (fn.body === undefined) {
let expr: ts.Expression|null = null;
if (context.foreignFunctionResolver) {
expr = context.foreignFunctionResolver(fn);
expr = context.foreignFunctionResolver(lhs, node.arguments);
}
if (expr === null) {
throw new Error(`could not resolve foreign function declaration`);
throw new Error(
`could not resolve foreign function declaration: ${node.getSourceFile().fileName} ${(lhs.node.name as ts.Identifier).text}`);
}

// If the function is declared in a different file, resolve the foreign function expression
Expand Down Expand Up @@ -642,9 +646,9 @@ class StaticInterpreter {
}
}

function isFunctionOrMethodDeclaration(node: ts.Node): node is ts.FunctionDeclaration|
ts.MethodDeclaration {
return ts.isFunctionDeclaration(node) || ts.isMethodDeclaration(node);
function isFunctionOrMethodReference(ref: Reference<ts.Node>):
ref is Reference<ts.FunctionDeclaration|ts.MethodDeclaration> {
return ts.isFunctionDeclaration(ref.node) || ts.isMethodDeclaration(ref.node);
}

function literal(value: ResolvedValue): any {
Expand Down
6 changes: 5 additions & 1 deletion packages/compiler-cli/test/ngtsc/fake_core/index.ts
Expand Up @@ -48,4 +48,8 @@ export class ChangeDetectorRef {}
export class ElementRef {}
export class Injector {}
export class TemplateRef {}
export class ViewContainerRef {}
export class ViewContainerRef {}

export function forwardRef<T>(fn: () => T): T {
return fn();
}
24 changes: 24 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Expand Up @@ -449,6 +449,30 @@ describe('ngtsc behavioral tests', () => {
expect(jsContents).toContain(`i0.ɵQ(1, ["test1"], true)`);
});

it('should handle queries that use forwardRef', () => {
writeConfig();
write(`test.ts`, `
import {Component, ContentChild, TemplateRef, ViewContainerRef, forwardRef} from '@angular/core';
@Component({
selector: 'test',
template: '<div #foo></div>',
})
class FooCmp {
@ContentChild(forwardRef(() => TemplateRef)) child: any;
@ContentChild(forwardRef(function() { return ViewContainerRef; })) child2: any;
}
`);

const exitCode = main(['-p', basePath], errorSpy);
expect(errorSpy).not.toHaveBeenCalled();
expect(exitCode).toBe(0);
const jsContents = getContents('test.js');
expect(jsContents).toContain(`i0.ɵQ(null, TemplateRef, true)`);
expect(jsContents).toContain(`i0.ɵQ(null, ViewContainerRef, true)`);
});

it('should generate host bindings for directives', () => {
writeConfig();
write(`test.ts`, `
Expand Down

0 comments on commit 92407a7

Please sign in to comment.