From b6f6b1178f06442d0bf3271fed2380bdebc8a94f Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 6 Mar 2019 16:35:08 -0800 Subject: [PATCH] fix(ivy): generate type references to a default import (#29146) This commit refactors and expands ngtsc's support for generating imports of values from imports of types (this is used for example when importing a class referenced in a type annotation in a constructor). Previously, this logic handled "import {Foo} from" and "import * as foo from" style imports, but failed on imports of default values ("import Foo from"). This commit moves the type-to-value logic to a separate file and expands it to cover the default import case. Doing this also required augmenting the ImportManager to track default as well as non-default import generation. The APIs were made a little cleaner at the same time. PR Close #29146 --- .../src/ngcc/src/rendering/esm_renderer.ts | 12 +- .../src/ngcc/src/rendering/renderer.ts | 4 +- .../test/rendering/esm2015_renderer_spec.ts | 15 +- .../ngcc/test/rendering/esm5_renderer_spec.ts | 15 +- .../src/ngcc/test/rendering/renderer_spec.ts | 15 +- .../src/ngtsc/reflection/index.ts | 3 +- .../src/ngtsc/reflection/src/type_to_value.ts | 185 ++++++++++++++++++ .../src/ngtsc/reflection/src/typescript.ts | 128 +----------- .../src/ngtsc/reflection/test/ts_host_spec.ts | 27 +++ .../src/ngtsc/transform/src/utils.ts | 18 +- .../src/ngtsc/translator/BUILD.bazel | 1 + .../src/ngtsc/translator/src/translator.ts | 46 +++-- .../src/ngtsc/typecheck/src/context.ts | 8 +- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 28 +++ 14 files changed, 349 insertions(+), 156 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts diff --git a/packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts b/packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts index 9bb8165a11..580ce15b99 100644 --- a/packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts +++ b/packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts @@ -23,9 +23,17 @@ export class EsmRenderer extends Renderer { /** * Add the imports at the top of the file */ - addImports(output: MagicString, imports: {name: string; as: string;}[]): void { + addImports(output: MagicString, imports: { + specifier: string; qualifier: string; isDefault: boolean + }[]): void { // The imports get inserted at the very top of the file. - imports.forEach(i => { output.appendLeft(0, `import * as ${i.as} from '${i.name}';\n`); }); + imports.forEach(i => { + if (!i.isDefault) { + output.appendLeft(0, `import * as ${i.qualifier} from '${i.specifier}';\n`); + } else { + output.appendLeft(0, `import ${i.qualifier} from '${i.specifier}';\n`); + } + }); } addExports(output: MagicString, entryPointBasePath: string, exports: { diff --git a/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts index e310b6172b..00fc737b3f 100644 --- a/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts @@ -245,7 +245,9 @@ export abstract class Renderer { protected abstract addConstants(output: MagicString, constants: string, file: ts.SourceFile): void; - protected abstract addImports(output: MagicString, imports: {name: string, as: string}[]): void; + protected abstract addImports( + output: MagicString, + imports: {specifier: string, qualifier: string, isDefault: boolean}[]): void; protected abstract addExports(output: MagicString, entryPointBasePath: string, exports: { identifier: string, from: string diff --git a/packages/compiler-cli/src/ngcc/test/rendering/esm2015_renderer_spec.ts b/packages/compiler-cli/src/ngcc/test/rendering/esm2015_renderer_spec.ts index a45ad7e28f..fd6419e8cc 100644 --- a/packages/compiler-cli/src/ngcc/test/rendering/esm2015_renderer_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/rendering/esm2015_renderer_spec.ts @@ -115,13 +115,24 @@ describe('Esm2015Renderer', () => { it('should insert the given imports at the start of the source file', () => { const {renderer} = setup(PROGRAM); const output = new MagicString(PROGRAM.contents); - renderer.addImports( - output, [{name: '@angular/core', as: 'i0'}, {name: '@angular/common', as: 'i1'}]); + renderer.addImports(output, [ + {specifier: '@angular/core', qualifier: 'i0', isDefault: false}, + {specifier: '@angular/common', qualifier: 'i1', isDefault: false} + ]); expect(output.toString()).toContain(`import * as i0 from '@angular/core'; import * as i1 from '@angular/common'; /* A copyright notice */`); }); + + it('should insert a default import at the start of the source file', () => { + const {renderer} = setup(PROGRAM); + const output = new MagicString(PROGRAM.contents); + renderer.addImports(output, [ + {specifier: 'test', qualifier: 'i0', isDefault: true}, + ]); + expect(output.toString()).toContain(`import i0 from 'test';`); + }); }); describe('addExports', () => { diff --git a/packages/compiler-cli/src/ngcc/test/rendering/esm5_renderer_spec.ts b/packages/compiler-cli/src/ngcc/test/rendering/esm5_renderer_spec.ts index fe8bf2aaa3..23f21f3d94 100644 --- a/packages/compiler-cli/src/ngcc/test/rendering/esm5_renderer_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/rendering/esm5_renderer_spec.ts @@ -152,13 +152,24 @@ describe('Esm5Renderer', () => { it('should insert the given imports at the start of the source file', () => { const {renderer} = setup(PROGRAM); const output = new MagicString(PROGRAM.contents); - renderer.addImports( - output, [{name: '@angular/core', as: 'i0'}, {name: '@angular/common', as: 'i1'}]); + renderer.addImports(output, [ + {specifier: '@angular/core', qualifier: 'i0', isDefault: false}, + {specifier: '@angular/common', qualifier: 'i1', isDefault: false} + ]); expect(output.toString()).toContain(`import * as i0 from '@angular/core'; import * as i1 from '@angular/common'; /* A copyright notice */`); }); + + it('should insert a default import at the start of the source file', () => { + const {renderer} = setup(PROGRAM); + const output = new MagicString(PROGRAM.contents); + renderer.addImports(output, [ + {specifier: 'test', qualifier: 'i0', isDefault: true}, + ]); + expect(output.toString()).toContain(`import i0 from 'test';`); + }); }); describe('addExports', () => { diff --git a/packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts b/packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts index 0c582c52f1..dc2f48f679 100644 --- a/packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts @@ -23,7 +23,8 @@ class TestRenderer extends Renderer { constructor(host: Esm2015ReflectionHost, isCore: boolean, bundle: EntryPointBundle) { super(host, isCore, bundle, '/src', '/dist'); } - addImports(output: MagicString, imports: {name: string, as: string}[]) { + addImports( + output: MagicString, imports: {specifier: string, qualifier: string, isDefault: boolean}[]) { output.prepend('\n// ADD IMPORTS\n'); } addExports(output: MagicString, baseEntryPointPath: string, exports: { @@ -171,7 +172,7 @@ A.ngComponentDef = ɵngcc0.ɵdefineComponent({ type: A, selectors: [["a"]], fact const addImportsSpy = renderer.addImports as jasmine.Spy; expect(addImportsSpy.calls.first().args[0].toString()).toEqual(RENDERED_CONTENTS); expect(addImportsSpy.calls.first().args[1]).toEqual([ - {name: '@angular/core', as: 'ɵngcc0'} + {specifier: '@angular/core', qualifier: 'ɵngcc0', isDefault: false} ]); }); @@ -287,7 +288,9 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", "" expect(addDefinitionsSpy.calls.first().args[2]) .toContain(`/*@__PURE__*/ ɵngcc0.setClassMetadata(`); const addImportsSpy = renderer.addImports as jasmine.Spy; - expect(addImportsSpy.calls.first().args[1]).toEqual([{name: './r3_symbols', as: 'ɵngcc0'}]); + expect(addImportsSpy.calls.first().args[1]).toEqual([ + {specifier: './r3_symbols', qualifier: 'ɵngcc0', isDefault: false} + ]); }); it('should render no imports in FESM bundles', () => { @@ -502,9 +505,9 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", "" export declare function withProviders8(): (MyModuleWithProviders)&{ngModule:SomeModule};`); expect(renderer.addImports).toHaveBeenCalledWith(jasmine.any(MagicString), [ - {name: './module', as: 'ɵngcc0'}, - {name: '@angular/core', as: 'ɵngcc1'}, - {name: 'some-library', as: 'ɵngcc2'}, + {specifier: './module', qualifier: 'ɵngcc0', isDefault: false}, + {specifier: '@angular/core', qualifier: 'ɵngcc1', isDefault: false}, + {specifier: 'some-library', qualifier: 'ɵngcc2', isDefault: false}, ]); diff --git a/packages/compiler-cli/src/ngtsc/reflection/index.ts b/packages/compiler-cli/src/ngtsc/reflection/index.ts index d5615ed5ac..75186f70d8 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/index.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/index.ts @@ -7,4 +7,5 @@ */ export * from './src/host'; -export {TypeScriptReflectionHost, filterToMembersWithDecorator, reflectIdentifierOfDeclaration, reflectNameOfDeclaration, reflectObjectLiteral, reflectTypeEntityToDeclaration, typeNodeToValueExpr} from './src/typescript'; \ No newline at end of file +export {DEFAULT_EXPORT_NAME, typeNodeToValueExpr} from './src/type_to_value'; +export {TypeScriptReflectionHost, filterToMembersWithDecorator, reflectIdentifierOfDeclaration, reflectNameOfDeclaration, reflectObjectLiteral, reflectTypeEntityToDeclaration} from './src/typescript'; \ No newline at end of file diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts b/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts new file mode 100644 index 0000000000..f8ffff8aad --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts @@ -0,0 +1,185 @@ +/** + * @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 * as ts from 'typescript'; + +import {TypeValueReference} from './host'; + +export const DEFAULT_EXPORT_NAME = '*'; + +/** + * Potentially convert a `ts.TypeNode` to a `TypeValueReference`, which indicates how to use the + * type given in the `ts.TypeNode` in a value position. + * + * This can return `null` if the `typeNode` is `null`, if it does not refer to a symbol with a value + * declaration, or if it is not possible to statically understand. + */ +export function typeToValue( + typeNode: ts.TypeNode | null, checker: ts.TypeChecker): TypeValueReference|null { + // It's not possible to get a value expression if the parameter doesn't even have a type. + if (typeNode === null || !ts.isTypeReferenceNode(typeNode)) { + return null; + } + + const symbols = resolveTypeSymbols(typeNode, checker); + if (symbols === null) { + return null; + } + + const {local, decl} = symbols; + // It's only valid to convert a type reference to a value reference if the type actually + // has a value declaration associated with it. + if (decl.valueDeclaration === undefined) { + return null; + } + + // 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. + + // Look at the local `ts.Symbol`'s declarations and see if it comes from an import + // statement. If so, extract the module specifier and the name of the imported type. + const firstDecl = local.declarations && local.declarations[0]; + + if (firstDecl && isImportSource(firstDecl)) { + const origin = extractModuleAndNameFromImport(firstDecl, symbols.importName); + return {local: false, valueDeclaration: decl.valueDeclaration, ...origin}; + } else { + const expression = typeNodeToValueExpr(typeNode); + if (expression !== null) { + return { + local: true, + expression, + }; + } else { + return null; + } + } +} + +/** + * Attempt to extract a `ts.Expression` that's equivalent to a `ts.TypeNode`, as the two have + * different AST shapes but can reference the same symbols. + * + * This will return `null` if an equivalent expression cannot be constructed. + */ +export function typeNodeToValueExpr(node: ts.TypeNode): ts.Expression|null { + if (ts.isTypeReferenceNode(node)) { + return entityNameToValue(node.typeName); + } else { + return null; + } +} + +/** + * Resolve a `TypeReference` node to the `ts.Symbol`s for both its declaration and its local source. + * + * In the event that the `TypeReference` refers to a locally declared symbol, these will be the + * same. If the `TypeReference` refers to an imported symbol, then `decl` will be the fully resolved + * `ts.Symbol` of the referenced symbol. `local` will be the `ts.Symbol` of the `ts.Identifer` which + * points to the import statement by which the symbol was imported. + * + * In the event `typeRef` refers to a default import, an `importName` will also be returned to + * give the identifier name within the current file by which the import is known. + */ +function resolveTypeSymbols(typeRef: ts.TypeReferenceNode, checker: ts.TypeChecker): + {local: ts.Symbol, decl: ts.Symbol, importName: string | null}|null { + const typeName = typeRef.typeName; + // typeRefSymbol is the ts.Symbol of the entire type reference. + const typeRefSymbol: ts.Symbol|undefined = checker.getSymbolAtLocation(typeName); + if (typeRefSymbol === undefined) { + return null; + } + + // local is the ts.Symbol for the local ts.Identifier for the type. + // If the type is actually locally declared or is imported by name, for example: + // import {Foo} from './foo'; + // then it'll be the same as top. If the type is imported via a namespace import, for example: + // import * as foo from './foo'; + // and then referenced as: + // constructor(f: foo.Foo) + // then local will be the ts.Symbol of `foo`, whereas top will be the ts.Symbol of `foo.Foo`. + // This allows tracking of the import behind whatever type reference exists. + let local = typeRefSymbol; + let importName: string|null = null; + + // TODO(alxhub): this is technically not correct. The user could have any import type with any + // amount of qualification following the imported type: + // + // import * as foo from 'foo' + // constructor(inject: foo.X.Y.Z) + // + // What we really want is the ability to express the arbitrary operation of `.X.Y.Z` on top of + // whatever import we generate for 'foo'. This logic is sufficient for now, though. + if (ts.isQualifiedName(typeName) && ts.isIdentifier(typeName.left) && + ts.isIdentifier(typeName.right)) { + const localTmp = checker.getSymbolAtLocation(typeName.left); + if (localTmp !== undefined) { + local = localTmp; + importName = typeName.right.text; + } + } + + // De-alias the top-level type reference symbol to get the symbol of the actual declaration. + let decl = typeRefSymbol; + if (typeRefSymbol.flags & ts.SymbolFlags.Alias) { + decl = checker.getAliasedSymbol(typeRefSymbol); + } + return {local, decl, importName}; +} + +function entityNameToValue(node: ts.EntityName): ts.Expression|null { + if (ts.isQualifiedName(node)) { + const left = entityNameToValue(node.left); + return left !== null ? ts.createPropertyAccess(left, node.right) : null; + } else if (ts.isIdentifier(node)) { + return ts.getMutableClone(node); + } else { + return null; + } +} + +function isImportSource(node: ts.Declaration): node is( + ts.ImportSpecifier | ts.NamespaceImport | ts.ImportClause) { + return ts.isImportSpecifier(node) || ts.isNamespaceImport(node) || ts.isImportClause(node); +} + +function extractModuleAndNameFromImport( + node: ts.ImportSpecifier | ts.NamespaceImport | ts.ImportClause, + localName: string | null): {name: string, moduleName: string} { + let name: string; + let moduleSpecifier: ts.Expression; + switch (node.kind) { + case ts.SyntaxKind.ImportSpecifier: + // The symbol was imported by name, in a ts.ImportSpecifier. + name = (node.propertyName || node.name).text; + moduleSpecifier = node.parent.parent.parent.moduleSpecifier; + break; + case ts.SyntaxKind.NamespaceImport: + // The symbol was imported via a namespace import. In this case, the name to use when + // importing it was extracted by resolveTypeSymbols. + if (localName === null) { + // resolveTypeSymbols() should have extracted the correct local name for the import. + throw new Error(`Debug failure: no local name provided for NamespaceImport`); + } + name = localName; + moduleSpecifier = node.parent.parent.moduleSpecifier; + break; + case ts.SyntaxKind.ImportClause: + name = DEFAULT_EXPORT_NAME; + moduleSpecifier = node.parent.moduleSpecifier; + break; + default: + throw new Error(`Unreachable: ${ts.SyntaxKind[(node as ts.Node).kind]}`); + } + + if (!ts.isStringLiteral(moduleSpecifier)) { + throw new Error('not a module specifier'); + } + const moduleName = moduleSpecifier.text; + return {moduleName, name}; +} diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts index 04aa900f06..f06b084a8e 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts @@ -9,6 +9,7 @@ import * as ts from 'typescript'; import {ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, FunctionDefinition, Import, ReflectionHost, TypeValueReference} from './host'; +import {typeToValue} from './type_to_value'; /** * reflector.ts implements static reflection of declarations using the TypeScript `ts.TypeChecker`. @@ -48,7 +49,7 @@ 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: TypeValueReference|null = null; + let originalTypeNode = node.type || null; let typeNode = originalTypeNode; @@ -67,68 +68,11 @@ export class TypeScriptReflectionHost implements ReflectionHost { } } - // It's not possible to get a value expression if the parameter doesn't even have a type. - if (typeNode && ts.isTypeReferenceNode(typeNode)) { - const symbols = resolveTypeSymbols(typeNode, this.checker); - if (symbols !== null) { - const {local, decl} = symbols; - // It's only valid to convert a type reference to a value reference if the type actually - // has a value declaration associated with it. - if (decl.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. - - // Look at the local `ts.Symbol`'s declarations and see if it comes from an import - // statement. If so, extract the module specifier and the name of the imported type. - const firstDecl = local.declarations && local.declarations[0]; - - if (firstDecl && ts.isImportSpecifier(firstDecl)) { - // The symbol was imported by name, in a ts.ImportSpecifier. - const name = (firstDecl.propertyName || firstDecl.name).text; - const moduleSpecifier = firstDecl.parent.parent.parent.moduleSpecifier; - if (!ts.isStringLiteral(moduleSpecifier)) { - throw new Error('not a module specifier'); - } - const moduleName = moduleSpecifier.text; - typeValueExpr = { - local: false, - name, - moduleName, - valueDeclaration: decl.valueDeclaration, - }; - } else if ( - firstDecl && ts.isNamespaceImport(firstDecl) && symbols.importName !== null) { - // The symbol was imported via a namespace import. In this case, the name to use when - // importing it was extracted by resolveTypeSymbols. - const name = symbols.importName; - const moduleSpecifier = firstDecl.parent.parent.moduleSpecifier; - if (!ts.isStringLiteral(moduleSpecifier)) { - throw new Error('not a module specifier'); - } - const moduleName = moduleSpecifier.text; - typeValueExpr = { - local: false, - name, - moduleName, - valueDeclaration: decl.valueDeclaration, - }; - } else { - const expression = typeNodeToValueExpr(typeNode); - if (expression !== null) { - typeValueExpr = { - local: true, - expression, - }; - } - } - } - } - } + const typeValueReference = typeToValue(typeNode, this.checker); return { name, - nameNode: node.name, - typeValueReference: typeValueExpr, + nameNode: node.name, typeValueReference, typeNode: originalTypeNode, decorators, }; }); @@ -490,25 +434,6 @@ function parameterName(name: ts.BindingName): string|null { } } -export function typeNodeToValueExpr(node: ts.TypeNode): ts.Expression|null { - if (ts.isTypeReferenceNode(node)) { - return entityNameToValue(node.typeName); - } else { - return null; - } -} - -function entityNameToValue(node: ts.EntityName): ts.Expression|null { - if (ts.isQualifiedName(node)) { - const left = entityNameToValue(node.left); - return left !== null ? ts.createPropertyAccess(left, node.right) : null; - } else if (ts.isIdentifier(node)) { - return ts.getMutableClone(node); - } else { - return null; - } -} - function propertyNameToString(node: ts.PropertyName): string|null { if (ts.isIdentifier(node) || ts.isStringLiteral(node) || ts.isNumericLiteral(node)) { return node.text; @@ -516,48 +441,3 @@ function propertyNameToString(node: ts.PropertyName): string|null { return null; } } - -/** - * Resolve a `TypeReference` node to the `ts.Symbol`s for both its declaration and its local source. - * - * In the event that the `TypeReference` refers to a locally declared symbol, these will be the - * same. If the `TypeReference` refers to an imported symbol, then `decl` will be the fully resolved - * `ts.Symbol` of the referenced symbol. `local` will be the `ts.Symbol` of the `ts.Identifer` which - * points to the import statement by which the symbol was imported. - */ -function resolveTypeSymbols(typeRef: ts.TypeReferenceNode, checker: ts.TypeChecker): - {local: ts.Symbol, decl: ts.Symbol, importName: string | null}|null { - const typeName = typeRef.typeName; - // typeRefSymbol is the ts.Symbol of the entire type reference. - const typeRefSymbol: ts.Symbol|undefined = checker.getSymbolAtLocation(typeName); - if (typeRefSymbol === undefined) { - return null; - } - - // local is the ts.Symbol for the local ts.Identifier for the type. - // If the type is actually locally declared or is imported by name, for example: - // import {Foo} from './foo'; - // then it'll be the same as top. If the type is imported via a namespace import, for example: - // import * as foo from './foo'; - // and then referenced as: - // constructor(f: foo.Foo) - // then local will be the ts.Symbol of `foo`, whereas top will be the ts.Symbol of `foo.Foo`. - // This allows tracking of the import behind whatever type reference exists. - let local = typeRefSymbol; - let importName: string|null = null; - if (ts.isQualifiedName(typeName) && ts.isIdentifier(typeName.left) && - ts.isIdentifier(typeName.right)) { - const localTmp = checker.getSymbolAtLocation(typeName.left); - if (localTmp !== undefined) { - local = localTmp; - importName = typeName.right.text; - } - } - - // De-alias the top-level type reference symbol to get the symbol of the actual declaration. - let decl = typeRefSymbol; - if (typeRefSymbol.flags & ts.SymbolFlags.Alias) { - decl = checker.getAliasedSymbol(typeRefSymbol); - } - return {local, decl, importName}; -} diff --git a/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts b/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts index 2425512d61..0ad0c20598 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts @@ -147,6 +147,33 @@ describe('reflector', () => { expectParameter(args[0], 'bar', {moduleName: './bar', name: 'Bar'}); }); + it('should reflect an argument from a default import', () => { + const {program} = makeProgram([ + { + name: 'bar.ts', + contents: ` + export default class Bar {} + ` + }, + { + name: 'entry.ts', + contents: ` + import Bar from './bar'; + + class Foo { + constructor(bar: Bar) {} + } + ` + } + ]); + 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', {moduleName: './bar', name: '*'}); + }); + it('should reflect a nullable argument', () => { const {program} = makeProgram([ { diff --git a/packages/compiler-cli/src/ngtsc/transform/src/utils.ts b/packages/compiler-cli/src/ngtsc/transform/src/utils.ts index 9e6df27e95..513d263cd6 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/utils.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/utils.ts @@ -19,10 +19,22 @@ export function addImports( extraStatements: ts.Statement[] = []): ts.SourceFile { // Generate the import statements to prepend. const addedImports = importManager.getAllImports(sf.fileName).map(i => { + const qualifier = ts.createIdentifier(i.qualifier); + let importClause: ts.ImportClause; + if (!i.isDefault) { + importClause = ts.createImportClause( + /* name */ undefined, + /* namedBindings */ ts.createNamespaceImport(qualifier)); + } else { + importClause = ts.createImportClause( + /* name */ qualifier, + /* namedBindings */ undefined); + } return ts.createImportDeclaration( - undefined, undefined, - ts.createImportClause(undefined, ts.createNamespaceImport(ts.createIdentifier(i.as))), - ts.createLiteral(i.name)); + /* decorators */ undefined, + /* modifiers */ undefined, + /* importClause */ importClause, + /* moduleSpecifier */ ts.createLiteral(i.specifier)); }); // Filter out the existing imports and the source file body. All new statements diff --git a/packages/compiler-cli/src/ngtsc/translator/BUILD.bazel b/packages/compiler-cli/src/ngtsc/translator/BUILD.bazel index 351b1a7513..966e62bcbb 100644 --- a/packages/compiler-cli/src/ngtsc/translator/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/translator/BUILD.bazel @@ -9,6 +9,7 @@ ts_library( "//packages:types", "//packages/compiler", "//packages/compiler-cli/src/ngtsc/imports", + "//packages/compiler-cli/src/ngtsc/reflection", "//packages/compiler-cli/src/ngtsc/util", "@npm//typescript", ], diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index 5fd405ab35..0ffd59cc20 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -10,6 +10,7 @@ import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinTyp import * as ts from 'typescript'; import {ImportRewriter, NoopImportRewriter} from '../../imports'; +import {DEFAULT_EXPORT_NAME} from '../../reflection'; export class Context { constructor(readonly isStatement: boolean) {} @@ -38,11 +39,9 @@ const BINARY_OPERATORS = new Map([ [BinaryOperator.Plus, ts.SyntaxKind.PlusToken], ]); - - export class ImportManager { - private moduleToIndex = new Map(); - private importedModules = new Set(); + private nonDefaultImports = new Map(); + private defaultImports = new Map(); private nextIndex = 0; constructor(protected rewriter: ImportRewriter = new NoopImportRewriter(), private prefix = 'i') { @@ -61,20 +60,39 @@ export class ImportManager { } // If not, this symbol will be imported. Allocate a prefix for the imported module if needed. - if (!this.moduleToIndex.has(moduleName)) { - this.moduleToIndex.set(moduleName, `${this.prefix}${this.nextIndex++}`); - } - const moduleImport = this.moduleToIndex.get(moduleName) !; - return {moduleImport, symbol}; + const isDefault = symbol === DEFAULT_EXPORT_NAME; + + // Use a different map for non-default vs default imports. This allows the same module to be + // imported in both ways simultaneously. + const trackingMap = !isDefault ? this.nonDefaultImports : this.defaultImports; + + if (!trackingMap.has(moduleName)) { + trackingMap.set(moduleName, `${this.prefix}${this.nextIndex++}`); + } + const moduleImport = trackingMap.get(moduleName) !; + + if (isDefault) { + // For an import of a module's default symbol, the moduleImport *is* the name to use to refer + // to the import. + return {moduleImport: null, symbol: moduleImport}; + } else { + // Non-default imports have a qualifier and the symbol name to import. + return {moduleImport, symbol}; + } } - getAllImports(contextPath: string): {name: string, as: string}[] { - return Array.from(this.moduleToIndex.keys()).map(name => { - const as = this.moduleToIndex.get(name) !; - name = this.rewriter.rewriteSpecifier(name, contextPath); - return {name, as}; + getAllImports(contextPath: string): {specifier: string, qualifier: string, isDefault: boolean}[] { + const imports: {specifier: string, qualifier: string, isDefault: boolean}[] = []; + this.nonDefaultImports.forEach((qualifier, specifier) => { + specifier = this.rewriter.rewriteSpecifier(specifier, contextPath); + imports.push({specifier, qualifier, isDefault: false}); }); + this.defaultImports.forEach((qualifier, specifier) => { + specifier = this.rewriter.rewriteSpecifier(specifier, contextPath); + imports.push({specifier, qualifier, isDefault: true}); + }); + return imports; } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index b9c7ab09d3..02f160e036 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -135,7 +135,13 @@ export class TypeCheckContext { // Write out the imports that need to be added to the beginning of the file. let imports = importManager.getAllImports(sf.fileName) - .map(i => `import * as ${i.as} from '${i.name}';`) + .map(i => { + if (!i.isDefault) { + return `import * as ${i.qualifier} from '${i.specifier}';`; + } else { + return `import ${i.qualifier} from '${i.specifier}';`; + } + }) .join('\n'); code = imports + '\n' + code; diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 6aaf47e99e..f3545e0abc 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -1970,6 +1970,34 @@ describe('ngtsc behavioral tests', () => { expect(jsContents).toMatch(setClassMetadataRegExp('type: i\\d\\.MyTypeB')); }); + it('should use default-imported types if they can be represented as values', () => { + env.tsconfig({}); + + env.write(`types.ts`, ` + export default class Default {} + export class Other {} + `); + env.write(`test.ts`, ` + import {Component} from '@angular/core'; + import {Other} from './types'; + import Default from './types'; + + @Component({selector: 'test', template: 'test'}) + export class SomeCmp { + constructor(arg: Default, other: Other) {} + } + `); + + env.driveMain(); + const jsContents = trim(env.getContents('test.js')); + expect(jsContents).toContain(`import i1 from "./types";`); + expect(jsContents).toContain(`import * as i2 from "./types";`); + expect(jsContents).toContain('i0.ɵdirectiveInject(i1)'); + expect(jsContents).toContain('i0.ɵdirectiveInject(i2.Other)'); + expect(jsContents).toMatch(setClassMetadataRegExp('type: i1')); + expect(jsContents).toMatch(setClassMetadataRegExp('type: i2.Other')); + }); + it('should use `undefined` in setClassMetadata if types can\'t be represented as values', () => { env.tsconfig({});