fix(ivy): add support for optional nullable injection tokens (#27552)

FW-778 #resolve

PR Close #27552
This commit is contained in:
Igor Minar 2018-12-07 12:10:26 -08:00 committed by Miško Hevery
parent 37c05bd575
commit 7fabe4429d
14 changed files with 110 additions and 47 deletions

View File

@ -11,9 +11,6 @@ ng_module(
],
),
module_name = "app_built",
tags = [
"fixme-ivy-aot",
],
deps = [
"//packages/compiler-cli/integrationtest/bazel/injectable_def/lib2",
"//packages/core",

View File

@ -0,0 +1,9 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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
*/
export {RootAppModule} from './root';

View File

@ -10,9 +10,6 @@ ts_library(
"**/*.ts",
],
),
tags = [
"fixme-ivy-aot",
],
deps = [
"//packages/compiler-cli/integrationtest/bazel/injectable_def/app",
"//packages/core",
@ -25,9 +22,6 @@ ts_library(
jasmine_node_test(
name = "test",
bootstrap = ["angular/tools/testing/init_node_spec.js"],
tags = [
"fixme-ivy-aot",
],
deps = [
":test_lib",
"//packages/platform-server",

View File

@ -797,10 +797,11 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
this.getParamInfoFromHelperCall(classSymbol, parameterNodes);
return parameterNodes.map((node, index) => {
const {decorators, type} =
paramInfo && paramInfo[index] ? paramInfo[index] : {decorators: null, type: null};
const {decorators, typeExpression} = paramInfo && paramInfo[index] ?
paramInfo[index] :
{decorators: null, typeExpression: null};
const nameNode = node.name;
return {name: getNameText(nameNode), nameNode, type, decorators};
return {name: getNameText(nameNode), nameNode, typeExpression, typeNode: null, decorators};
});
}
@ -832,12 +833,12 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
element =>
ts.isObjectLiteralExpression(element) ? reflectObjectLiteral(element) : null)
.map(paramInfo => {
const type = paramInfo && paramInfo.get('type') || null;
const typeExpression = paramInfo && paramInfo.get('type') || null;
const decoratorInfo = paramInfo && paramInfo.get('decorators') || null;
const decorators = decoratorInfo &&
this.reflectDecorators(decoratorInfo)
.filter(decorator => this.isFromCore(decorator));
return {type, decorators};
return {typeExpression, decorators};
});
}
}
@ -857,7 +858,8 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
*/
protected getParamInfoFromHelperCall(
classSymbol: ts.Symbol, parameterNodes: ts.ParameterDeclaration[]): ParamInfo[] {
const parameters: ParamInfo[] = parameterNodes.map(() => ({type: null, decorators: null}));
const parameters: ParamInfo[] =
parameterNodes.map(() => ({typeExpression: null, decorators: null}));
const helperCalls = this.getHelperCallsForClass(classSymbol, '__decorate');
helperCalls.forEach(helperCall => {
const {classDecorators} =
@ -871,7 +873,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
metadataArg.text === 'design:paramtypes';
const types = typesArg && ts.isArrayLiteralExpression(typesArg) && typesArg.elements;
if (isParamTypeDecorator && types) {
types.forEach((type, index) => parameters[index].type = type);
types.forEach((type, index) => parameters[index].typeExpression = type);
}
break;
case '__param':
@ -1024,7 +1026,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
export type ParamInfo = {
decorators: Decorator[] | null,
type: ts.Expression | null
typeExpression: ts.Expression | null
};
/**

View File

@ -173,10 +173,10 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
if (expression && ts.isArrayLiteralExpression(expression)) {
const elements = expression.elements;
return elements.map(reflectArrayElement).map(paramInfo => {
const type = paramInfo && paramInfo.get('type') || null;
const typeExpression = paramInfo && paramInfo.get('type') || null;
const decoratorInfo = paramInfo && paramInfo.get('decorators') || null;
const decorators = decoratorInfo && this.reflectDecorators(decoratorInfo);
return {type, decorators};
return {typeExpression, decorators};
});
}
return null;

View File

@ -262,7 +262,7 @@ describe('Fesm2015ReflectionHost [import helper style]', () => {
expect(parameters !.map(parameter => parameter.name)).toEqual([
'_viewContainer', '_template', 'injected'
]);
expect(parameters !.map(parameter => parameter.type !.getText())).toEqual([
expect(parameters !.map(parameter => parameter.typeExpression !.getText())).toEqual([
'ViewContainerRef', 'TemplateRef', 'String'
]);
});
@ -296,7 +296,7 @@ describe('Fesm2015ReflectionHost [import helper style]', () => {
const classNode = getDeclaration(
program, '/some_directive.js', 'SomeDirective', ts.isVariableDeclaration);
const ctrDecorators = host.getConstructorParameters(classNode) !;
const identifierOfViewContainerRef = ctrDecorators[0].type !as ts.Identifier;
const identifierOfViewContainerRef = ctrDecorators[0].typeExpression !as ts.Identifier;
const expectedDeclarationNode = getDeclaration(
program, '/some_directive.js', 'ViewContainerRef', ts.isClassDeclaration);

View File

@ -841,7 +841,7 @@ describe('Fesm2015ReflectionHost', () => {
expect(parameters.map(parameter => parameter.name)).toEqual([
'_viewContainer', '_template', 'injected'
]);
expect(parameters.map(parameter => parameter.type !.getText())).toEqual([
expect(parameters.map(parameter => parameter.typeExpression !.getText())).toEqual([
'ViewContainerRef', 'TemplateRef', 'undefined'
]);
});
@ -1140,7 +1140,7 @@ describe('Fesm2015ReflectionHost', () => {
const classNode =
getDeclaration(program, SOME_DIRECTIVE_FILE.name, 'SomeDirective', ts.isClassDeclaration);
const ctrDecorators = host.getConstructorParameters(classNode) !;
const identifierOfViewContainerRef = ctrDecorators[0].type !as ts.Identifier;
const identifierOfViewContainerRef = ctrDecorators[0].typeExpression !as ts.Identifier;
const expectedDeclarationNode = getDeclaration(
program, SOME_DIRECTIVE_FILE.name, 'ViewContainerRef', ts.isVariableDeclaration);

View File

@ -277,7 +277,7 @@ describe('Esm5ReflectionHost [import helper style]', () => {
expect(parameters !.map(parameter => parameter.name)).toEqual([
'_viewContainer', '_template', 'injected'
]);
expect(parameters !.map(parameter => parameter.type !.getText())).toEqual([
expect(parameters !.map(parameter => parameter.typeExpression !.getText())).toEqual([
'ViewContainerRef', 'TemplateRef', 'String'
]);
});
@ -311,7 +311,7 @@ describe('Esm5ReflectionHost [import helper style]', () => {
const classNode = getDeclaration(
program, '/some_directive.js', 'SomeDirective', ts.isVariableDeclaration);
const ctrDecorators = host.getConstructorParameters(classNode) !;
const identifierOfViewContainerRef = ctrDecorators[0].type !as ts.Identifier;
const identifierOfViewContainerRef = ctrDecorators[0].typeExpression !as ts.Identifier;
const expectedDeclarationNode = getDeclaration(
program, '/some_directive.js', 'ViewContainerRef', ts.isVariableDeclaration);

View File

@ -824,7 +824,7 @@ describe('Esm5ReflectionHost', () => {
expect(parameters !.map(parameter => parameter.name)).toEqual([
'_viewContainer', '_template', 'injected'
]);
expect(parameters !.map(parameter => parameter.type !.getText())).toEqual([
expect(parameters !.map(parameter => parameter.typeExpression !.getText())).toEqual([
'ViewContainerRef', 'TemplateRef', 'undefined'
]);
});
@ -1079,7 +1079,7 @@ describe('Esm5ReflectionHost', () => {
const classNode = getDeclaration(
program, SOME_DIRECTIVE_FILE.name, 'SomeDirective', ts.isVariableDeclaration);
const ctrDecorators = host.getConstructorParameters(classNode) !;
const identifierOfViewContainerRef = ctrDecorators[0].type !as ts.Identifier;
const identifierOfViewContainerRef = ctrDecorators[0].typeExpression !as ts.Identifier;
const expectedDeclarationNode = getDeclaration(
program, SOME_DIRECTIVE_FILE.name, 'ViewContainerRef', ts.isVariableDeclaration);

View File

@ -82,7 +82,8 @@ export function generateSetClassMetadataCall(
function ctorParameterToMetadata(param: CtorParameter, isCore: boolean): ts.Expression {
// Parameters sometimes have a type that can be referenced. If so, then use it, otherwise
// its type is undefined.
const type = param.type !== null ? param.type : ts.createIdentifier('undefined');
const type =
param.typeExpression !== null ? param.typeExpression : ts.createIdentifier('undefined');
const properties: ts.ObjectLiteralElementLike[] = [
ts.createPropertyAssignment('type', type),
];

View File

@ -26,7 +26,7 @@ export function getConstructorDependencies(
}
}
ctorParams.forEach((param, idx) => {
let tokenExpr = param.type;
let tokenExpr = param.typeExpression;
let optional = false, self = false, skipSelf = false, host = false;
let resolved = R3ResolvedDependencyType.Token;
(param.decorators || []).filter(dec => isCore || isAngularCore(dec)).forEach(dec => {
@ -62,7 +62,7 @@ export function getConstructorDependencies(
if (tokenExpr === null) {
throw new FatalDiagnosticError(
ErrorCode.PARAM_MISSING_TOKEN, param.nameNode,
`No suitable token for parameter ${param.name || idx} of class ${clazz.name!.text}`);
`No suitable injection token for parameter '${param.name || idx}' of class '${clazz.name!.text}'. Found: ${param.typeNode!.getText()}`);
}
const token = new WrappedNodeExpr(tokenExpr);
useType.push({token, optional, self, skipSelf, host, resolved});

View File

@ -172,12 +172,22 @@ export interface CtorParameter {
nameNode: ts.BindingName;
/**
* TypeScript `ts.Expression` representing the type of the parameter, if the type is a simple
* expression type.
* TypeScript `ts.Expression` representing the type value of the parameter, if the type is a
* simple
* expression type that can be converted to a value.
*
* If the type is not present or cannot be represented as an expression, `type` is `null`.
*/
type: ts.Expression|null;
typeExpression: ts.Expression|null;
/**
* TypeScript `ts.TypeNode` representing the type node found in the type position.
*
* This field can be used for diagnostics reporting if `typeExpression` is `null`.
*
* Can be null, if the param has no type declared.
*/
typeNode: ts.TypeNode|null;
/**
* Any `Decorator`s which are present on the parameter, or `null` if none are present.

View File

@ -49,25 +49,43 @@ export class TypeScriptReflectionHost implements ReflectionHost {
// It may or may not be possible to write an expression that refers to the value side of the
// type named for the parameter.
let typeValueExpr: ts.Expression|null = null;
let originalTypeNode = node.type || null;
let typeNode = originalTypeNode;
// Check if we are dealing with a simple nullable union type e.g. `foo: Foo|null`
// and extract the type. More complext union types e.g. `foo: Foo|Bar` are not supported.
// We also don't need to support `foo: Foo|undefined` because Angular's DI injects `null` for
// optional tokes that don't have providers.
if (typeNode && ts.isUnionTypeNode(typeNode)) {
let childTypeNodes = typeNode.types.filter(
childTypeNode => childTypeNode.kind !== ts.SyntaxKind.NullKeyword);
if (childTypeNodes.length === 1) {
typeNode = childTypeNodes[0];
} else {
typeNode = null;
}
}
// It's not possible to get a value expression if the parameter doesn't even have a type.
if (node.type !== undefined) {
if (typeNode) {
// It's only valid to convert a type reference to a value reference if the type actually has
// a
// value declaration associated with it.
const type = this.checker.getTypeFromTypeNode(node.type);
if (type.symbol !== undefined && type.symbol.valueDeclaration !== undefined) {
// a value declaration associated with it.
let type: ts.Type|null = this.checker.getTypeFromTypeNode(typeNode);
if (type && type.symbol !== undefined && type.symbol.valueDeclaration !== undefined) {
// The type points to a valid value declaration. Rewrite the TypeReference into an
// Expression
// which references the value pointed to by the TypeReference, if possible.
typeValueExpr = typeNodeToValueExpr(node.type);
typeValueExpr = typeNodeToValueExpr(typeNode);
}
}
return {
name,
nameNode: node.name,
type: typeValueExpr, decorators,
typeExpression: typeValueExpr,
typeNode: originalTypeNode, decorators,
};
});
}

View File

@ -119,6 +119,34 @@ describe('reflector', () => {
expectParameter(args[0], 'bar', 'Bar');
expectParameter(args[1], 'otherBar', 'star.Bar');
});
it('should reflect an nullable argument', () => {
const {program} = makeProgram([
{
name: 'bar.ts',
contents: `
export class Bar {}
`
},
{
name: 'entry.ts',
contents: `
import {Bar} from './bar';
class Foo {
constructor(bar: Bar|null) {}
}
`
}
]);
const clazz = getDeclaration(program, 'entry.ts', 'Foo', ts.isClassDeclaration);
const checker = program.getTypeChecker();
const host = new TypeScriptReflectionHost(checker);
const args = host.getConstructorParameters(clazz) !;
expect(args.length).toBe(1);
expectParameter(args[0], 'bar', 'Bar');
});
});
it('should reflect a re-export', () => {
@ -169,10 +197,10 @@ function expectParameter(
decoratorFrom?: string): void {
expect(param.name !).toEqual(name);
if (type === undefined) {
expect(param.type).toBeNull();
expect(param.typeExpression).toBeNull();
} else {
expect(param.type).not.toBeNull();
expect(argExpressionToString(param.type !)).toEqual(type);
expect(param.typeExpression).not.toBeNull();
expect(argExpressionToString(param.typeExpression !)).toEqual(type);
}
if (decorator !== undefined) {
expect(param.decorators).not.toBeNull();
@ -184,7 +212,11 @@ function expectParameter(
}
}
function argExpressionToString(name: ts.Node): string {
function argExpressionToString(name: ts.Node | null): string {
if (name == null) {
throw new Error('\'name\' argument can\'t be null');
}
if (ts.isIdentifier(name)) {
return name.text;
} else if (ts.isPropertyAccessExpression(name)) {