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
This commit is contained in:
Pete Bacon Darwin 2019-05-23 22:40:17 +01:00 committed by Kara Erickson
parent 869e3e8edc
commit 2dfd97d8f0
7 changed files with 155 additions and 29 deletions

View File

@ -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;

View File

@ -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;
}

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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'));
}
}