From 2dfd97d8f0d3652466678adc0e8ede8ab0338a76 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 23 May 2019 22:40:17 +0100 Subject: [PATCH] fix(ivy): ngcc - support bare array constructor param decorators (#30591) Previously we expected the constructor parameter `decorators` property to be an array wrapped in a function. Now we also support an array not wrapped in a function. PR Close #30591 --- .../ngcc/src/host/esm2015_host.ts | 38 +++++++++++++---- .../compiler-cli/ngcc/src/host/esm5_host.ts | 8 +++- .../ngcc/test/host/commonjs_host_spec.ts | 30 +++++++++++++ .../ngcc/test/host/esm2015_host_spec.ts | 26 ++++++++++++ .../ngcc/test/host/esm5_host_spec.ts | 36 ++++++++++++---- .../ngcc/test/host/umd_host_spec.ts | 42 +++++++++++++++---- .../src/ngtsc/annotations/src/util.ts | 4 +- 7 files changed, 155 insertions(+), 29 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts index 8d75c2f3f9..2e8f0ea154 100644 --- a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts @@ -1084,16 +1084,28 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N /** * Get the parameter type and decorators for the constructor of a class, - * where the information is stored on a static method of the class. + * where the information is stored on a static property of the class. * - * Note that in ESM2015, the method is defined by an arrow function that returns an array of - * decorator and type information. + * Note that in ESM2015, the property is defined an array, or by an arrow function that returns an + * array, of decorator and type information. + * + * For example, * * ``` * SomeDirective.ctorParameters = () => [ - * { type: ViewContainerRef, }, - * { type: TemplateRef, }, - * { type: undefined, decorators: [{ type: Inject, args: [INJECTED_TOKEN,] },] }, + * {type: ViewContainerRef}, + * {type: TemplateRef}, + * {type: undefined, decorators: [{ type: Inject, args: [INJECTED_TOKEN]}]}, + * ]; + * ``` + * + * or + * + * ``` + * SomeDirective.ctorParameters = [ + * {type: ViewContainerRef}, + * {type: TemplateRef}, + * {type: undefined, decorators: [{type: Inject, args: [INJECTED_TOKEN]}]}, * ]; * ``` * @@ -1102,9 +1114,12 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N */ protected getParamInfoFromStaticProperty(paramDecoratorsProperty: ts.Symbol): ParamInfo[]|null { const paramDecorators = getPropertyValueFromSymbol(paramDecoratorsProperty); - if (paramDecorators && ts.isArrowFunction(paramDecorators)) { - if (ts.isArrayLiteralExpression(paramDecorators.body)) { - const elements = paramDecorators.body.elements; + if (paramDecorators) { + // The decorators array may be wrapped in an arrow function. If so unwrap it. + const container = + ts.isArrowFunction(paramDecorators) ? paramDecorators.body : paramDecorators; + if (ts.isArrayLiteralExpression(container)) { + const elements = container.elements; return elements .map( element => @@ -1119,6 +1134,11 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N .filter(decorator => this.isFromCore(decorator)); return {typeExpression, decorators}; }); + } else if (paramDecorators !== undefined) { + this.logger.warn( + 'Invalid constructor parameter decorator in ' + + paramDecorators.getSourceFile().fileName + ':\n', + paramDecorators.getText()); } } return null; diff --git a/packages/compiler-cli/ngcc/src/host/esm5_host.ts b/packages/compiler-cli/ngcc/src/host/esm5_host.ts index ca6b5ebaee..2bf6960f0f 100644 --- a/packages/compiler-cli/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm5_host.ts @@ -355,8 +355,9 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { */ protected getParamInfoFromStaticProperty(paramDecoratorsProperty: ts.Symbol): ParamInfo[]|null { const paramDecorators = getPropertyValueFromSymbol(paramDecoratorsProperty); + // The decorators array may be wrapped in a function. If so unwrap it. const returnStatement = getReturnStatement(paramDecorators); - const expression = returnStatement && returnStatement.expression; + const expression = returnStatement ? returnStatement.expression : paramDecorators; if (expression && ts.isArrayLiteralExpression(expression)) { const elements = expression.elements; return elements.map(reflectArrayElement).map(paramInfo => { @@ -366,6 +367,11 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { const decorators = decoratorInfo && this.reflectDecorators(decoratorInfo); return {typeExpression, decorators}; }); + } else if (paramDecorators !== undefined) { + this.logger.warn( + 'Invalid constructor parameter decorator in ' + paramDecorators.getSourceFile().fileName + + ':\n', + paramDecorators.getText()); } return null; } diff --git a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts index 05804114b2..0e9145bf98 100644 --- a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts @@ -27,6 +27,7 @@ runInEachFileSystem(() => { let SOME_DIRECTIVE_FILE: TestFile; let TOPLEVEL_DECORATORS_FILE: TestFile; + let CTOR_DECORATORS_ARRAY_FILE: TestFile; let SIMPLE_ES2015_CLASS_FILE: TestFile; let SIMPLE_CLASS_FILE: TestFile; let FOO_FUNCTION_FILE: TestFile; @@ -112,6 +113,20 @@ exports.SomeDirective = SomeDirective; ` }; + CTOR_DECORATORS_ARRAY_FILE = { + name: _('/ctor_decorated_as_array.js'), + contents: ` + var core = require('@angular/core'); + var CtorDecoratedAsArray = (function() { + function CtorDecoratedAsArray(arg1) { + } + CtorDecoratedAsArray.ctorParameters = [{ type: ParamType, decorators: [{ type: Inject },] }]; + return CtorDecoratedAsArray; + }()); + exports.SomeDirective = SomeDirective; + `, + }; + SIMPLE_ES2015_CLASS_FILE = { name: _('/simple_es2015_class.d.ts'), contents: ` @@ -1284,6 +1299,21 @@ exports.ExternalModule = ExternalModule; ]); }); + it('should accept `ctorParameters` as an array', () => { + loadTestFiles([CTOR_DECORATORS_ARRAY_FILE]); + const {program, host: compilerHost} = + makeTestBundleProgram(CTOR_DECORATORS_ARRAY_FILE.name); + const host = new CommonJsReflectionHost(new MockLogger(), false, program, compilerHost); + const classNode = getDeclaration( + program, CTOR_DECORATORS_ARRAY_FILE.name, 'CtorDecoratedAsArray', + isNamedVariableDeclaration); + const parameters = host.getConstructorParameters(classNode) !; + + expect(parameters).toBeDefined(); + expect(parameters.map(parameter => parameter.name)).toEqual(['arg1']); + expectTypeValueReferencesForParameters(parameters, ['ParamType']); + }); + it('should throw if the symbol is not a class', () => { loadTestFiles([FOO_FUNCTION_FILE]); const {program, host: compilerHost} = makeTestBundleProgram(FOO_FUNCTION_FILE.name); diff --git a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts index 8b95f3526f..505cfda15c 100644 --- a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts @@ -25,6 +25,7 @@ runInEachFileSystem(() => { let _: typeof absoluteFrom; let SOME_DIRECTIVE_FILE: TestFile; + let CTOR_DECORATORS_ARRAY_FILE: TestFile; let ACCESSORS_FILE: TestFile; let SIMPLE_CLASS_FILE: TestFile; let CLASS_EXPRESSION_FILE: TestFile; @@ -89,6 +90,17 @@ runInEachFileSystem(() => { `, }; + CTOR_DECORATORS_ARRAY_FILE = { + name: _('/ctor_decorated_as_array.js'), + contents: ` + class CtorDecoratedAsArray { + constructor(arg1) { + } + } + CtorDecoratedAsArray.ctorParameters = [{ type: ParamType, decorators: [{ type: Inject },] }]; + `, + }; + ACCESSORS_FILE = { name: _('/accessors.js'), contents: ` @@ -1078,6 +1090,20 @@ runInEachFileSystem(() => { parameters, ['ViewContainerRef', 'TemplateRef', null]); }); + it('should accept `ctorParameters` as an array', () => { + loadTestFiles([CTOR_DECORATORS_ARRAY_FILE]); + const {program} = makeTestBundleProgram(CTOR_DECORATORS_ARRAY_FILE.name); + const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const classNode = getDeclaration( + program, CTOR_DECORATORS_ARRAY_FILE.name, 'CtorDecoratedAsArray', + isNamedClassDeclaration); + const parameters = host.getConstructorParameters(classNode) !; + + expect(parameters).toBeDefined(); + expect(parameters.map(parameter => parameter.name)).toEqual(['arg1']); + expectTypeValueReferencesForParameters(parameters, ['ParamType']); + }); + it('should throw if the symbol is not a class', () => { loadTestFiles([FOO_FUNCTION_FILE]); const {program} = makeTestBundleProgram(FOO_FUNCTION_FILE.name); diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts index b09b4b1cda..3b2e34b027 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts @@ -26,6 +26,7 @@ runInEachFileSystem(() => { let _: typeof absoluteFrom; let SOME_DIRECTIVE_FILE: TestFile; + let CTOR_DECORATORS_ARRAY_FILE: TestFile; let ACCESSORS_FILE: TestFile; let SIMPLE_ES2015_CLASS_FILE: TestFile; let SIMPLE_CLASS_FILE: TestFile; @@ -112,6 +113,18 @@ runInEachFileSystem(() => { `, }; + CTOR_DECORATORS_ARRAY_FILE = { + name: _('/ctor_decorated_as_array.js'), + contents: ` + var CtorDecoratedAsArray = (function() { + function CtorDecoratedAsArray(arg1) { + } + CtorDecoratedAsArray.ctorParameters = [{ type: ParamType, decorators: [{ type: Inject },] }]; + return CtorDecoratedAsArray; + }()); + `, + }; + ACCESSORS_FILE = { name: _('/accessors.js'), contents: ` @@ -368,15 +381,6 @@ runInEachFileSystem(() => { return NoParameters; }()); - var ArrowFunction = (function() { - function ArrowFunction(arg1) { - } - ArrowFunction.ctorParameters = () => [ - { type: 'ParamType', decorators: [{ type: Inject },] } - ]; - return ArrowFunction; - }()); - var NotArrayLiteral = (function() { function NotArrayLiteral(arg1) { } @@ -1134,6 +1138,20 @@ runInEachFileSystem(() => { expect(staticProperty.value !.getText()).toEqual(`'static'`); }); + it('should accept `ctorParameters` as an array', () => { + loadTestFiles([CTOR_DECORATORS_ARRAY_FILE]); + const {program} = makeTestBundleProgram(CTOR_DECORATORS_ARRAY_FILE.name); + const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const classNode = getDeclaration( + program, CTOR_DECORATORS_ARRAY_FILE.name, 'CtorDecoratedAsArray', + isNamedVariableDeclaration); + const parameters = host.getConstructorParameters(classNode) !; + + expect(parameters).toBeDefined(); + expect(parameters.map(parameter => parameter.name)).toEqual(['arg1']); + expectTypeValueReferencesForParameters(parameters, ['ParamType']); + }); + it('should throw if the symbol is not a class', () => { loadTestFiles([FOO_FUNCTION_FILE]); const {program} = makeTestBundleProgram(FOO_FUNCTION_FILE.name); diff --git a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts index 2edceabf97..74bbce8dfc 100644 --- a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts @@ -27,6 +27,7 @@ runInEachFileSystem(() => { let SOME_DIRECTIVE_FILE: TestFile; let TOPLEVEL_DECORATORS_FILE: TestFile; + let CTOR_DECORATORS_ARRAY_FILE: TestFile; let SIMPLE_ES2015_CLASS_FILE: TestFile; let SIMPLE_CLASS_FILE: TestFile; let FOO_FUNCTION_FILE: TestFile; @@ -120,6 +121,23 @@ runInEachFileSystem(() => { })));`, }; + CTOR_DECORATORS_ARRAY_FILE = { + name: _('/ctor_decorated_as_array.js'), + contents: ` + (function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@angular/core')) : + typeof define === 'function' && define.amd ? define('ctor_decorated_as_array', ['exports', '@angular/core'], factory) : + (factory(global.ctor_decorated_as_array,global.ng.core)); + }(this, (function (exports,core) { 'use strict'; + var CtorDecoratedAsArray = (function() { + function CtorDecoratedAsArray(arg1) { + } + CtorDecoratedAsArray.ctorParameters = [{ type: ParamType, decorators: [{ type: Inject },] }]; + return CtorDecoratedAsArray; + }()); + exports.CtorDecoratedAsArray = CtorDecoratedAsArray; + })));`, + }; SIMPLE_ES2015_CLASS_FILE = { name: _('/simple_es2015_class.d.ts'), @@ -360,15 +378,6 @@ runInEachFileSystem(() => { return NoParameters; }()); - var ArrowFunction = (function() { - function ArrowFunction(arg1) { - } - ArrowFunction.ctorParameters = () => [ - { type: 'ParamType', decorators: [{ type: core.Inject },] } - ]; - return ArrowFunction; - }()); - var NotArrayLiteral = (function() { function NotArrayLiteral(arg1) { } @@ -1391,6 +1400,21 @@ runInEachFileSystem(() => { ]); }); + it('should accept `ctorParameters` as an array', () => { + loadTestFiles([CTOR_DECORATORS_ARRAY_FILE]); + const {program, host: compilerHost} = + makeTestBundleProgram(CTOR_DECORATORS_ARRAY_FILE.name); + const host = new UmdReflectionHost(new MockLogger(), false, program, compilerHost); + const classNode = getDeclaration( + program, CTOR_DECORATORS_ARRAY_FILE.name, 'CtorDecoratedAsArray', + isNamedVariableDeclaration); + const parameters = host.getConstructorParameters(classNode) !; + + expect(parameters).toBeDefined(); + expect(parameters.map(parameter => parameter.name)).toEqual(['arg1']); + expectTypeValueReferencesForParameters(parameters, ['ParamType']); + }); + it('should throw if the symbol is not a class', () => { loadTestFiles([FOO_FUNCTION_FILE]); const {program, host: compilerHost} = makeTestBundleProgram(FOO_FUNCTION_FILE.name); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts index a9866b33e8..29af6e7523 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts @@ -146,7 +146,9 @@ export function validateConstructorDependencies( // There is at least one error. throw new FatalDiagnosticError( ErrorCode.PARAM_MISSING_TOKEN, param.nameNode, - `No suitable injection token for parameter '${param.name || index}' of class '${clazz.name!.text}'. Found: ${param.typeNode!.getText()}`); + `No suitable injection token for parameter '${param.name || index}' of class '${clazz.name.text}'.\n` + + (param.typeNode !== null ? `Found ${param.typeNode.getText()}` : + 'no type or decorator')); } }