From 862c08f64ec2161ecd779729a314ce1890d9ec0f Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Wed, 15 Mar 2017 09:24:56 -0700 Subject: [PATCH] fix(compiler): support interface types in injectable constuctors (#14894) Fixes #12631 --- .../compiler-cli/integrationtest/src/basic.ts | 4 +- .../integrationtest/src/custom_token.ts | 13 ++++++ .../integrationtest/src/module.ts | 2 + .../test/aot/static_reflector_spec.ts | 25 +++++++++++ .../test/aot/static_symbol_resolver_spec.ts | 4 +- tools/@angular/tsc-wrapped/src/bundler.ts | 5 ++- tools/@angular/tsc-wrapped/src/collector.ts | 31 ++++++++++--- tools/@angular/tsc-wrapped/src/schema.ts | 7 ++- .../tsc-wrapped/test/collector.spec.ts | 43 ++++++++++++++++++- 9 files changed, 122 insertions(+), 12 deletions(-) create mode 100644 packages/compiler-cli/integrationtest/src/custom_token.ts diff --git a/packages/compiler-cli/integrationtest/src/basic.ts b/packages/compiler-cli/integrationtest/src/basic.ts index 49f9998b2b3393..4021df9d500fdb 100644 --- a/packages/compiler-cli/integrationtest/src/basic.ts +++ b/packages/compiler-cli/integrationtest/src/basic.ts @@ -8,6 +8,7 @@ import {Component, Inject, LOCALE_ID, TRANSLATIONS_FORMAT} from '@angular/core'; +import {CUSTOM, Named} from './custom_token'; @Component({ selector: 'basic', @@ -21,7 +22,8 @@ export class BasicComp { ctxArr: any[] = []; constructor( @Inject(LOCALE_ID) public localeId: string, - @Inject(TRANSLATIONS_FORMAT) public translationsFormat: string) { + @Inject(TRANSLATIONS_FORMAT) public translationsFormat: string, + @Inject(CUSTOM) public custom: Named) { this.ctxProp = 'initialValue'; } } diff --git a/packages/compiler-cli/integrationtest/src/custom_token.ts b/packages/compiler-cli/integrationtest/src/custom_token.ts new file mode 100644 index 00000000000000..698b4dd5fe2e2e --- /dev/null +++ b/packages/compiler-cli/integrationtest/src/custom_token.ts @@ -0,0 +1,13 @@ +/** + * @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 + */ + +import {InjectionToken} from '@angular/core'; + +export interface Named { name: string; } + +export const CUSTOM = new InjectionToken('CUSTOM'); diff --git a/packages/compiler-cli/integrationtest/src/module.ts b/packages/compiler-cli/integrationtest/src/module.ts index 10149b33c28b06..363c5d565d6df5 100644 --- a/packages/compiler-cli/integrationtest/src/module.ts +++ b/packages/compiler-cli/integrationtest/src/module.ts @@ -20,6 +20,7 @@ import {MultipleComponentsMyComp, NextComp} from './a/multiple_components'; import {AnimateCmp} from './animate'; import {BasicComp} from './basic'; import {ComponentUsingThirdParty} from './comp_using_3rdp'; +import {CUSTOM, Named} from './custom_token'; import {CompWithAnalyzeEntryComponentsProvider, CompWithEntryComponents} from './entry_components'; import {CompConsumingEvents, CompUsingPipes, CompWithProviders, CompWithReferences, DirPublishingEvents, ModuleUsingCustomElements} from './features'; import {CompUsingRootModuleDirectiveAndPipe, SomeDirectiveInRootModule, SomeLibModule, SomePipeInRootModule, SomeService} from './module_fixtures'; @@ -75,6 +76,7 @@ export const SERVER_ANIMATIONS_PROVIDERS: Provider[] = [{ providers: [ SomeService, SERVER_ANIMATIONS_PROVIDERS, + {provide: CUSTOM, useValue: {name: 'some name'}}, ], entryComponents: [ AnimateCmp, diff --git a/packages/compiler/test/aot/static_reflector_spec.ts b/packages/compiler/test/aot/static_reflector_spec.ts index 454451afa343bc..b931593287c184 100644 --- a/packages/compiler/test/aot/static_reflector_spec.ts +++ b/packages/compiler/test/aot/static_reflector_spec.ts @@ -726,6 +726,31 @@ describe('StaticReflector', () => { expect(reflector.annotations(reflector.getStaticSymbol('/tmp/src/main.ts', 'Child'))) .toEqual([]); }); + + it('should support constructor parameters with @Inject and an interface type', () => { + const data = Object.create(DEFAULT_TEST_DATA); + const file = '/tmp/src/inject_interface.ts'; + data[file] = ` + import {Injectable, Inject} from '@angular/core'; + import {F} from './f'; + + export interface InjectedInterface { + + } + + export class Token {} + + @Injectable() + export class SomeClass { + constructor (@Inject(Token) injected: InjectedInterface, t: Token, @Inject(Token) f: F) {} + } + `; + + init(data); + + expect(reflector.parameters(reflector.getStaticSymbol(file, 'SomeClass'))[0].length) + .toEqual(1); + }); }); }); diff --git a/packages/compiler/test/aot/static_symbol_resolver_spec.ts b/packages/compiler/test/aot/static_symbol_resolver_spec.ts index 8037be76f0b337..01c4c158a84adf 100644 --- a/packages/compiler/test/aot/static_symbol_resolver_spec.ts +++ b/packages/compiler/test/aot/static_symbol_resolver_spec.ts @@ -424,7 +424,9 @@ export class MockStaticSymbolResolverHost implements StaticSymbolResolverHost { filePath, this.data[filePath], ts.ScriptTarget.ES5, /* setParentNodes */ true); const diagnostics: ts.Diagnostic[] = (sf).parseDiagnostics; if (diagnostics && diagnostics.length) { - throw Error(`Error encountered during parse of file ${filePath}`); + const errors = diagnostics.map(d => `(${d.start}-${d.start+d.length}): ${d.messageText}`) + .join('\n '); + throw Error(`Error encountered during parse of file ${filePath}\n${errors}`); } return [this.collector.getMetadata(sf)]; } diff --git a/tools/@angular/tsc-wrapped/src/bundler.ts b/tools/@angular/tsc-wrapped/src/bundler.ts index 46338aa54247f6..aa913acaedff00 100644 --- a/tools/@angular/tsc-wrapped/src/bundler.ts +++ b/tools/@angular/tsc-wrapped/src/bundler.ts @@ -9,8 +9,8 @@ import * as path from 'path'; import * as ts from 'typescript'; import {MetadataCollector} from './collector'; +import {ClassMetadata, ConstructorMetadata, FunctionMetadata, MemberMetadata, MetadataArray, MetadataEntry, MetadataError, MetadataImportedSymbolReferenceExpression, MetadataMap, MetadataObject, MetadataSymbolicBinaryExpression, MetadataSymbolicCallExpression, MetadataSymbolicExpression, MetadataSymbolicIfExpression, MetadataSymbolicIndexExpression, MetadataSymbolicPrefixExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataSymbolicSpreadExpression, MetadataValue, MethodMetadata, ModuleMetadata, VERSION, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isInterfaceMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataImportedSymbolReferenceExpression, isMetadataModuleReferenceExpression, isMetadataSymbolicExpression, isMetadataSymbolicReferenceExpression, isMethodMetadata} from './schema'; -import {ClassMetadata, ConstructorMetadata, FunctionMetadata, MemberMetadata, MetadataArray, MetadataEntry, MetadataError, MetadataImportedSymbolReferenceExpression, MetadataMap, MetadataObject, MetadataSymbolicBinaryExpression, MetadataSymbolicCallExpression, MetadataSymbolicExpression, MetadataSymbolicIfExpression, MetadataSymbolicIndexExpression, MetadataSymbolicPrefixExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataSymbolicSpreadExpression, MetadataValue, MethodMetadata, ModuleMetadata, VERSION, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataImportedSymbolReferenceExpression, isMetadataModuleReferenceExpression, isMetadataSymbolicExpression, isMetadataSymbolicReferenceExpression, isMethodMetadata} from './schema'; // The character set used to produce private names. const PRIVATE_NAME_CHARS = [ @@ -268,6 +268,9 @@ export class MetadataBundler { if (isFunctionMetadata(value)) { return this.convertFunction(moduleName, value); } + if (isInterfaceMetadata(value)) { + return value; + } return this.convertValue(moduleName, value); } diff --git a/tools/@angular/tsc-wrapped/src/collector.ts b/tools/@angular/tsc-wrapped/src/collector.ts index 15658ee9dc6558..43c24a27f53487 100644 --- a/tools/@angular/tsc-wrapped/src/collector.ts +++ b/tools/@angular/tsc-wrapped/src/collector.ts @@ -9,7 +9,7 @@ import * as ts from 'typescript'; import {Evaluator, errorSymbol} from './evaluator'; -import {ClassMetadata, ConstructorMetadata, FunctionMetadata, MemberMetadata, MetadataEntry, MetadataError, MetadataMap, MetadataSymbolicBinaryExpression, MetadataSymbolicCallExpression, MetadataSymbolicExpression, MetadataSymbolicIfExpression, MetadataSymbolicIndexExpression, MetadataSymbolicPrefixExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataSymbolicSpreadExpression, MetadataValue, MethodMetadata, ModuleExportMetadata, ModuleMetadata, VERSION, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataSymbolicExpression, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSelectExpression, isMethodMetadata} from './schema'; +import {ClassMetadata, ConstructorMetadata, FunctionMetadata, InterfaceMetadata, MemberMetadata, MetadataEntry, MetadataError, MetadataMap, MetadataSymbolicBinaryExpression, MetadataSymbolicCallExpression, MetadataSymbolicExpression, MetadataSymbolicIfExpression, MetadataSymbolicIndexExpression, MetadataSymbolicPrefixExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataSymbolicSpreadExpression, MetadataValue, MethodMetadata, ModuleExportMetadata, ModuleMetadata, VERSION, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataSymbolicExpression, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSelectExpression, isMethodMetadata} from './schema'; import {Symbols} from './symbols'; // In TypeScript 2.1 these flags moved @@ -56,7 +56,8 @@ export class MetadataCollector { */ public getMetadata(sourceFile: ts.SourceFile, strict: boolean = false): ModuleMetadata { const locals = new Symbols(sourceFile); - const nodeMap = new Map(); + const nodeMap = + new Map(); const evaluator = new Evaluator(locals, nodeMap, this.options); let metadata: {[name: string]: MetadataValue | ClassMetadata | FunctionMetadata}|undefined; let exports: ModuleExportMetadata[]; @@ -264,13 +265,14 @@ export class MetadataCollector { }); const isExportedIdentifier = (identifier: ts.Identifier) => exportMap.has(identifier.text); - const isExported = (node: ts.FunctionDeclaration | ts.ClassDeclaration | ts.EnumDeclaration) => - isExport(node) || isExportedIdentifier(node.name); + const isExported = + (node: ts.FunctionDeclaration | ts.ClassDeclaration | ts.InterfaceDeclaration | + ts.EnumDeclaration) => isExport(node) || isExportedIdentifier(node.name); const exportedIdentifierName = (identifier: ts.Identifier) => exportMap.get(identifier.text) || identifier.text; const exportedName = - (node: ts.FunctionDeclaration | ts.ClassDeclaration | ts.EnumDeclaration) => - exportedIdentifierName(node.name); + (node: ts.FunctionDeclaration | ts.ClassDeclaration | ts.InterfaceDeclaration | + ts.EnumDeclaration) => exportedIdentifierName(node.name); // Predeclare classes and functions @@ -290,6 +292,15 @@ export class MetadataCollector { } break; + case ts.SyntaxKind.InterfaceDeclaration: + const interfaceDeclaration = node; + if (interfaceDeclaration.name) { + const interfaceName = interfaceDeclaration.name.text; + // All references to interfaces should be converted to references to `any`. + locals.define(interfaceName, {__symbolic: 'reference', name: 'any'}); + } + break; + case ts.SyntaxKind.FunctionDeclaration: const functionDeclaration = node; if (!isExported(functionDeclaration)) { @@ -356,6 +367,14 @@ export class MetadataCollector { // Otherwise don't record metadata for the class. break; + case ts.SyntaxKind.InterfaceDeclaration: + const interfaceDeclaration = node; + if (interfaceDeclaration.name && isExported(interfaceDeclaration)) { + if (!metadata) metadata = {}; + metadata[exportedName(interfaceDeclaration)] = {__symbolic: 'interface'}; + } + break; + case ts.SyntaxKind.FunctionDeclaration: // Record functions that return a single value. Record the parameter // names substitution will be performed by the StaticReflector. diff --git a/tools/@angular/tsc-wrapped/src/schema.ts b/tools/@angular/tsc-wrapped/src/schema.ts index 8ac91241979d5b..7b7fbc97293a89 100644 --- a/tools/@angular/tsc-wrapped/src/schema.ts +++ b/tools/@angular/tsc-wrapped/src/schema.ts @@ -17,7 +17,7 @@ export const VERSION = 3; -export type MetadataEntry = ClassMetadata | FunctionMetadata | MetadataValue; +export type MetadataEntry = ClassMetadata | InterfaceMetadata | FunctionMetadata | MetadataValue; export interface ModuleMetadata { __symbolic: 'module'; @@ -47,6 +47,11 @@ export function isClassMetadata(value: any): value is ClassMetadata { return value && value.__symbolic === 'class'; } +export interface InterfaceMetadata { __symbolic: 'interface'; } +export function isInterfaceMetadata(value: any): value is InterfaceMetadata { + return value && value.__symbolic === 'interface'; +} + export interface MetadataMap { [name: string]: MemberMetadata[]; } export interface MemberMetadata { diff --git a/tools/@angular/tsc-wrapped/test/collector.spec.ts b/tools/@angular/tsc-wrapped/test/collector.spec.ts index 396730aafe6acd..f179d64fe76c14 100644 --- a/tools/@angular/tsc-wrapped/test/collector.spec.ts +++ b/tools/@angular/tsc-wrapped/test/collector.spec.ts @@ -50,7 +50,8 @@ describe('Collector', () => { 'static-method-with-default.ts', 'class-inheritance.ts', 'class-inheritance-parent.ts', - 'class-inheritance-declarations.d.ts' + 'class-inheritance-declarations.d.ts', + 'interface-reference.ts' ]); service = ts.createLanguageService(host, documentRegistry); program = service.getProgram(); @@ -60,11 +61,18 @@ describe('Collector', () => { it('should not have errors in test data', () => { expectValidSources(service, program); }); it('should return undefined for modules that have no metadata', () => { - const sourceFile = program.getSourceFile('app/hero.ts'); + const sourceFile = program.getSourceFile('app/empty.ts'); const metadata = collector.getMetadata(sourceFile); expect(metadata).toBeUndefined(); }); + it('should return an interface reference for interfaces', () => { + const sourceFile = program.getSourceFile('app/hero.ts'); + const metadata = collector.getMetadata(sourceFile); + expect(metadata).toEqual( + {__symbolic: 'module', version: 3, metadata: {Hero: {__symbolic: 'interface'}}}); + }); + it('should be able to collect a simple component\'s metadata', () => { const sourceFile = program.getSourceFile('app/hero-detail.component.ts'); const metadata = collector.getMetadata(sourceFile); @@ -609,6 +617,22 @@ describe('Collector', () => { }); }); + it('should collect any for interface parameter reference', () => { + const source = program.getSourceFile('/interface-reference.ts'); + const metadata = collector.getMetadata(source); + expect((metadata.metadata['SomeClass'] as ClassMetadata).members).toEqual({ + __ctor__: [{ + __symbolic: 'constructor', + parameterDecorators: [[{ + __symbolic: 'call', + expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Inject'}, + arguments: ['a'] + }]], + parameters: [{__symbolic: 'reference', name: 'any'}] + }] + }); + }); + describe('in strict mode', () => { it('should throw if an error symbol is collecting a reference to a non-exported symbol', () => { const source = program.getSourceFile('/local-symbol-ref.ts'); @@ -759,6 +783,7 @@ const FILES: Directory = { id: number; name: string; }`, + 'empty.ts': ``, 'hero-detail.component.ts': ` import {Component, Input} from 'angular2/core'; import {Hero} from './hero'; @@ -927,6 +952,15 @@ const FILES: Directory = { } } `, + 'interface-reference.ts': ` + import {Injectable, Inject} from 'angular2/core'; + export interface Test {} + + @Injectable() + export class SomeClass { + constructor(@Inject("a") test: Test) {} + } + `, 'import-star.ts': ` import {Injectable} from 'angular2/core'; import * as common from 'angular2/common'; @@ -1146,6 +1180,11 @@ const FILES: Directory = { (): any; } export declare var Injectable: InjectableFactory; + export interface InjectFactory { + (binding?: any): any; + new (binding?: any): any; + } + export declare var Inject: InjectFactory; export interface OnInit { ngOnInit(): any; }