From 3cb44e34d4ec3b4de8b7c5b38567ed806722ad77 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 20 Jul 2018 12:52:26 -0700 Subject: [PATCH 1/8] feat(ivy): use the ReflectionHost to resolve parameters and initializers ngtsc's static resolver can evaluate function calls where parameters have default values. In TypeScript code these default values live on the function definition, but in ES5 code the default values are represented by statements in the function body. A new ReflectionHost method getDefinitionOfFunction() abstracts over this difference, and allows the static reflector to more accurately evaluate ES5 code. --- .../src/ngtsc/host/src/reflection.ts | 77 ++++++++++++++++++- .../src/ngtsc/metadata/src/reflector.ts | 17 +++- .../src/ngtsc/metadata/src/resolver.ts | 12 +-- .../src/ngtsc/metadata/test/reflector_spec.ts | 4 +- 4 files changed, 97 insertions(+), 13 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/host/src/reflection.ts b/packages/compiler-cli/src/ngtsc/host/src/reflection.ts index 16b3867fb00c..845ea46790e2 100644 --- a/packages/compiler-cli/src/ngtsc/host/src/reflection.ts +++ b/packages/compiler-cli/src/ngtsc/host/src/reflection.ts @@ -147,9 +147,9 @@ export interface ClassMember { } /** - * A parameter to a function or constructor. + * A parameter to a constructor. */ -export interface Parameter { +export interface CtorParameter { /** * Name of the parameter, if available. * @@ -180,6 +180,54 @@ export interface Parameter { decorators: Decorator[]|null; } +/** + * Definition of a function or method, including its body if present and any parameters. + * + * In TypeScript code this metadata will be a simple reflection of the declarations in the node + * itself. In ES5 code this can be more complicated, as the default values for parameters may + * be extracted from certain body statements. + */ +export interface FunctionDefinition { + /** + * A reference to the node which declares the function. + */ + node: T; + + /** + * Statements of the function body, if a body is present, or null if no body is present. + * + * This list may have been filtered to exclude statements which perform parameter default value + * initialization. + */ + body: ts.Statement[]|null; + + /** + * Metadata regarding the function's parameters, including possible default value expressions. + */ + parameters: Parameter[]; +} + +/** + * A parameter to a function or method. + */ +export interface Parameter { + /** + * Name of the parameter, if available. + */ + name: string|null; + + /** + * Declaration which created this parameter. + */ + node: ts.ParameterDeclaration; + + /** + * Expression which represents the default value of the parameter, if any. + */ + initializer: ts.Expression|null; +} + /** * The source of an imported symbol, including the original symbol name and the module from which it * was imported. @@ -273,7 +321,30 @@ export interface ReflectionHost { * a constructor exists. If the constructor exists and has 0 parameters, this array will be empty. * If the class has no constructor, this method returns `null`. */ - getConstructorParameters(declaration: ts.Declaration): Parameter[]|null; + getConstructorParameters(declaration: ts.Declaration): CtorParameter[]|null; + + /** + * Reflect over a function and return metadata about its parameters and body. + * + * Functions in TypeScript and ES5 code have different AST representations, in particular around + * default values for parameters. A TypeScript function has its default value as the initializer + * on the parameter declaration, whereas an ES5 function has its default value set in a statement + * of the form: + * + * if (param === void 0) { param = 3; } + * + * This method abstracts over these details, and interprets the function declaration and body to + * extract parameter default values and the "real" body. + * + * A current limitation is that this metadata has no representation for shorthand assignment of + * parameter objects in the function signature. + * + * @param fn a TypeScript `ts.Declaration` node representing the function over which to reflect. + * + * @returns a `FunctionDefinition` giving metadata about the function definition. + */ + getDefinitionOfFunction(fn: T): FunctionDefinition; /** * Determine if an identifier was imported from another module and return `Import` metadata diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts b/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts index 2332fe94c1e7..04b94a20e8b2 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {ClassMember, ClassMemberKind, Declaration, Decorator, Import, Parameter, ReflectionHost} from '../../host'; +import {ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, FunctionDefinition, Import, ReflectionHost} from '../../host'; /** * reflector.ts implements static reflection of declarations using the TypeScript `ts.TypeChecker`. @@ -31,7 +31,7 @@ export class TypeScriptReflectionHost implements ReflectionHost { .filter((member): member is ClassMember => member !== null); } - getConstructorParameters(declaration: ts.Declaration): Parameter[]|null { + getConstructorParameters(declaration: ts.Declaration): CtorParameter[]|null { const clazz = castDeclarationToClassOrDie(declaration); // First, find the constructor. @@ -146,6 +146,19 @@ export class TypeScriptReflectionHost implements ReflectionHost { return this.getDeclarationOfSymbol(symbol); } + getDefinitionOfFunction(node: T): FunctionDefinition { + return { + node, + body: node.body !== undefined ? Array.from(node.body.statements) : null, + parameters: node.parameters.map(param => { + const name = parameterName(param.name); + const initializer = param.initializer || null; + return {name, node: param, initializer}; + }), + }; + } + /** * Resolve a `ts.Symbol` to its declaration, keeping track of the `viaModule` along the way. * diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts index acc8ff3f17ff..37875e50b2ac 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts @@ -547,11 +547,11 @@ class StaticInterpreter { `calling something that is not a function declaration? ${ts.SyntaxKind[lhs.node.kind]} (${node.getText()})`); } - const fn = lhs.node; + const fn = this.host.getDefinitionOfFunction(lhs.node); // If the function is foreign (declared through a d.ts file), attempt to resolve it with the // foreignFunctionResolver, if one is specified. - if (fn.body === undefined) { + if (fn.body === null) { let expr: ts.Expression|null = null; if (context.foreignFunctionResolver) { expr = context.foreignFunctionResolver(lhs, node.arguments); @@ -572,10 +572,10 @@ class StaticInterpreter { } const body = fn.body; - if (body.statements.length !== 1 || !ts.isReturnStatement(body.statements[0])) { + if (body.length !== 1 || !ts.isReturnStatement(body[0])) { throw new Error('Function body must have a single return statement only.'); } - const ret = body.statements[0] as ts.ReturnStatement; + const ret = body[0] as ts.ReturnStatement; const newScope: Scope = new Map(); fn.parameters.forEach((param, index) => { @@ -584,10 +584,10 @@ class StaticInterpreter { const arg = node.arguments[index]; value = this.visitExpression(arg, context); } - if (value === undefined && param.initializer !== undefined) { + if (value === undefined && param.initializer !== null) { value = this.visitExpression(param.initializer, context); } - newScope.set(param, value); + newScope.set(param.node, value); }); return ret.expression !== undefined ? diff --git a/packages/compiler-cli/src/ngtsc/metadata/test/reflector_spec.ts b/packages/compiler-cli/src/ngtsc/metadata/test/reflector_spec.ts index 47e05f0642e9..71ff87090a58 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/test/reflector_spec.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/test/reflector_spec.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {Parameter} from '../../host'; +import {CtorParameter} from '../../host'; import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript'; import {TypeScriptReflectionHost} from '../src/reflector'; @@ -165,7 +165,7 @@ describe('reflector', () => { }); function expectParameter( - param: Parameter, name: string, type?: string, decorator?: string, + param: CtorParameter, name: string, type?: string, decorator?: string, decoratorFrom?: string): void { expect(param.name !).toEqual(name); if (type === undefined) { From f479fd5862e68777d3e3ecabcdcb46efe64ae8a8 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 25 Jul 2018 07:57:35 +0100 Subject: [PATCH 2/8] feat(ivy): implement `getDefinitionOfFunction` on ES2015 and ES5 reflection hosts --- .../src/ngcc/src/host/esm2015_host.ts | 7 +- .../src/ngcc/src/host/esm5_host.ts | 76 ++++++++++++- .../src/ngcc/test/host/esm2015_host_spec.ts | 100 +++++++++++++++++- .../src/ngcc/test/host/esm5_host_spec.ts | 85 +++++++++++++++ 4 files changed, 260 insertions(+), 8 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts index a2dff5d7df03..97605c5538df 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts @@ -8,9 +8,10 @@ import * as ts from 'typescript'; -import {ClassMember, ClassMemberKind, Decorator, Parameter} from '../../../ngtsc/host'; +import {ClassMember, ClassMemberKind, CtorParameter, Decorator} from '../../../ngtsc/host'; import {TypeScriptReflectionHost, reflectObjectLiteral} from '../../../ngtsc/metadata'; import {getNameText} from '../utils'; + import {NgccReflectionHost} from './ngcc_host'; export const DECORATORS = 'decorators' as ts.__String; @@ -162,7 +163,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N * * @throws if `declaration` does not resolve to a class declaration. */ - getConstructorParameters(clazz: ts.Declaration): Parameter[]|null { + getConstructorParameters(clazz: ts.Declaration): CtorParameter[]|null { const classSymbol = this.getClassSymbol(clazz); if (!classSymbol) { throw new Error( @@ -170,7 +171,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } const parameterNodes = this.getConstructorParameterDeclarations(classSymbol); if (parameterNodes) { - const parameters: Parameter[] = []; + const parameters: CtorParameter[] = []; const decoratorInfo = this.getConstructorDecorators(classSymbol); parameterNodes.forEach((node, index) => { const info = decoratorInfo[index]; diff --git a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts index b79b13d0de20..db03b43db274 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts @@ -7,8 +7,9 @@ */ import * as ts from 'typescript'; -import {ClassMember, ClassMemberKind, Decorator} from '../../../ngtsc/host'; +import {ClassMember, ClassMemberKind, Decorator, FunctionDefinition, Parameter} from '../../../ngtsc/host'; import {reflectObjectLiteral} from '../../../ngtsc/metadata'; +import {getNameText} from '../utils'; import {CONSTRUCTOR_PARAMS, Esm2015ReflectionHost, getPropertyValueFromSymbol} from './esm2015_host'; /** @@ -54,6 +55,29 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { return undefined; } + /** + * Parse a function declaration to find the relevant metadata about it. + * In ESM5 we need to do special work with optional arguments to the function, since they get + * their own initializer statement that needs to be parsed and then not included in the "body" + * statements of the function. + * @param node the function declaration to parse. + */ + getDefinitionOfFunction(node: T): FunctionDefinition { + const parameters = + node.parameters.map(p => ({name: getNameText(p.name), node: p, initializer: null})); + let lookingForParamInitializers = true; + + const statements = node.body && node.body.statements.filter(s => { + lookingForParamInitializers = + lookingForParamInitializers && reflectParamInitializer(s, parameters); + // If we are no longer looking for parameter initializers then we include this statement + return !lookingForParamInitializers; + }); + + return {node, body: statements || null, parameters}; + } + /** * Find the declarations of the constructor parameters of a class identified by its symbol. * In ESM5 there is no "class" so the constructor that we want is actually the declaration @@ -134,4 +158,52 @@ function getReturnStatement(declaration: ts.Expression | undefined): ts.ReturnSt function reflectArrayElement(element: ts.Expression) { return ts.isObjectLiteralExpression(element) ? reflectObjectLiteral(element) : null; -} \ No newline at end of file +} + +/** + * Parse the statement to extract the ESM5 parameter initializer if there is one. + * If one is found, add it to the appropriate parameter in the `parameters` collection. + * + * The form we are looking for is: + * + * ``` + * if (arg === void 0) { arg = initializer; } + * ``` + * + * @param statement A statement that may be initializing an optional parameter + * @param parameters The collection of parameters that were found in the function definition + * @returns true if the statement was a parameter initializer + */ +function reflectParamInitializer(statement: ts.Statement, parameters: Parameter[]) { + if (ts.isIfStatement(statement) && isUndefinedComparison(statement.expression) && + ts.isBlock(statement.thenStatement) && statement.thenStatement.statements.length === 1) { + const ifStatementComparison = statement.expression; // (arg === void 0) + const thenStatement = statement.thenStatement.statements[0]; // arg = initializer; + if (isAssignment(thenStatement)) { + const comparisonName = ifStatementComparison.left.text; + const assignmentName = thenStatement.expression.left.text; + if (comparisonName === assignmentName) { + const parameter = parameters.find(p => p.name === comparisonName); + if (parameter) { + parameter.initializer = thenStatement.expression.right; + return true; + } + } + } + } + return false; +} + +function isUndefinedComparison(expression: ts.Expression): expression is ts.Expression& + {left: ts.Identifier, right: ts.Expression} { + return ts.isBinaryExpression(expression) && + expression.operatorToken.kind === ts.SyntaxKind.EqualsEqualsEqualsToken && + ts.isVoidExpression(expression.right) && ts.isIdentifier(expression.left); +} + +function isAssignment(statement: ts.Statement): statement is ts.ExpressionStatement& + {expression: {left: ts.Identifier, right: ts.Expression}} { + return ts.isExpressionStatement(statement) && ts.isBinaryExpression(statement.expression) && + statement.expression.operatorToken.kind === ts.SyntaxKind.EqualsToken && + ts.isIdentifier(statement.expression.left); +} diff --git a/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts index 03a35831cde1..b693399b81cb 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts @@ -353,6 +353,38 @@ const EXPORTS_FILES = [ }, ]; +const FUNCTION_BODY_FILE = { + name: '/function_body.js', + contents: ` + function foo(x) { + return x; + } + function bar(x, y = 42) { + return x + y; + } + function baz(x) { + let y; + if (y === void 0) { y = 42; } + return x; + } + let y; + function qux(x) { + if (x === void 0) { y = 42; } + return y; + } + function moo() { + let x; + if (x === void 0) { x = 42; } + return x; + } + let x; + function juu() { + if (x === void 0) { x = 42; } + return x; + } + ` +}; + describe('Esm2015ReflectionHost', () => { describe('getDecoratorsOfDeclaration()', () => { @@ -701,7 +733,7 @@ describe('Esm2015ReflectionHost', () => { }); }); - describe('getConstructorParameters', () => { + describe('getConstructorParameters()', () => { it('should find the decorated constructor parameters', () => { const program = makeProgram(SOME_DIRECTIVE_FILE); const host = new Esm2015ReflectionHost(program.getTypeChecker()); @@ -897,7 +929,69 @@ describe('Esm2015ReflectionHost', () => { }); }); - describe('getImportOfIdentifier', () => { + describe('getDefinitionOfFunction()', () => { + it('should return an object describing the function declaration passed as an argument', () => { + const program = makeProgram(FUNCTION_BODY_FILE); + const host = new Esm2015ReflectionHost(program.getTypeChecker()); + + const fooNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'foo', ts.isFunctionDeclaration) !; + const fooDef = host.getDefinitionOfFunction(fooNode); + expect(fooDef.node).toBe(fooNode); + expect(fooDef.body !.length).toEqual(1); + expect(fooDef.body ![0].getText()).toEqual(`return x;`); + expect(fooDef.parameters.length).toEqual(1); + expect(fooDef.parameters[0].name).toEqual('x'); + expect(fooDef.parameters[0].initializer).toBe(null); + + const barNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'bar', ts.isFunctionDeclaration) !; + const barDef = host.getDefinitionOfFunction(barNode); + expect(barDef.node).toBe(barNode); + expect(barDef.body !.length).toEqual(1); + expect(ts.isReturnStatement(barDef.body ![0])).toBeTruthy(); + expect(barDef.body ![0].getText()).toEqual(`return x + y;`); + expect(barDef.parameters.length).toEqual(2); + expect(barDef.parameters[0].name).toEqual('x'); + expect(fooDef.parameters[0].initializer).toBe(null); + expect(barDef.parameters[1].name).toEqual('y'); + expect(barDef.parameters[1].initializer !.getText()).toEqual('42'); + + const bazNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'baz', ts.isFunctionDeclaration) !; + const bazDef = host.getDefinitionOfFunction(bazNode); + expect(bazDef.node).toBe(bazNode); + expect(bazDef.body !.length).toEqual(3); + expect(bazDef.parameters.length).toEqual(1); + expect(bazDef.parameters[0].name).toEqual('x'); + expect(bazDef.parameters[0].initializer).toBe(null); + + const quxNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'qux', ts.isFunctionDeclaration) !; + const quxDef = host.getDefinitionOfFunction(quxNode); + expect(quxDef.node).toBe(quxNode); + expect(quxDef.body !.length).toEqual(2); + expect(quxDef.parameters.length).toEqual(1); + expect(quxDef.parameters[0].name).toEqual('x'); + expect(quxDef.parameters[0].initializer).toBe(null); + + const mooNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'moo', ts.isFunctionDeclaration) !; + const mooDef = host.getDefinitionOfFunction(mooNode); + expect(mooDef.node).toBe(mooNode); + expect(mooDef.body !.length).toEqual(3); + expect(mooDef.parameters).toEqual([]); + + const juuNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'juu', ts.isFunctionDeclaration) !; + const juuDef = host.getDefinitionOfFunction(juuNode); + expect(juuDef.node).toBe(juuNode); + expect(juuDef.body !.length).toEqual(2); + expect(juuDef.parameters).toEqual([]); + }); + }); + + describe('getImportOfIdentifier()', () => { it('should find the import of an identifier', () => { const program = makeProgram(...IMPORTS_FILES); const host = new Esm2015ReflectionHost(program.getTypeChecker()); @@ -929,7 +1023,7 @@ describe('Esm2015ReflectionHost', () => { }); }); - describe('getDeclarationOfIdentifier', () => { + describe('getDeclarationOfIdentifier()', () => { it('should return the declaration of a locally defined identifier', () => { const program = makeProgram(SOME_DIRECTIVE_FILE); const host = new Esm2015ReflectionHost(program.getTypeChecker()); diff --git a/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts index 172eefb98f8b..623bcbef8e7f 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts @@ -406,6 +406,43 @@ const EXPORTS_FILES = [ }, ]; +const FUNCTION_BODY_FILE = { + name: '/function_body.js', + contents: ` + function foo(x) { + return x; + } + function bar(x, y) { + if (y === void 0) { y = 42; } + return x + y; + } + function complex() { + var x = 42; + return 42; + } + function baz(x) { + var y; + if (x === void 0) { y = 42; } + return y; + } + var y; + function qux(x) { + if (x === void 0) { y = 42; } + return y; + } + function moo() { + var x; + if (x === void 0) { x = 42; } + return x; + } + var x; + function juu() { + if (x === void 0) { x = 42; } + return x; + } + ` +}; + describe('Esm5ReflectionHost', () => { describe('getDecoratorsOfDeclaration()', () => { @@ -928,6 +965,54 @@ describe('Esm5ReflectionHost', () => { }); }); + describe('getDefinitionOfFunction()', () => { + it('should return an object describing the function declaration passed as an argument', () => { + const program = makeProgram(FUNCTION_BODY_FILE); + const host = new Esm5ReflectionHost(program.getTypeChecker()); + + const fooNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'foo', ts.isFunctionDeclaration) !; + const fooDef = host.getDefinitionOfFunction(fooNode); + expect(fooDef.node).toBe(fooNode); + expect(fooDef.body !.length).toEqual(1); + expect(fooDef.body ![0].getText()).toEqual(`return x;`); + expect(fooDef.parameters.length).toEqual(1); + expect(fooDef.parameters[0].name).toEqual('x'); + expect(fooDef.parameters[0].initializer).toBe(null); + + const barNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'bar', ts.isFunctionDeclaration) !; + const barDef = host.getDefinitionOfFunction(barNode); + expect(barDef.node).toBe(barNode); + expect(barDef.body !.length).toEqual(1); + expect(ts.isReturnStatement(barDef.body ![0])).toBeTruthy(); + expect(barDef.body ![0].getText()).toEqual(`return x + y;`); + expect(barDef.parameters.length).toEqual(2); + expect(barDef.parameters[0].name).toEqual('x'); + expect(fooDef.parameters[0].initializer).toBe(null); + expect(barDef.parameters[1].name).toEqual('y'); + expect(barDef.parameters[1].initializer !.getText()).toEqual('42'); + + const bazNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'baz', ts.isFunctionDeclaration) !; + const bazDef = host.getDefinitionOfFunction(bazNode); + expect(bazDef.node).toBe(bazNode); + expect(bazDef.body !.length).toEqual(3); + expect(bazDef.parameters.length).toEqual(1); + expect(bazDef.parameters[0].name).toEqual('x'); + expect(bazDef.parameters[0].initializer).toBe(null); + + const quxNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'qux', ts.isFunctionDeclaration) !; + const quxDef = host.getDefinitionOfFunction(quxNode); + expect(quxDef.node).toBe(quxNode); + expect(quxDef.body !.length).toEqual(2); + expect(quxDef.parameters.length).toEqual(1); + expect(quxDef.parameters[0].name).toEqual('x'); + expect(quxDef.parameters[0].initializer).toBe(null); + }); + }); + describe('getImportOfIdentifier', () => { it('should find the import of an identifier', () => { const program = makeProgram(...IMPORTS_FILES); From da09227cae3bca5a380f8f3485f9db12a47bf116 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Mon, 16 Jul 2018 08:45:32 +0100 Subject: [PATCH 3/8] fix(ivy): allow `FunctionExpression` to indicate a method declaration In some code formats (e.g. ES5) methods can actually be function expressions. For example: ```js function MyClass() {} // this static method is declared as a function expression MyClass.staticMethod = function() { ... }; ``` --- packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts index 37875e50b2ac..b6539e5a198a 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts @@ -671,7 +671,8 @@ class StaticInterpreter { function isFunctionOrMethodReference(ref: Reference): ref is Reference { - return ts.isFunctionDeclaration(ref.node) || ts.isMethodDeclaration(ref.node); + return ts.isFunctionDeclaration(ref.node) || ts.isMethodDeclaration(ref.node) || + ts.isFunctionExpression(ref.node); } function literal(value: ResolvedValue): any { From d3fe3842a051680978c1f0ff48030370583ff68b Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Tue, 24 Jul 2018 16:51:44 +0300 Subject: [PATCH 4/8] refactor(ivy): remove unused arg from ngcc `Analyzer` --- packages/compiler-cli/src/ngcc/src/analyzer.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/analyzer.ts b/packages/compiler-cli/src/ngcc/src/analyzer.ts index f7be4aa09246..f55a6e6948b6 100644 --- a/packages/compiler-cli/src/ngcc/src/analyzer.ts +++ b/packages/compiler-cli/src/ngcc/src/analyzer.ts @@ -66,7 +66,7 @@ export class Analyzer { analyzeFile(file: ParsedFile): AnalyzedFile { const constantPool = new ConstantPool(); const analyzedClasses = - file.decoratedClasses.map(clazz => this.analyzeClass(file.sourceFile, constantPool, clazz)) + file.decoratedClasses.map(clazz => this.analyzeClass(constantPool, clazz)) .filter(isDefined); return { @@ -75,8 +75,7 @@ export class Analyzer { }; } - protected analyzeClass(file: ts.SourceFile, pool: ConstantPool, clazz: ParsedClass): AnalyzedClass - |undefined { + protected analyzeClass(pool: ConstantPool, clazz: ParsedClass): AnalyzedClass|undefined { const matchingHandlers = this.handlers .map(handler => ({ handler, From db88cb16ce61f14c7405ba1f9b4feca6126bf7de Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Tue, 24 Jul 2018 16:53:23 +0300 Subject: [PATCH 5/8] feat(ivy): add support for esm2015 and esm5 in ngcc `PackageParser` Since non-flat module formats (esm2015, esm5) have different structure than their flat counterparts (and since we are operating on JS files inside `node_modules/`, we need to configure TS to include deeply nested JS files (by specifying a sufficiently high `maxNodeModuleJsDepth`). Remains to be determined if this has any (noticeable) performance implications. --- packages/compiler-cli/src/ngcc/src/main.ts | 5 +++-- .../ngcc/src/transform/package_transformer.ts | 7 ++++++- packages/compiler-cli/test/ngcc/ngcc_spec.ts | 18 ++++++++++++++++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/main.ts b/packages/compiler-cli/src/ngcc/src/main.ts index fece2d136977..878507821c24 100644 --- a/packages/compiler-cli/src/ngcc/src/main.ts +++ b/packages/compiler-cli/src/ngcc/src/main.ts @@ -10,12 +10,13 @@ import {PackageTransformer} from './transform/package_transformer'; export function mainNgcc(args: string[]): number { const packagePath = resolve(args[0]); + const format = args[1] || 'fesm2015'; - // TODO: find all the package tyoes to transform + // TODO: find all the package types to transform // TODO: error/warning logging/handling etc const transformer = new PackageTransformer(); - transformer.transform(packagePath, 'fesm2015'); + transformer.transform(packagePath, format); return 0; } diff --git a/packages/compiler-cli/src/ngcc/src/transform/package_transformer.ts b/packages/compiler-cli/src/ngcc/src/transform/package_transformer.ts index ab974015c06a..67f26586430b 100644 --- a/packages/compiler-cli/src/ngcc/src/transform/package_transformer.ts +++ b/packages/compiler-cli/src/ngcc/src/transform/package_transformer.ts @@ -46,7 +46,12 @@ export class PackageTransformer { const targetNodeModules = sourceNodeModules.replace(/node_modules$/, 'node_modules_ngtsc'); const entryPointPaths = getEntryPoints(packagePath, format); entryPointPaths.forEach(entryPointPath => { - const options: ts.CompilerOptions = {allowJs: true, rootDir: entryPointPath}; + const options: ts.CompilerOptions = { + allowJs: true, + maxNodeModuleJsDepth: Infinity, + rootDir: entryPointPath, + }; + const host = ts.createCompilerHost(options); const packageProgram = ts.createProgram([entryPointPath], options, host); const entryPointFile = packageProgram.getSourceFile(entryPointPath) !; diff --git a/packages/compiler-cli/test/ngcc/ngcc_spec.ts b/packages/compiler-cli/test/ngcc/ngcc_spec.ts index aab460be005c..bb8fcd8ff49c 100644 --- a/packages/compiler-cli/test/ngcc/ngcc_spec.ts +++ b/packages/compiler-cli/test/ngcc/ngcc_spec.ts @@ -72,11 +72,11 @@ describe('ngcc behavioral tests', () => { setupNodeModules(support); }); - it('should run ngcc without errors', () => { + it('should run ngcc without errors for fesm2015', () => { const nodeModulesPath = path.join(basePath, 'node_modules'); console.error(nodeModulesPath); const commonPath = path.join(nodeModulesPath, '@angular/common'); - const exitCode = mainNgcc([commonPath]); + const exitCode = mainNgcc([commonPath, 'fesm2015']); console.warn(find('node_modules_ngtsc').filter(p => p.endsWith('.js') || p.endsWith('map'))); @@ -85,4 +85,18 @@ describe('ngcc behavioral tests', () => { expect(exitCode).toBe(0); }); + + it('should run ngcc without errors for esm2015', () => { + const nodeModulesPath = path.join(basePath, 'node_modules'); + console.error(nodeModulesPath); + const commonPath = path.join(nodeModulesPath, '@angular/common'); + const exitCode = mainNgcc([commonPath, 'esm2015']); + + console.warn(find('node_modules_ngtsc').filter(p => p.endsWith('.js') || p.endsWith('map'))); + + console.warn(cat('node_modules_ngtsc/@angular/common/esm2015/src/directives/ng_if.js').stdout); + console.warn(cat('node_modules_ngtsc/@angular/common/esm2015/http/src/module.js').stdout); + + expect(exitCode).toBe(0); + }); }); From 613bc8baf47156a65a552793b7a3a9bd69973ada Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Wed, 25 Jul 2018 13:01:58 +0300 Subject: [PATCH 6/8] fix(ivy): correctly detect classes in ngcc `Esm5ReflectionHost` --- .../src/ngcc/src/host/esm2015_host.ts | 11 +-- .../src/ngcc/src/host/esm5_host.ts | 66 ++++++++++--- .../src/ngcc/src/host/ngcc_host.ts | 2 +- .../src/ngcc/test/host/esm5_host_spec.ts | 98 +++++++++++++++++-- .../src/ngtsc/host/src/reflection.ts | 4 +- .../src/ngtsc/metadata/src/reflector.ts | 2 +- 6 files changed, 154 insertions(+), 29 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts index 97605c5538df..451ffbcbd241 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts @@ -188,12 +188,11 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } /** - * Find a symbol for a declaration that we think is a class. - * @param declaration The declaration whose symbol we are finding - * @returns the symbol for the declaration or `undefined` if it is not - * a "class" or has no symbol. + * Find a symbol for a node that we think is a class. + * @param node The node whose symbol we are finding. + * @returns The symbol for the node or `undefined` if it is not a "class" or has no symbol. */ - getClassSymbol(declaration: ts.Declaration): ts.Symbol|undefined { + getClassSymbol(declaration: ts.Node): ts.Symbol|undefined { return ts.isClassDeclaration(declaration) ? declaration.name && this.checker.getSymbolAtLocation(declaration.name) : undefined; @@ -423,4 +422,4 @@ function isNamedDeclaration(node: ts.Declaration): node is ts.NamedDeclaration { function isClassMemberType(node: ts.Declaration): node is ts.ClassElement| ts.PropertyAccessExpression|ts.BinaryExpression { return ts.isClassElement(node) || isPropertyAccess(node) || ts.isBinaryExpression(node); -} \ No newline at end of file +} diff --git a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts index db03b43db274..d5be24844485 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts @@ -33,24 +33,64 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { constructor(checker: ts.TypeChecker) { super(checker); } /** - * Check whether the given declaration node actually represents a class. + * Check whether the given node actually represents a class. */ - isClass(node: ts.Declaration): boolean { return !!this.getClassSymbol(node); } + isClass(node: ts.Node): boolean { return super.isClass(node) || !!this.getClassSymbol(node); } /** - * In ESM5 the implementation of a class is a function expression that is hidden inside an IIFE. + * Find a symbol for a node that we think is a class. + * + * In ES5, the implementation of a class is a function expression that is hidden inside an IIFE. * So we need to dig around inside to get hold of the "class" symbol. - * @param declaration the top level declaration that represents an exported class. + * + * `node` might be one of: + * - A class declaration (from a declaration file). + * - The declaration of the outer variable, which is assigned the result of the IIFE. + * - The function declaration inside the IIFE, which is eventually returned and assigned to the + * outer variable. + * + * @param node The top level declaration that represents an exported class or the function + * expression inside the IIFE. + * @returns The symbol for the node or `undefined` if it is not a "class" or has no symbol. */ - getClassSymbol(declaration: ts.Declaration): ts.Symbol|undefined { - if (ts.isVariableDeclaration(declaration)) { - const iifeBody = getIifeBody(declaration); - if (iifeBody) { - const innerClassIdentifier = getReturnIdentifier(iifeBody); - if (innerClassIdentifier) { - return this.checker.getSymbolAtLocation(innerClassIdentifier); - } - } + getClassSymbol(node: ts.Node): ts.Symbol|undefined { + const symbol = super.getClassSymbol(node); + if (symbol) return symbol; + + if (ts.isVariableDeclaration(node)) { + const iifeBody = getIifeBody(node); + if (!iifeBody) return undefined; + + const innerClassIdentifier = getReturnIdentifier(iifeBody); + if (!innerClassIdentifier) return undefined; + + return this.checker.getSymbolAtLocation(innerClassIdentifier); + } + + if (ts.isFunctionDeclaration(node)) { + // It might be the function expression inside the IIFE. We need to go 5 levels up... + + // 1. IIFE body. + let outerNode = node.parent; + if (!outerNode || !ts.isBlock(outerNode)) return undefined; + + // 2. IIFE function expression. + outerNode = outerNode.parent; + if (!outerNode || !ts.isFunctionExpression(outerNode)) return undefined; + + // 3. IIFE call expression. + outerNode = outerNode.parent; + if (!outerNode || !ts.isCallExpression(outerNode)) return undefined; + + // 4. Parenthesis around IIFE. + outerNode = outerNode.parent; + if (!outerNode || !ts.isParenthesizedExpression(outerNode)) return undefined; + + // 5. Outer variable declaration. + outerNode = outerNode.parent; + if (!outerNode || !ts.isVariableDeclaration(outerNode)) return undefined; + + return this.getClassSymbol(outerNode); } return undefined; } diff --git a/packages/compiler-cli/src/ngcc/src/host/ngcc_host.ts b/packages/compiler-cli/src/ngcc/src/host/ngcc_host.ts index ef32c719ed7c..c0746643fccb 100644 --- a/packages/compiler-cli/src/ngcc/src/host/ngcc_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/ngcc_host.ts @@ -12,5 +12,5 @@ import {ReflectionHost} from '../../../ngtsc/host'; * A reflection host that has extra methods for looking at non-Typescript package formats */ export interface NgccReflectionHost extends ReflectionHost { - getClassSymbol(declaration: ts.Declaration): ts.Symbol|undefined; + getClassSymbol(node: ts.Node): ts.Symbol|undefined; } diff --git a/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts index 623bcbef8e7f..7b0b6973ec56 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts @@ -8,6 +8,7 @@ import * as ts from 'typescript'; import {ClassMemberKind, Import} from '../../../ngtsc/host'; +import {Esm2015ReflectionHost} from '../../src/host/esm2015_host'; import {Esm5ReflectionHost} from '../../src/host/esm5_host'; import {getDeclaration, makeProgram} from '../helpers/utils'; @@ -50,7 +51,8 @@ const SIMPLE_CLASS_FILE = { name: '/simple_class.js', contents: ` var EmptyClass = (function() { - function EmptyClass() {} + function EmptyClass() { + } return EmptyClass; }()); var NoDecoratorConstructorClass = (function() { @@ -1122,20 +1124,104 @@ describe('Esm5ReflectionHost', () => { }); }); - describe('isClass()', () => { - it('should return true if a given node is an ES5 class declaration', () => { + describe('getClassSymbol()', () => { + let superGetClassSymbolSpy: jasmine.Spy; + + beforeEach(() => { + superGetClassSymbolSpy = spyOn(Esm2015ReflectionHost.prototype, 'getClassSymbol'); + }); + + it('should return the class symbol returned by the superclass (if any)', () => { + const mockNode = {} as ts.Node; + const mockSymbol = {} as ts.Symbol; + superGetClassSymbolSpy.and.returnValue(mockSymbol); + + const host = new Esm5ReflectionHost({} as any); + + expect(host.getClassSymbol(mockNode)).toBe(mockSymbol); + expect(superGetClassSymbolSpy).toHaveBeenCalledWith(mockNode); + }); + + it('should return the class symbol for an ES5 class (outer variable declaration)', () => { const program = makeProgram(SIMPLE_CLASS_FILE); const host = new Esm5ReflectionHost(program.getTypeChecker()); const node = getDeclaration(program, SIMPLE_CLASS_FILE.name, 'EmptyClass', ts.isVariableDeclaration); - expect(host.isClass(node)).toBe(true); + expect(host.getClassSymbol(node)).toBeDefined(); + }); + + it('should return the class symbol for an ES5 class (inner function declaration)', () => { + const program = makeProgram(SIMPLE_CLASS_FILE); + const host = new Esm5ReflectionHost(program.getTypeChecker()); + const outerNode = + getDeclaration(program, SIMPLE_CLASS_FILE.name, 'EmptyClass', ts.isVariableDeclaration); + const innerNode = + (((outerNode.initializer as ts.ParenthesizedExpression).expression as ts.CallExpression) + .expression as ts.FunctionExpression) + .body.statements.find(ts.isFunctionDeclaration) !; + + expect(host.getClassSymbol(innerNode)).toBeDefined(); + }); + + it('should return the same class symbol for outer and inner declarations', () => { + const program = makeProgram(SIMPLE_CLASS_FILE); + const host = new Esm5ReflectionHost(program.getTypeChecker()); + const outerNode = + getDeclaration(program, SIMPLE_CLASS_FILE.name, 'EmptyClass', ts.isVariableDeclaration); + const innerNode = + (((outerNode.initializer as ts.ParenthesizedExpression).expression as ts.CallExpression) + .expression as ts.FunctionExpression) + .body.statements.find(ts.isFunctionDeclaration) !; + + expect(host.getClassSymbol(innerNode)).toBe(host.getClassSymbol(outerNode)); }); - it('should return false if a given node is not an ES5 class declaration', () => { + it('should return undefined if node is not an ES5 class', () => { const program = makeProgram(FOO_FUNCTION_FILE); const host = new Esm5ReflectionHost(program.getTypeChecker()); const node = getDeclaration(program, FOO_FUNCTION_FILE.name, 'foo', ts.isFunctionDeclaration); - expect(host.isClass(node)).toBe(false); + expect(host.getClassSymbol(node)).toBeUndefined(); + }); + }); + + describe('isClass()', () => { + let host: Esm5ReflectionHost; + let mockNode: ts.Node; + let superIsClassSpy: jasmine.Spy; + let getClassSymbolSpy: jasmine.Spy; + + beforeEach(() => { + host = new Esm5ReflectionHost(null as any); + mockNode = {} as any; + + superIsClassSpy = spyOn(Esm2015ReflectionHost.prototype, 'isClass'); + getClassSymbolSpy = spyOn(Esm5ReflectionHost.prototype, 'getClassSymbol'); + }); + + it('should return true if superclass returns true', () => { + superIsClassSpy.and.returnValue(true); + + expect(host.isClass(mockNode)).toBe(true); + expect(superIsClassSpy).toHaveBeenCalledWith(mockNode); + expect(getClassSymbolSpy).not.toHaveBeenCalled(); + }); + + it('should return true if it can find a symbol for the class', () => { + superIsClassSpy.and.returnValue(false); + getClassSymbolSpy.and.returnValue(true); + + expect(host.isClass(mockNode)).toBe(true); + expect(superIsClassSpy).toHaveBeenCalledWith(mockNode); + expect(getClassSymbolSpy).toHaveBeenCalledWith(mockNode); + }); + + it('should return false if it cannot find a symbol for the class', () => { + superIsClassSpy.and.returnValue(false); + getClassSymbolSpy.and.returnValue(false); + + expect(host.isClass(mockNode)).toBe(false); + expect(superIsClassSpy).toHaveBeenCalledWith(mockNode); + expect(getClassSymbolSpy).toHaveBeenCalledWith(mockNode); }); }); }); diff --git a/packages/compiler-cli/src/ngtsc/host/src/reflection.ts b/packages/compiler-cli/src/ngtsc/host/src/reflection.ts index 845ea46790e2..21f4efede4ad 100644 --- a/packages/compiler-cli/src/ngtsc/host/src/reflection.ts +++ b/packages/compiler-cli/src/ngtsc/host/src/reflection.ts @@ -406,9 +406,9 @@ export interface ReflectionHost { getExportsOfModule(module: ts.Node): Map|null; /** - * Check whether the given declaration node actually represents a class. + * Check whether the given node actually represents a class. */ - isClass(node: ts.Declaration): boolean; + isClass(node: ts.Node): boolean; hasBaseClass(node: ts.Declaration): boolean; } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts b/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts index 04b94a20e8b2..608dde3c3ada 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts @@ -127,7 +127,7 @@ export class TypeScriptReflectionHost implements ReflectionHost { return map; } - isClass(node: ts.Declaration): boolean { + isClass(node: ts.Node): boolean { // In TypeScript code, classes are ts.ClassDeclarations. return ts.isClassDeclaration(node); } From b60e4b93bf0326588dacedc7fedcde053a5b9317 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Wed, 25 Jul 2018 13:06:32 +0300 Subject: [PATCH 7/8] feat(ivy): enable processing of esm5 format in ngcc --- .../ngcc/src/transform/package_transformer.ts | 3 + .../src/ngtsc/annotations/src/ng_module.ts | 66 +++++++++---------- packages/compiler-cli/test/ngcc/ngcc_spec.ts | 28 ++++++++ 3 files changed, 64 insertions(+), 33 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/transform/package_transformer.ts b/packages/compiler-cli/src/ngcc/src/transform/package_transformer.ts index 67f26586430b..044b0a1bbc40 100644 --- a/packages/compiler-cli/src/ngcc/src/transform/package_transformer.ts +++ b/packages/compiler-cli/src/ngcc/src/transform/package_transformer.ts @@ -81,6 +81,7 @@ export class PackageTransformer { case 'esm2015': case 'fesm2015': return new Esm2015ReflectionHost(program.getTypeChecker()); + case 'esm5': case 'fesm5': return new Esm5ReflectionHost(program.getTypeChecker()); default: @@ -93,6 +94,7 @@ export class PackageTransformer { case 'esm2015': case 'fesm2015': return new Esm2015FileParser(program, host); + case 'esm5': case 'fesm5': return new Esm5FileParser(program, host); default: @@ -105,6 +107,7 @@ export class PackageTransformer { case 'esm2015': case 'fesm2015': return new Esm2015Renderer(host); + case 'esm5': case 'fesm5': return new Esm5Renderer(host); default: diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index de80ac54b42d..4d3a0e92e2bc 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -64,21 +64,21 @@ export class NgModuleDecoratorHandler implements DecoratorHandler this._extractModuleFromModuleWithProvidersFn(ref.node)); - imports = resolveTypeList(importsMeta, 'imports'); + imports = this.resolveTypeList(importsMeta, 'imports'); } let exports: Reference[] = []; if (ngModule.has('exports')) { const exportsMeta = staticallyResolve( ngModule.get('exports') !, this.reflector, this.checker, ref => this._extractModuleFromModuleWithProvidersFn(ref.node)); - exports = resolveTypeList(exportsMeta, 'exports'); + exports = this.resolveTypeList(exportsMeta, 'exports'); } // Register this module's information with the SelectorScopeRegistry. This ensures that during @@ -181,39 +181,39 @@ export class NgModuleDecoratorHandler implements DecoratorHandler { - // Unwrap ModuleWithProviders for modules that are locally declared (and thus static resolution - // was able to descend into the function and return an object literal, a Map). - if (entry instanceof Map && entry.has('ngModule')) { - entry = entry.get('ngModule') !; + /** + * Compute a list of `Reference`s from a resolved metadata value. + */ + private resolveTypeList(resolvedList: ResolvedValue, name: string): Reference[] { + const refList: Reference[] = []; + if (!Array.isArray(resolvedList)) { + throw new Error(`Expected array when reading property ${name}`); } - if (Array.isArray(entry)) { - // Recurse into nested arrays. - refList.push(...resolveTypeList(entry, name)); - } else if (entry instanceof Reference) { - if (!entry.expressable) { - throw new Error(`Value at position ${idx} in ${name} array is not expressable`); - } else if (!ts.isClassDeclaration(entry.node)) { - throw new Error(`Value at position ${idx} in ${name} array is not a class declaration`); + resolvedList.forEach((entry, idx) => { + // Unwrap ModuleWithProviders for modules that are locally declared (and thus static + // resolution was able to descend into the function and return an object literal, a Map). + if (entry instanceof Map && entry.has('ngModule')) { + entry = entry.get('ngModule') !; } - refList.push(entry); - } else { - // TODO(alxhub): expand ModuleWithProviders. - throw new Error(`Value at position ${idx} in ${name} array is not a reference: ${entry}`); - } - }); - return refList; + if (Array.isArray(entry)) { + // Recurse into nested arrays. + refList.push(...this.resolveTypeList(entry, name)); + } else if (entry instanceof Reference) { + if (!entry.expressable) { + throw new Error(`Value at position ${idx} in ${name} array is not expressable`); + } else if (!this.reflector.isClass(entry.node)) { + throw new Error(`Value at position ${idx} in ${name} array is not a class declaration`); + } + refList.push(entry); + } else { + // TODO(alxhub): expand ModuleWithProviders. + throw new Error(`Value at position ${idx} in ${name} array is not a reference: ${entry}`); + } + }); + + return refList; + } } diff --git a/packages/compiler-cli/test/ngcc/ngcc_spec.ts b/packages/compiler-cli/test/ngcc/ngcc_spec.ts index bb8fcd8ff49c..bf913fbed1b1 100644 --- a/packages/compiler-cli/test/ngcc/ngcc_spec.ts +++ b/packages/compiler-cli/test/ngcc/ngcc_spec.ts @@ -86,6 +86,20 @@ describe('ngcc behavioral tests', () => { expect(exitCode).toBe(0); }); + it('should run ngcc without errors for fesm5', () => { + const nodeModulesPath = path.join(basePath, 'node_modules'); + console.error(nodeModulesPath); + const commonPath = path.join(nodeModulesPath, '@angular/common'); + const exitCode = mainNgcc([commonPath, 'fesm5']); + + console.warn(find('node_modules_ngtsc').filter(p => p.endsWith('.js') || p.endsWith('map'))); + + console.warn(cat('node_modules_ngtsc/@angular/common/fesm5/common.js').stdout); + console.warn(cat('node_modules_ngtsc/@angular/common/fesm5/common.js.map').stdout); + + expect(exitCode).toBe(0); + }); + it('should run ngcc without errors for esm2015', () => { const nodeModulesPath = path.join(basePath, 'node_modules'); console.error(nodeModulesPath); @@ -99,4 +113,18 @@ describe('ngcc behavioral tests', () => { expect(exitCode).toBe(0); }); + + it('should run ngcc without errors for esm5', () => { + const nodeModulesPath = path.join(basePath, 'node_modules'); + console.error(nodeModulesPath); + const commonPath = path.join(nodeModulesPath, '@angular/common'); + const exitCode = mainNgcc([commonPath, 'esm5']); + + console.warn(find('node_modules_ngtsc').filter(p => p.endsWith('.js') || p.endsWith('map'))); + + console.warn(cat('node_modules_ngtsc/@angular/common/esm5/src/directives/ng_if.js').stdout); + console.warn(cat('node_modules_ngtsc/@angular/common/esm5/http/src/module.js').stdout); + + expect(exitCode).toBe(0); + }); }); From af5a439556b0f068cf617bfd9a8998537f089286 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 20 Aug 2018 16:35:06 +0300 Subject: [PATCH 8/8] fixup! fix(ivy): correctly detect classes in ngcc `Esm5ReflectionHost` --- packages/compiler-cli/src/ngcc/src/host/esm5_host.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts index d5be24844485..cab372ec3ef6 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts @@ -65,9 +65,7 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { if (!innerClassIdentifier) return undefined; return this.checker.getSymbolAtLocation(innerClassIdentifier); - } - - if (ts.isFunctionDeclaration(node)) { + } else if (ts.isFunctionDeclaration(node)) { // It might be the function expression inside the IIFE. We need to go 5 levels up... // 1. IIFE body. @@ -92,6 +90,7 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { return this.getClassSymbol(outerNode); } + return undefined; }