diff --git a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts index 5d50f1d025..4d1c7ff90e 100644 --- a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts @@ -12,7 +12,7 @@ import * as ts from 'typescript'; import {BaseDefDecoratorHandler, ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader} from '../../../ngtsc/annotations'; import {CycleAnalyzer, ImportGraph} from '../../../ngtsc/cycles'; -import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, ReferenceEmitter} from '../../../ngtsc/imports'; +import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../../ngtsc/imports'; import {PartialEvaluator} from '../../../ngtsc/partial_evaluator'; import {AbsoluteFsPath, LogicalFileSystem} from '../../../ngtsc/path'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../ngtsc/scope'; @@ -85,14 +85,18 @@ export class DecorationAnalyzer { new ComponentDecoratorHandler( this.reflectionHost, this.evaluator, this.scopeRegistry, this.isCore, this.resourceManager, this.rootDirs, /* defaultPreserveWhitespaces */ false, /* i18nUseExternalIds */ true, - this.moduleResolver, this.cycleAnalyzer, this.refEmitter), + this.moduleResolver, this.cycleAnalyzer, this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER), new DirectiveDecoratorHandler( - this.reflectionHost, this.evaluator, this.scopeRegistry, this.isCore), - new InjectableDecoratorHandler(this.reflectionHost, this.isCore, /* strictCtorDeps */ false), + this.reflectionHost, this.evaluator, this.scopeRegistry, NOOP_DEFAULT_IMPORT_RECORDER, + this.isCore), + new InjectableDecoratorHandler( + this.reflectionHost, NOOP_DEFAULT_IMPORT_RECORDER, this.isCore, /* strictCtorDeps */ false), new NgModuleDecoratorHandler( this.reflectionHost, this.evaluator, this.scopeRegistry, this.referencesRegistry, - this.isCore, /* routeAnalyzer */ null, this.refEmitter), - new PipeDecoratorHandler(this.reflectionHost, this.evaluator, this.scopeRegistry, this.isCore), + this.isCore, /* routeAnalyzer */ null, this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER), + new PipeDecoratorHandler( + this.reflectionHost, this.evaluator, this.scopeRegistry, NOOP_DEFAULT_IMPORT_RECORDER, + this.isCore), ]; constructor( diff --git a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts index 9b387129b0..408064d9d9 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts @@ -901,8 +901,9 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N return { name: getNameText(nameNode), nameNode, - typeValueReference: - typeExpression !== null ? {local: true as true, expression: typeExpression} : null, + typeValueReference: typeExpression !== null ? + {local: true as true, expression: typeExpression, defaultImportStatement: null} : + null, typeNode: null, decorators }; }); 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 7f8360ad93..b3736f38b6 100644 --- a/packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts +++ b/packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts @@ -25,17 +25,10 @@ export class EsmRenderer extends Renderer { /** * Add the imports at the top of the file */ - addImports(output: MagicString, imports: { - specifier: string; qualifier: string; isDefault: boolean - }[]): void { + addImports(output: MagicString, imports: {specifier: string; qualifier: string;}[]): void { // The imports get inserted at the very top of the file. - 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`); - } - }); + imports.forEach( + i => { output.appendLeft(0, `import * as ${i.qualifier} from '${i.specifier}';\n`); }); } addExports(output: MagicString, entryPointBasePath: string, exports: ExportInfo[]): void { diff --git a/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts index 70bd3fc70b..ec535adf26 100644 --- a/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts @@ -13,7 +13,7 @@ import {basename, dirname, relative, resolve} from 'canonical-path'; import {SourceMapConsumer, SourceMapGenerator, RawSourceMap} from 'source-map'; import * as ts from 'typescript'; -import {NoopImportRewriter, ImportRewriter, R3SymbolsImportRewriter} from '@angular/compiler-cli/src/ngtsc/imports'; +import {NoopImportRewriter, ImportRewriter, R3SymbolsImportRewriter, NOOP_DEFAULT_IMPORT_RECORDER} from '@angular/compiler-cli/src/ngtsc/imports'; import {CompileResult} from '@angular/compiler-cli/src/ngtsc/transform'; import {translateStatement, translateType, ImportManager} from '../../../ngtsc/translator'; import {NgccFlatImportRewriter} from './ngcc_import_rewriter'; @@ -245,9 +245,8 @@ export abstract class Renderer { protected abstract addConstants(output: MagicString, constants: string, file: ts.SourceFile): void; - protected abstract addImports( - output: MagicString, - imports: {specifier: string, qualifier: string, isDefault: boolean}[]): void; + protected abstract addImports(output: MagicString, imports: {specifier: string, + qualifier: string}[]): void; protected abstract addExports( output: MagicString, entryPointBasePath: string, exports: ExportInfo[]): void; protected abstract addDefinitions( @@ -464,7 +463,8 @@ export function mergeSourceMaps( export function renderConstantPool( sourceFile: ts.SourceFile, constantPool: ConstantPool, imports: ImportManager): string { const printer = ts.createPrinter(); - return constantPool.statements.map(stmt => translateStatement(stmt, imports)) + return constantPool.statements + .map(stmt => translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER)) .map(stmt => printer.printNode(ts.EmitHint.Unspecified, stmt, sourceFile)) .join('\n'); } @@ -481,12 +481,13 @@ export function renderDefinitions( sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager): string { const printer = ts.createPrinter(); const name = (compiledClass.declaration as ts.NamedDeclaration).name !; + const translate = (stmt: Statement) => + translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER); const definitions = compiledClass.compilation .map( - c => c.statements.map(statement => translateStatement(statement, imports)) - .concat(translateStatement( - createAssignmentStatement(name, c.name, c.initializer), imports)) + c => c.statements.map(statement => translate(statement)) + .concat(translate(createAssignmentStatement(name, c.name, c.initializer))) .map( statement => printer.printNode(ts.EmitHint.Unspecified, statement, sourceFile)) diff --git a/packages/compiler-cli/src/ngcc/test/host/esm2015_host_import_helper_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm2015_host_import_helper_spec.ts index ee1bcba5ea..348d2d2efc 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm2015_host_import_helper_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm2015_host_import_helper_spec.ts @@ -302,7 +302,8 @@ describe('Fesm2015ReflectionHost [import helper style]', () => { const ctrDecorators = host.getConstructorParameters(classNode) !; const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference !as{ local: true, - expression: ts.Identifier + expression: ts.Identifier, + defaultImportStatement: null, }).expression; const expectedDeclarationNode = getDeclaration( diff --git a/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts index e595ec0582..850cde1841 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts @@ -1295,7 +1295,8 @@ describe('Fesm2015ReflectionHost', () => { const ctrDecorators = host.getConstructorParameters(classNode) !; const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference !as{ local: true, - expression: ts.Identifier + expression: ts.Identifier, + defaultImportStatement: null, }).expression; const expectedDeclarationNode = getDeclaration( diff --git a/packages/compiler-cli/src/ngcc/test/host/esm5_host_import_helper_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm5_host_import_helper_spec.ts index 6c558b1e48..ede5edc814 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm5_host_import_helper_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm5_host_import_helper_spec.ts @@ -338,7 +338,8 @@ describe('Esm5ReflectionHost [import helper style]', () => { const ctrDecorators = host.getConstructorParameters(classNode) !; const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference !as{ local: true, - expression: ts.Identifier + expression: ts.Identifier, + defaultImportStatement: null, }).expression; const expectedDeclarationNode = getDeclaration( diff --git a/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts index 0f93aa57e8..6010821969 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts @@ -1276,7 +1276,8 @@ describe('Esm5ReflectionHost', () => { const ctrDecorators = host.getConstructorParameters(classNode) !; const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference !as{ local: true, - expression: ts.Identifier + expression: ts.Identifier, + defaultImportStatement: null, }).expression; const expectedDeclarationNode = getDeclaration( 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 b5154418d8..f982cd09b6 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 @@ -116,23 +116,14 @@ describe('Esm2015Renderer', () => { const {renderer} = setup(PROGRAM); const output = new MagicString(PROGRAM.contents); renderer.addImports(output, [ - {specifier: '@angular/core', qualifier: 'i0', isDefault: false}, - {specifier: '@angular/common', qualifier: 'i1', isDefault: false} + {specifier: '@angular/core', qualifier: 'i0'}, + {specifier: '@angular/common', qualifier: 'i1'} ]); 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 62ecd76e8a..4d6704a5d0 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 @@ -153,23 +153,14 @@ describe('Esm5Renderer', () => { const {renderer} = setup(PROGRAM); const output = new MagicString(PROGRAM.contents); renderer.addImports(output, [ - {specifier: '@angular/core', qualifier: 'i0', isDefault: false}, - {specifier: '@angular/common', qualifier: 'i1', isDefault: false} + {specifier: '@angular/core', qualifier: 'i0'}, + {specifier: '@angular/common', qualifier: 'i1'} ]); 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 dc2f48f679..dd03dee87b 100644 --- a/packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts @@ -23,8 +23,7 @@ class TestRenderer extends Renderer { constructor(host: Esm2015ReflectionHost, isCore: boolean, bundle: EntryPointBundle) { super(host, isCore, bundle, '/src', '/dist'); } - addImports( - output: MagicString, imports: {specifier: string, qualifier: string, isDefault: boolean}[]) { + addImports(output: MagicString, imports: {specifier: string, qualifier: string}[]) { output.prepend('\n// ADD IMPORTS\n'); } addExports(output: MagicString, baseEntryPointPath: string, exports: { @@ -172,7 +171,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([ - {specifier: '@angular/core', qualifier: 'ɵngcc0', isDefault: false} + {specifier: '@angular/core', qualifier: 'ɵngcc0'} ]); }); @@ -289,7 +288,7 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", "" .toContain(`/*@__PURE__*/ ɵngcc0.setClassMetadata(`); const addImportsSpy = renderer.addImports as jasmine.Spy; expect(addImportsSpy.calls.first().args[1]).toEqual([ - {specifier: './r3_symbols', qualifier: 'ɵngcc0', isDefault: false} + {specifier: './r3_symbols', qualifier: 'ɵngcc0'} ]); }); @@ -505,9 +504,9 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", "" export declare function withProviders8(): (MyModuleWithProviders)&{ngModule:SomeModule};`); expect(renderer.addImports).toHaveBeenCalledWith(jasmine.any(MagicString), [ - {specifier: './module', qualifier: 'ɵngcc0', isDefault: false}, - {specifier: '@angular/core', qualifier: 'ɵngcc1', isDefault: false}, - {specifier: 'some-library', qualifier: 'ɵngcc2', isDefault: false}, + {specifier: './module', qualifier: 'ɵngcc0'}, + {specifier: '@angular/core', qualifier: 'ɵngcc1'}, + {specifier: 'some-library', qualifier: 'ɵngcc2'}, ]); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 9156e7890e..9197e756c5 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -12,7 +12,7 @@ import * as ts from 'typescript'; import {CycleAnalyzer} from '../../cycles'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; -import {ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; +import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; import {EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection'; import {LocalModuleScopeRegistry, ScopeDirective, extractDirectiveGuards} from '../../scope'; @@ -45,7 +45,7 @@ export class ComponentDecoratorHandler implements private resourceLoader: ResourceLoader, private rootDirs: string[], private defaultPreserveWhitespaces: boolean, private i18nUseExternalIds: boolean, private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer, - private refEmitter: ReferenceEmitter) {} + private refEmitter: ReferenceEmitter, private defaultImportRecorder: DefaultImportRecorder) {} private literalCache = new Map(); private boundTemplateCache = new Map>(); @@ -135,7 +135,7 @@ export class ComponentDecoratorHandler implements // @Component inherits @Directive, so begin by extracting the @Directive metadata and building // on it. const directiveResult = extractDirectiveMetadata( - node, decorator, this.reflector, this.evaluator, this.isCore, + node, decorator, this.reflector, this.evaluator, this.defaultImportRecorder, this.isCore, this.elementSchemaRegistry.getDefaultComponentElementName()); if (directiveResult === undefined) { // `extractDirectiveMetadata` returns undefined when the @Directive has `jit: true`. In this @@ -302,7 +302,8 @@ export class ComponentDecoratorHandler implements viewProviders, i18nUseExternalIds: this.i18nUseExternalIds, relativeContextFilePath }, - metadataStmt: generateSetClassMetadataCall(node, this.reflector, this.isCore), + metadataStmt: generateSetClassMetadataCall( + node, this.reflector, this.defaultImportRecorder, this.isCore), parsedTemplate: template.nodes, }, typeCheck: true, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index c53a39e492..8c3c231d59 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -10,7 +10,7 @@ import {ConstantPool, Expression, ParseError, ParsedHostBindings, R3DirectiveMet import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; -import {Reference} from '../../imports'; +import {DefaultImportRecorder, Reference} from '../../imports'; import {DynamicValue, EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {ClassMember, ClassMemberKind, Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection'; import {LocalModuleScopeRegistry} from '../../scope/src/local'; @@ -30,7 +30,8 @@ export class DirectiveDecoratorHandler implements DecoratorHandler { constructor( private reflector: ReflectionHost, private evaluator: PartialEvaluator, - private scopeRegistry: LocalModuleScopeRegistry, private isCore: boolean) {} + private scopeRegistry: LocalModuleScopeRegistry, + private defaultImportRecorder: DefaultImportRecorder, private isCore: boolean) {} readonly precedence = HandlerPrecedence.PRIMARY; @@ -50,8 +51,8 @@ export class DirectiveDecoratorHandler implements } analyze(node: ts.ClassDeclaration, decorator: Decorator): AnalysisOutput { - const directiveResult = - extractDirectiveMetadata(node, decorator, this.reflector, this.evaluator, this.isCore); + const directiveResult = extractDirectiveMetadata( + node, decorator, this.reflector, this.evaluator, this.defaultImportRecorder, this.isCore); const analysis = directiveResult && directiveResult.metadata; // If the directive has a selector, it should be registered with the `SelectorScopeRegistry` so @@ -77,7 +78,8 @@ export class DirectiveDecoratorHandler implements return { analysis: { meta: analysis, - metadataStmt: generateSetClassMetadataCall(node, this.reflector, this.isCore), + metadataStmt: generateSetClassMetadataCall( + node, this.reflector, this.defaultImportRecorder, this.isCore), } }; } @@ -103,7 +105,8 @@ export class DirectiveDecoratorHandler implements */ export function extractDirectiveMetadata( clazz: ts.ClassDeclaration, decorator: Decorator, reflector: ReflectionHost, - evaluator: PartialEvaluator, isCore: boolean, defaultSelector: string | null = null): { + evaluator: PartialEvaluator, defaultImportRecorder: DefaultImportRecorder, isCore: boolean, + defaultSelector: string | null = null): { decorator: Map, metadata: R3DirectiveMetadata, decoratedElements: ClassMember[], @@ -205,7 +208,7 @@ export function extractDirectiveMetadata( const usesInheritance = reflector.hasBaseClass(clazz); const metadata: R3DirectiveMetadata = { name: clazz.name !.text, - deps: getValidConstructorDependencies(clazz, reflector, isCore), host, + deps: getValidConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore), host, lifecycle: { usesOnChanges, }, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts index cc90389f81..b020c9316b 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts @@ -10,6 +10,7 @@ import {Expression, LiteralExpr, R3DependencyMetadata, R3InjectableMetadata, R3R import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; +import {DefaultImportRecorder} from '../../imports'; import {Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; @@ -27,8 +28,8 @@ export interface InjectableHandlerData { export class InjectableDecoratorHandler implements DecoratorHandler { constructor( - private reflector: ReflectionHost, private isCore: boolean, private strictCtorDeps: boolean) { - } + private reflector: ReflectionHost, private defaultImportRecorder: DefaultImportRecorder, + private isCore: boolean, private strictCtorDeps: boolean) {} readonly precedence = HandlerPrecedence.SHARED; @@ -51,8 +52,10 @@ export class InjectableDecoratorHandler implements return { analysis: { meta: extractInjectableMetadata( - node, decorator, this.reflector, this.isCore, this.strictCtorDeps), - metadataStmt: generateSetClassMetadataCall(node, this.reflector, this.isCore), + node, decorator, this.reflector, this.defaultImportRecorder, this.isCore, + this.strictCtorDeps), + metadataStmt: generateSetClassMetadataCall( + node, this.reflector, this.defaultImportRecorder, this.isCore), }, }; } @@ -78,7 +81,8 @@ export class InjectableDecoratorHandler implements * A `null` return value indicates this is @Injectable has invalid data. */ function extractInjectableMetadata( - clazz: ts.ClassDeclaration, decorator: Decorator, reflector: ReflectionHost, isCore: boolean, + clazz: ts.ClassDeclaration, decorator: Decorator, reflector: ReflectionHost, + defaultImportRecorder: DefaultImportRecorder, isCore: boolean, strictCtorDeps: boolean): R3InjectableMetadata { if (clazz.name === undefined) { throw new FatalDiagnosticError( @@ -101,9 +105,10 @@ function extractInjectableMetadata( // signature does not work for DI then an ngInjectableDef that throws. let ctorDeps: R3DependencyMetadata[]|'invalid'|null = null; if (strictCtorDeps) { - ctorDeps = getValidConstructorDependencies(clazz, reflector, isCore); + ctorDeps = getValidConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore); } else { - const possibleCtorDeps = getConstructorDependencies(clazz, reflector, isCore); + const possibleCtorDeps = + getConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore); if (possibleCtorDeps !== null) { if (possibleCtorDeps.deps !== null) { // This use of @Injectable has valid constructor dependencies. @@ -124,7 +129,7 @@ function extractInjectableMetadata( providedIn: new LiteralExpr(null), ctorDeps, }; } else if (decorator.args.length === 1) { - const rawCtorDeps = getConstructorDependencies(clazz, reflector, isCore); + const rawCtorDeps = getConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore); let ctorDeps: R3DependencyMetadata[]|'invalid'|null = null; // rawCtorDeps will be null if the class has no constructor. diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts b/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts index c807e9a14d..a976cd3848 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts @@ -9,10 +9,12 @@ import {Expression, ExternalExpr, FunctionExpr, Identifiers, InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, ReturnStatement, Statement, WrappedNodeExpr, literalMap} from '@angular/compiler'; import * as ts from 'typescript'; +import {DefaultImportRecorder} from '../../imports'; import {CtorParameter, Decorator, ReflectionHost} from '../../reflection'; import {valueReferenceToExpression} from './util'; + /** * Given a class declaration, generate a call to `setClassMetadata` with the Angular metadata * present on the class or its member fields. @@ -21,7 +23,8 @@ import {valueReferenceToExpression} from './util'; * as a `Statement` for inclusion along with the class. */ export function generateSetClassMetadataCall( - clazz: ts.Declaration, reflection: ReflectionHost, isCore: boolean): Statement|null { + clazz: ts.Declaration, reflection: ReflectionHost, defaultImportRecorder: DefaultImportRecorder, + isCore: boolean): Statement|null { if (!reflection.isClass(clazz) || clazz.name === undefined || !ts.isIdentifier(clazz.name)) { return null; } @@ -44,7 +47,8 @@ export function generateSetClassMetadataCall( let metaCtorParameters: Expression = new LiteralExpr(null); const classCtorParameters = reflection.getConstructorParameters(clazz); if (classCtorParameters !== null) { - const ctorParameters = classCtorParameters.map(param => ctorParameterToMetadata(param, isCore)); + const ctorParameters = classCtorParameters.map( + param => ctorParameterToMetadata(param, defaultImportRecorder, isCore)); metaCtorParameters = new FunctionExpr([], [ new ReturnStatement(new LiteralArrayExpr(ctorParameters)), ]); @@ -80,11 +84,13 @@ export function generateSetClassMetadataCall( /** * Convert a reflected constructor parameter to metadata. */ -function ctorParameterToMetadata(param: CtorParameter, isCore: boolean): Expression { +function ctorParameterToMetadata( + param: CtorParameter, defaultImportRecorder: DefaultImportRecorder, + isCore: boolean): Expression { // Parameters sometimes have a type that can be referenced. If so, then use it, otherwise // its type is undefined. const type = param.typeValueReference !== null ? - valueReferenceToExpression(param.typeValueReference) : + valueReferenceToExpression(param.typeValueReference, defaultImportRecorder) : new LiteralExpr(undefined); const mapEntries: {key: string, value: Expression, quoted: false}[] = [ diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index 78e3b66118..b609707f81 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -10,7 +10,7 @@ import {Expression, ExternalExpr, InvokeFunctionExpr, LiteralArrayExpr, R3Identi import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; -import {Reference, ReferenceEmitter} from '../../imports'; +import {DefaultImportRecorder, Reference, ReferenceEmitter} from '../../imports'; import {PartialEvaluator, ResolvedValue} from '../../partial_evaluator'; import {Decorator, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../reflection'; import {NgModuleRouteAnalyzer} from '../../routing'; @@ -39,7 +39,8 @@ export class NgModuleDecoratorHandler implements DecoratorHandler { constructor( private reflector: ReflectionHost, private evaluator: PartialEvaluator, - private scopeRegistry: LocalModuleScopeRegistry, private isCore: boolean) {} + private scopeRegistry: LocalModuleScopeRegistry, + private defaultImportRecorder: DefaultImportRecorder, private isCore: boolean) {} readonly precedence = HandlerPrecedence.PRIMARY; @@ -98,9 +99,12 @@ export class PipeDecoratorHandler implements DecoratorHandler { - let token = valueReferenceToExpression(param.typeValueReference); + let token = valueReferenceToExpression(param.typeValueReference, defaultImportRecorder); let optional = false, self = false, skipSelf = false, host = false; let resolved = R3ResolvedDependencyType.Token; (param.decorators || []).filter(dec => isCore || isAngularCore(dec)).forEach(dec => { @@ -101,13 +102,24 @@ export function getConstructorDependencies( * references are converted to an `ExternalExpr`. Note that this is only valid in the context of the * file in which the `TypeValueReference` originated. */ -export function valueReferenceToExpression(valueRef: TypeValueReference): Expression; -export function valueReferenceToExpression(valueRef: null): null; -export function valueReferenceToExpression(valueRef: TypeValueReference | null): Expression|null; -export function valueReferenceToExpression(valueRef: TypeValueReference | null): Expression|null { +export function valueReferenceToExpression( + valueRef: TypeValueReference, defaultImportRecorder: DefaultImportRecorder): Expression; +export function valueReferenceToExpression( + valueRef: null, defaultImportRecorder: DefaultImportRecorder): null; +export function valueReferenceToExpression( + valueRef: TypeValueReference | null, defaultImportRecorder: DefaultImportRecorder): Expression| + null; +export function valueReferenceToExpression( + valueRef: TypeValueReference | null, defaultImportRecorder: DefaultImportRecorder): Expression| + null { if (valueRef === null) { return null; } else if (valueRef.local) { + if (defaultImportRecorder !== null && valueRef.defaultImportStatement !== null && + ts.isIdentifier(valueRef.expression)) { + defaultImportRecorder.recordImportedIdentifier( + valueRef.expression, valueRef.defaultImportStatement); + } return new WrappedNodeExpr(valueRef.expression); } else { // TODO(alxhub): this cast is necessary because the g3 typescript version doesn't narrow here. @@ -116,10 +128,10 @@ export function valueReferenceToExpression(valueRef: TypeValueReference | null): } export function getValidConstructorDependencies( - clazz: ts.ClassDeclaration, reflector: ReflectionHost, isCore: boolean): R3DependencyMetadata[]| - null { + clazz: ts.ClassDeclaration, reflector: ReflectionHost, + defaultImportRecorder: DefaultImportRecorder, isCore: boolean): R3DependencyMetadata[]|null { return validateConstructorDependencies( - clazz, getConstructorDependencies(clazz, reflector, isCore)); + clazz, getConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore)); } export function validateConstructorDependencies( diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts index 03d268b612..a0cc4416be 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts @@ -10,7 +10,7 @@ import * as ts from 'typescript'; import {CycleAnalyzer, ImportGraph} from '../../cycles'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; -import {ModuleResolver, ReferenceEmitter} from '../../imports'; +import {ModuleResolver, NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../imports'; import {PartialEvaluator} from '../../partial_evaluator'; import {TypeScriptReflectionHost} from '../../reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope'; @@ -55,7 +55,7 @@ describe('ComponentDecoratorHandler', () => { const handler = new ComponentDecoratorHandler( reflectionHost, evaluator, scopeRegistry, false, new NoopResourceLoader(), [''], false, - true, moduleResolver, cycleAnalyzer, refEmitter); + true, moduleResolver, cycleAnalyzer, refEmitter, NOOP_DEFAULT_IMPORT_RECORDER); const TestCmp = getDeclaration(program, 'entry.ts', 'TestCmp', ts.isClassDeclaration); const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp)); if (detected === undefined) { diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts index 8dea509ab5..a314de85b6 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {ReferenceEmitter} from '../../imports'; +import {NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../imports'; import {PartialEvaluator} from '../../partial_evaluator'; import {TypeScriptReflectionHost} from '../../reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope'; @@ -43,7 +43,8 @@ describe('DirectiveDecoratorHandler', () => { const scopeRegistry = new LocalModuleScopeRegistry( new MetadataDtsModuleScopeResolver(checker, reflectionHost, null), new ReferenceEmitter([]), null); - const handler = new DirectiveDecoratorHandler(reflectionHost, evaluator, scopeRegistry, false); + const handler = new DirectiveDecoratorHandler( + reflectionHost, evaluator, scopeRegistry, NOOP_DEFAULT_IMPORT_RECORDER, false); const analyzeDirective = (dirName: string) => { const DirNode = getDeclaration(program, 'entry.ts', dirName, ts.isClassDeclaration); diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/metadata_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/metadata_spec.ts index 837c685fcb..ec24285097 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/metadata_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/metadata_spec.ts @@ -8,11 +8,10 @@ import * as ts from 'typescript'; -import {NoopImportRewriter} from '../../imports'; +import {NOOP_DEFAULT_IMPORT_RECORDER, NoopImportRewriter} from '../../imports'; import {TypeScriptReflectionHost} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript'; import {ImportManager, translateStatement} from '../../translator'; - import {generateSetClassMetadataCall} from '../src/metadata'; const CORE = { @@ -83,13 +82,13 @@ function compileAndPrint(contents: string): string { ]); const host = new TypeScriptReflectionHost(program.getTypeChecker()); const target = getDeclaration(program, 'index.ts', 'Target', ts.isClassDeclaration); - const call = generateSetClassMetadataCall(target, host, false); + const call = generateSetClassMetadataCall(target, host, NOOP_DEFAULT_IMPORT_RECORDER, false); if (call === null) { return ''; } const sf = program.getSourceFile('index.ts') !; const im = new ImportManager(new NoopImportRewriter(), 'i'); - const tsStatement = translateStatement(call, im); + const tsStatement = translateStatement(call, im, NOOP_DEFAULT_IMPORT_RECORDER); const res = ts.createPrinter().printNode(ts.EmitHint.Unspecified, tsStatement, sf); return res.replace(/\s+/g, ' '); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/ng_module_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/ng_module_spec.ts index 1464e81f45..ad94014bc0 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/ng_module_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/ng_module_spec.ts @@ -10,7 +10,7 @@ import {WrappedNodeExpr} from '@angular/compiler'; import {R3Reference} from '@angular/compiler/src/compiler'; import * as ts from 'typescript'; -import {LocalIdentifierStrategy, ReferenceEmitter} from '../../imports'; +import {LocalIdentifierStrategy, NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../imports'; import {PartialEvaluator} from '../../partial_evaluator'; import {TypeScriptReflectionHost} from '../../reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope'; @@ -61,7 +61,8 @@ describe('NgModuleDecoratorHandler', () => { const refEmitter = new ReferenceEmitter([new LocalIdentifierStrategy()]); const handler = new NgModuleDecoratorHandler( - reflectionHost, evaluator, scopeRegistry, referencesRegistry, false, null, refEmitter); + reflectionHost, evaluator, scopeRegistry, referencesRegistry, false, null, refEmitter, + NOOP_DEFAULT_IMPORT_RECORDER); const TestModule = getDeclaration(program, 'entry.ts', 'TestModule', ts.isClassDeclaration); const detected = handler.detect(TestModule, reflectionHost.getDecoratorsOfDeclaration(TestModule)); diff --git a/packages/compiler-cli/src/ngtsc/imports/index.ts b/packages/compiler-cli/src/ngtsc/imports/index.ts index ce541d85ae..aae8fd42c4 100644 --- a/packages/compiler-cli/src/ngtsc/imports/index.ts +++ b/packages/compiler-cli/src/ngtsc/imports/index.ts @@ -8,6 +8,7 @@ export {AliasGenerator, AliasStrategy} from './src/alias'; export {ImportRewriter, NoopImportRewriter, R3SymbolsImportRewriter, validateAndRewriteCoreSymbol} from './src/core'; +export {DefaultImportRecorder, DefaultImportTracker, NOOP_DEFAULT_IMPORT_RECORDER} from './src/default'; export {AbsoluteModuleStrategy, FileToModuleHost, FileToModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter} from './src/emitter'; export {Reexport} from './src/reexport'; export {ImportMode, OwningModule, Reference} from './src/references'; diff --git a/packages/compiler-cli/src/ngtsc/imports/src/default.ts b/packages/compiler-cli/src/ngtsc/imports/src/default.ts new file mode 100644 index 0000000000..e205052acb --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/imports/src/default.ts @@ -0,0 +1,188 @@ +/** +* @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 {getSourceFile} from '../../util/src/typescript'; + +/** + * Registers and records usages of `ts.Identifer`s that came from default import statements. + * + * See `DefaultImportTracker` for details. + */ +export interface DefaultImportRecorder { + /** + * Record an association between a `ts.Identifier` which might be emitted and the + * `ts.ImportDeclaration` from which it came. + * + * Alone, this method has no effect as the `ts.Identifier` might not be used in the output. + * The identifier must later be marked as used with `recordUsedIdentifier` in order for its + * import to be preserved. + */ + recordImportedIdentifier(id: ts.Identifier, decl: ts.ImportDeclaration): void; + + /** + * Record the fact that the given `ts.Identifer` will be emitted, and thus its + * `ts.ImportDeclaration`, if it was a previously registered default import, must be preserved. + * + * This method can be called safely for any `ts.Identifer`, regardless of its origin. It will only + * have an effect if the identifier came from a `ts.ImportDeclaration` default import which was + * previously registered with `recordImportedIdentifier`. + */ + recordUsedIdentifier(id: ts.Identifier): void; +} + +/** + * An implementation of `DefaultImportRecorder` which does nothing. + * + * This is useful when default import tracking isn't required, such as when emitting .d.ts code + * or for ngcc. + */ +export const NOOP_DEFAULT_IMPORT_RECORDER: DefaultImportRecorder = { + recordImportedIdentifier: (id: ts.Identifier) => void{}, + recordUsedIdentifier: (id: ts.Identifier) => void{}, +}; + +/** + * TypeScript has trouble with generating default imports inside of transformers for some module + * formats. The issue is that for the statement: + * + * import X from 'some/module'; + * console.log(X); + * + * TypeScript will not use the "X" name in generated code. For normal user code, this is fine + * because references to X will also be renamed. However, if both the import and any references are + * added in a transformer, TypeScript does not associate the two, and will leave the "X" references + * dangling while renaming the import variable. The generated code looks something like: + * + * const module_1 = require('some/module'); + * console.log(X); // now X is a dangling reference. + * + * Therefore, we cannot synthetically add default imports, and must reuse the imports that users + * include. Doing this poses a challenge for imports that are only consumed in the type position in + * the user's code. If Angular reuses the imported symbol in a value position (for example, we + * see a constructor parameter of type Foo and try to write "inject(Foo)") we will also end up with + * a dangling reference, as TS will elide the import because it was only used in the type position + * originally. + * + * To avoid this, the compiler must "touch" the imports with `ts.updateImportClause`, and should + * only do this for imports which are actually consumed. The `DefaultImportTracker` keeps track of + * these imports as they're encountered and emitted, and implements a transform which can correctly + * flag the imports as required. + * + * This problem does not exist for non-default imports as the compiler can easily insert + * "import * as X" style imports for those, and the "X" identifier survives transformation. + */ +export class DefaultImportTracker implements DefaultImportRecorder { + /** + * A `Map` which tracks the `Map` of default import `ts.Identifier`s to their + * `ts.ImportDeclaration`s. These declarations are not guaranteed to be used. + */ + private sourceFileToImportMap = + new Map>(); + + /** + * A `Map` which tracks the `Set` of `ts.ImportDeclaration`s for default imports that were used in + * a given `ts.SourceFile` and need to be preserved. + */ + private sourceFileToUsedImports = new Map>(); + recordImportedIdentifier(id: ts.Identifier, decl: ts.ImportDeclaration): void { + const sf = getSourceFile(id); + if (!this.sourceFileToImportMap.has(sf)) { + this.sourceFileToImportMap.set(sf, new Map()); + } + this.sourceFileToImportMap.get(sf) !.set(id, decl); + } + + recordUsedIdentifier(id: ts.Identifier): void { + const sf = getSourceFile(id); + if (!this.sourceFileToImportMap.has(sf)) { + // The identifier's source file has no registered default imports at all. + return; + } + const identiferToDeclaration = this.sourceFileToImportMap.get(sf) !; + if (!identiferToDeclaration.has(id)) { + // The identifier isn't from a registered default import. + return; + } + const decl = identiferToDeclaration.get(id) !; + + // Add the default import declaration to the set of used import declarations for the file. + if (!this.sourceFileToUsedImports.has(sf)) { + this.sourceFileToUsedImports.set(sf, new Set()); + } + this.sourceFileToUsedImports.get(sf) !.add(decl); + } + + /** + * Get a `ts.TransformerFactory` which will preserve default imports that were previously marked + * as used. + * + * This transformer must run after any other transformers which call `recordUsedIdentifier`. + */ + importPreservingTransformer(): ts.TransformerFactory { + return (context: ts.TransformationContext) => { + return (sf: ts.SourceFile) => { return this.transformSourceFile(sf); }; + }; + } + + /** + * Process a `ts.SourceFile` and replace any `ts.ImportDeclaration`s. + */ + private transformSourceFile(sf: ts.SourceFile): ts.SourceFile { + const originalSf = ts.getOriginalNode(sf) as ts.SourceFile; + // Take a fast path if no import declarations need to be preserved in the file. + if (!this.sourceFileToUsedImports.has(originalSf)) { + return sf; + } + + // There are declarations that need to be preserved. + const importsToPreserve = this.sourceFileToUsedImports.get(originalSf) !; + + // Generate a new statement list which preserves any imports present in `importsToPreserve`. + const statements = sf.statements.map(stmt => { + if (ts.isImportDeclaration(stmt) && importsToPreserve.has(stmt)) { + // Preserving an import that's marked as unreferenced (type-only) is tricky in TypeScript. + // + // Various approaches have been tried, with mixed success: + // + // 1. Using `ts.updateImportDeclaration` does not cause the import to be retained. + // + // 2. Using `ts.createImportDeclaration` with the same `ts.ImportClause` causes the import + // to correctly be retained, but when emitting CommonJS module format code, references + // to the imported value will not match the import variable. + // + // 3. Emitting "import * as" imports instead generates the correct import variable, but + // references are missing the ".default" access. This happens to work for tsickle code + // with goog.module transformations as tsickle strips the ".default" anyway. + // + // 4. It's possible to trick TypeScript by setting `ts.NodeFlag.Synthesized` on the import + // declaration. This causes the import to be correctly retained and generated, but can + // violate invariants elsewhere in the compiler and cause crashes. + // + // 5. Using `ts.getMutableClone` seems to correctly preserve the import and correctly + // generate references to the import variable across all module types. + // + // Therefore, option 5 is the one used here. It seems to be implemented as the correct way + // to perform option 4, which preserves all the compiler's invariants. + // + // TODO(alxhub): discuss with the TypeScript team and determine if there's a better way to + // deal with this issue. + stmt = ts.getMutableClone(stmt); + } + return stmt; + }); + + // Save memory - there's no need to keep these around once the transform has run for the given + // file. + this.sourceFileToImportMap.delete(originalSf); + this.sourceFileToUsedImports.delete(originalSf); + + return ts.updateSourceFileNode(sf, statements); + } +} diff --git a/packages/compiler-cli/src/ngtsc/imports/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/imports/test/BUILD.bazel new file mode 100644 index 0000000000..870fe78ec2 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/imports/test/BUILD.bazel @@ -0,0 +1,26 @@ +load("//tools:defaults.bzl", "jasmine_node_test", "ts_library") + +package(default_visibility = ["//visibility:public"]) + +ts_library( + name = "test_lib", + testonly = True, + srcs = glob([ + "**/*.ts", + ]), + deps = [ + "//packages:types", + "//packages/compiler-cli/src/ngtsc/imports", + "//packages/compiler-cli/src/ngtsc/testing", + "@npm//typescript", + ], +) + +jasmine_node_test( + name = "test", + bootstrap = ["angular/tools/testing/init_node_no_angular_spec.js"], + deps = [ + ":test_lib", + "//tools/testing:node_no_angular", + ], +) diff --git a/packages/compiler-cli/src/ngtsc/imports/test/default_spec.ts b/packages/compiler-cli/src/ngtsc/imports/test/default_spec.ts new file mode 100644 index 0000000000..2f9b284f09 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/imports/test/default_spec.ts @@ -0,0 +1,90 @@ +/** + * @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 {getDeclaration, makeProgram} from '../../testing/in_memory_typescript'; +import {DefaultImportTracker} from '../src/default'; + +describe('DefaultImportTracker', () => { + it('should prevent a default import from being elided if used', () => { + const {program, host} = makeProgram( + [ + {name: 'dep.ts', contents: `export default class Foo {}`}, + {name: 'test.ts', contents: `import Foo from './dep'; export function test(f: Foo) {}`}, + + // This control file is identical to the test file, but will not have its import marked + // for preservation. It exists to verify that it is in fact the action of + // DefaultImportTracker and not some other artifact of the test setup which causes the + // import to be preserved. It will also verify that DefaultImportTracker does not preserve + // imports which are not marked for preservation. + {name: 'ctrl.ts', contents: `import Foo from './dep'; export function test(f: Foo) {}`}, + ], + { + module: ts.ModuleKind.ES2015, + }); + const fooClause = getDeclaration(program, 'test.ts', 'Foo', ts.isImportClause); + const fooId = fooClause.name !; + const fooDecl = fooClause.parent; + + const tracker = new DefaultImportTracker(); + tracker.recordImportedIdentifier(fooId, fooDecl); + tracker.recordUsedIdentifier(fooId); + program.emit(undefined, undefined, undefined, undefined, { + before: [tracker.importPreservingTransformer()], + }); + const testContents = host.readFile('/test.js') !; + expect(testContents).toContain(`import Foo from './dep';`); + + // The control should have the import elided. + const ctrlContents = host.readFile('/ctrl.js'); + expect(ctrlContents).not.toContain(`import Foo from './dep';`); + }); + + it('should transpile imports correctly into commonjs', () => { + const {program, host} = makeProgram( + [ + {name: 'dep.ts', contents: `export default class Foo {}`}, + {name: 'test.ts', contents: `import Foo from './dep'; export function test(f: Foo) {}`}, + ], + { + module: ts.ModuleKind.CommonJS, + }); + const fooClause = getDeclaration(program, 'test.ts', 'Foo', ts.isImportClause); + const fooId = ts.updateIdentifier(fooClause.name !); + const fooDecl = fooClause.parent; + + const tracker = new DefaultImportTracker(); + tracker.recordImportedIdentifier(fooId, fooDecl); + tracker.recordUsedIdentifier(fooId); + program.emit(undefined, undefined, undefined, undefined, { + before: [ + addReferenceTransformer(fooId), + tracker.importPreservingTransformer(), + ], + }); + const testContents = host.readFile('/test.js') !; + expect(testContents).toContain(`var dep_1 = require("./dep");`); + expect(testContents).toContain(`var ref = dep_1["default"];`); + }); +}); + +function addReferenceTransformer(id: ts.Identifier): ts.TransformerFactory { + return (context: ts.TransformationContext) => { + return (sf: ts.SourceFile) => { + if (id.getSourceFile().fileName === sf.fileName) { + return ts.updateSourceFileNode(sf, [ + ...sf.statements, ts.createVariableStatement(undefined, ts.createVariableDeclarationList([ + ts.createVariableDeclaration('ref', undefined, id), + ])) + ]); + } + return sf; + }; + }; +} diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 403e48f9c4..23835421e8 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -17,7 +17,7 @@ import {BaseDefDecoratorHandler} from './annotations/src/base_def'; import {CycleAnalyzer, ImportGraph} from './cycles'; import {ErrorCode, ngErrorCode} from './diagnostics'; import {FlatIndexGenerator, ReferenceGraph, checkForPrivateExports, findFlatIndexEntryPoint} from './entry_point'; -import {AbsoluteModuleStrategy, AliasGenerator, AliasStrategy, FileToModuleHost, FileToModuleStrategy, ImportRewriter, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NoopImportRewriter, R3SymbolsImportRewriter, Reference, ReferenceEmitter} from './imports'; +import {AbsoluteModuleStrategy, AliasGenerator, AliasStrategy, DefaultImportTracker, FileToModuleHost, FileToModuleStrategy, ImportRewriter, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NoopImportRewriter, R3SymbolsImportRewriter, Reference, ReferenceEmitter} from './imports'; import {PartialEvaluator} from './partial_evaluator'; import {AbsoluteFsPath, LogicalFileSystem} from './path'; import {TypeScriptReflectionHost} from './reflection'; @@ -56,6 +56,7 @@ export class NgtscProgram implements api.Program { private refEmitter: ReferenceEmitter|null = null; private fileToModuleHost: FileToModuleHost|null = null; + private defaultImportTracker: DefaultImportTracker; constructor( rootNames: ReadonlyArray, private options: api.CompilerOptions, @@ -132,6 +133,7 @@ export class NgtscProgram implements api.Program { this.entryPoint = entryPoint !== null ? this.tsProgram.getSourceFile(entryPoint) || null : null; this.moduleResolver = new ModuleResolver(this.tsProgram, options, this.host); this.cycleAnalyzer = new CycleAnalyzer(new ImportGraph(this.moduleResolver)); + this.defaultImportTracker = new DefaultImportTracker(); } getTsProgram(): ts.Program { return this.tsProgram; } @@ -273,11 +275,13 @@ export class NgtscProgram implements api.Program { }; const customTransforms = opts && opts.customTransformers; + const beforeTransforms = [ ivyTransformFactory( - compilation, this.reflector, this.importRewriter, this.isCore, + compilation, this.reflector, this.importRewriter, this.defaultImportTracker, this.isCore, this.closureCompilerEnabled), aliasTransformFactory(compilation.exportStatements) as ts.TransformerFactory, + this.defaultImportTracker.importPreservingTransformer(), ]; const afterDeclarationsTransforms = [ declarationTransformFactory(compilation), @@ -386,14 +390,17 @@ export class NgtscProgram implements api.Program { this.reflector, evaluator, scopeRegistry, this.isCore, this.resourceManager, this.rootDirs, this.options.preserveWhitespaces || false, this.options.i18nUseExternalIds !== false, this.moduleResolver, this.cycleAnalyzer, - this.refEmitter), - new DirectiveDecoratorHandler(this.reflector, evaluator, scopeRegistry, this.isCore), + this.refEmitter, this.defaultImportTracker), + new DirectiveDecoratorHandler( + this.reflector, evaluator, scopeRegistry, this.defaultImportTracker, this.isCore), new InjectableDecoratorHandler( - this.reflector, this.isCore, this.options.strictInjectionParameters || false), + this.reflector, this.defaultImportTracker, this.isCore, + this.options.strictInjectionParameters || false), new NgModuleDecoratorHandler( this.reflector, evaluator, scopeRegistry, referencesRegistry, this.isCore, - this.routeAnalyzer, this.refEmitter), - new PipeDecoratorHandler(this.reflector, evaluator, scopeRegistry, this.isCore), + this.routeAnalyzer, this.refEmitter, this.defaultImportTracker), + new PipeDecoratorHandler( + this.reflector, evaluator, scopeRegistry, this.defaultImportTracker, this.isCore), ]; return new IvyCompilation( diff --git a/packages/compiler-cli/src/ngtsc/reflection/index.ts b/packages/compiler-cli/src/ngtsc/reflection/index.ts index 75186f70d8..145dd3a55a 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/index.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/index.ts @@ -7,5 +7,5 @@ */ export * from './src/host'; -export {DEFAULT_EXPORT_NAME, typeNodeToValueExpr} from './src/type_to_value'; +export {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/host.ts b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts index ef46b8fd0d..3a751d417f 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts @@ -167,7 +167,7 @@ export interface ClassMember { * valid within the local file where the type was referenced. */ export type TypeValueReference = { - local: true; expression: ts.Expression; + local: true; expression: ts.Expression; defaultImportStatement: ts.ImportDeclaration | null; } | { local: false; 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 index f8ffff8aad..edcab882ab 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts @@ -10,8 +10,6 @@ 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. @@ -45,7 +43,16 @@ export function typeToValue( // 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)) { + if (firstDecl && ts.isImportClause(firstDecl) && firstDecl.name !== undefined) { + // This is a default import. + return { + local: true, + // Copying the name here ensures the generated references will be correctly transformed along + // with the import. + expression: ts.updateIdentifier(firstDecl.name), + defaultImportStatement: firstDecl.parent, + }; + } else if (firstDecl && isImportSource(firstDecl)) { const origin = extractModuleAndNameFromImport(firstDecl, symbols.importName); return {local: false, valueDeclaration: decl.valueDeclaration, ...origin}; } else { @@ -54,6 +61,7 @@ export function typeToValue( return { local: true, expression, + defaultImportStatement: null, }; } else { return null; @@ -143,9 +151,8 @@ function entityNameToValue(node: ts.EntityName): ts.Expression|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 isImportSource(node: ts.Declaration): node is(ts.ImportSpecifier | ts.NamespaceImport) { + return ts.isImportSpecifier(node) || ts.isNamespaceImport(node); } function extractModuleAndNameFromImport( @@ -169,10 +176,6 @@ function extractModuleAndNameFromImport( 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]}`); } 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 0ad0c20598..4e2926ce25 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 @@ -171,7 +171,12 @@ describe('reflector', () => { const host = new TypeScriptReflectionHost(checker); const args = host.getConstructorParameters(clazz) !; expect(args.length).toBe(1); - expectParameter(args[0], 'bar', {moduleName: './bar', name: '*'}); + const param = args[0].typeValueReference; + if (param === null || !param.local) { + return fail('Expected local parameter'); + } + expect(param).not.toBeNull(); + expect(param.defaultImportStatement).not.toBeNull(); }); it('should reflect a nullable argument', () => { diff --git a/packages/compiler-cli/src/ngtsc/testing/in_memory_typescript.ts b/packages/compiler-cli/src/ngtsc/testing/in_memory_typescript.ts index 9bbdf1f2c2..51bb2be60d 100644 --- a/packages/compiler-cli/src/ngtsc/testing/in_memory_typescript.ts +++ b/packages/compiler-cli/src/ngtsc/testing/in_memory_typescript.ts @@ -133,6 +133,10 @@ export function getDeclaration( if (stmt.name !== undefined && stmt.name.text === name) { chosenDecl = stmt; } + } else if ( + ts.isImportDeclaration(stmt) && stmt.importClause !== undefined && + stmt.importClause.name !== undefined && stmt.importClause.name.text === name) { + chosenDecl = stmt.importClause; } }); diff --git a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts index 89fb187f1c..563e3f02cf 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts @@ -9,12 +9,11 @@ import {ConstantPool} from '@angular/compiler'; import * as ts from 'typescript'; -import {ImportRewriter} from '../../imports'; +import {DefaultImportRecorder, ImportRewriter} from '../../imports'; import {Decorator, ReflectionHost} from '../../reflection'; import {ImportManager, translateExpression, translateStatement} from '../../translator'; import {VisitListEntryResult, Visitor, visit} from '../../util/src/visitor'; -import {CompileResult} from './api'; import {IvyCompilation} from './compilation'; import {addImports} from './utils'; @@ -33,11 +32,13 @@ interface FileOverviewMeta { export function ivyTransformFactory( compilation: IvyCompilation, reflector: ReflectionHost, importRewriter: ImportRewriter, - isCore: boolean, isClosureCompilerEnabled: boolean): ts.TransformerFactory { + defaultImportRecorder: DefaultImportRecorder, isCore: boolean, + isClosureCompilerEnabled: boolean): ts.TransformerFactory { return (context: ts.TransformationContext): ts.Transformer => { return (file: ts.SourceFile): ts.SourceFile => { return transformIvySourceFile( - compilation, context, reflector, importRewriter, file, isCore, isClosureCompilerEnabled); + compilation, context, reflector, importRewriter, file, isCore, isClosureCompilerEnabled, + defaultImportRecorder); }; }; } @@ -45,8 +46,8 @@ export function ivyTransformFactory( class IvyVisitor extends Visitor { constructor( private compilation: IvyCompilation, private reflector: ReflectionHost, - private importManager: ImportManager, private isCore: boolean, - private constantPool: ConstantPool) { + private importManager: ImportManager, private defaultImportRecorder: DefaultImportRecorder, + private isCore: boolean, private constantPool: ConstantPool) { super(); } @@ -63,14 +64,16 @@ class IvyVisitor extends Visitor { res.forEach(field => { // Translate the initializer for the field into TS nodes. - const exprNode = translateExpression(field.initializer, this.importManager); + const exprNode = + translateExpression(field.initializer, this.importManager, this.defaultImportRecorder); // Create a static property declaration for the new field. const property = ts.createProperty( undefined, [ts.createToken(ts.SyntaxKind.StaticKeyword)], field.name, undefined, undefined, exprNode); - field.statements.map(stmt => translateStatement(stmt, this.importManager)) + field.statements + .map(stmt => translateStatement(stmt, this.importManager, this.defaultImportRecorder)) .forEach(stmt => statements.push(stmt)); members.push(property); @@ -202,17 +205,20 @@ class IvyVisitor extends Visitor { function transformIvySourceFile( compilation: IvyCompilation, context: ts.TransformationContext, reflector: ReflectionHost, importRewriter: ImportRewriter, file: ts.SourceFile, isCore: boolean, - isClosureCompilerEnabled: boolean): ts.SourceFile { + isClosureCompilerEnabled: boolean, + defaultImportRecorder: DefaultImportRecorder): ts.SourceFile { const constantPool = new ConstantPool(); const importManager = new ImportManager(importRewriter); // Recursively scan through the AST and perform any updates requested by the IvyCompilation. - const visitor = new IvyVisitor(compilation, reflector, importManager, isCore, constantPool); + const visitor = new IvyVisitor( + compilation, reflector, importManager, defaultImportRecorder, isCore, constantPool); let sf = visit(file, visitor, context); // Generate the constant statements first, as they may involve adding additional imports // to the ImportManager. - const constants = constantPool.statements.map(stmt => translateStatement(stmt, importManager)); + const constants = constantPool.statements.map( + stmt => translateStatement(stmt, importManager, defaultImportRecorder)); // Preserve @fileoverview comments required by Closure, since the location might change as a // result of adding extra imports and constant pool statements. diff --git a/packages/compiler-cli/src/ngtsc/transform/src/utils.ts b/packages/compiler-cli/src/ngtsc/transform/src/utils.ts index 513d263cd6..a113506905 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/utils.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/utils.ts @@ -20,16 +20,9 @@ export function addImports( // 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); - } + const importClause = ts.createImportClause( + /* name */ undefined, + /* namedBindings */ ts.createNamespaceImport(qualifier)); return ts.createImportDeclaration( /* decorators */ undefined, /* modifiers */ undefined, diff --git a/packages/compiler-cli/src/ngtsc/translator/BUILD.bazel b/packages/compiler-cli/src/ngtsc/translator/BUILD.bazel index 966e62bcbb..351b1a7513 100644 --- a/packages/compiler-cli/src/ngtsc/translator/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/translator/BUILD.bazel @@ -9,7 +9,6 @@ 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 0ffd59cc20..94f630ae6b 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -9,8 +9,7 @@ import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeVisitor, TypeofExpr, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; import * as ts from 'typescript'; -import {ImportRewriter, NoopImportRewriter} from '../../imports'; -import {DEFAULT_EXPORT_NAME} from '../../reflection'; +import {DefaultImportRecorder, ImportRewriter, NOOP_DEFAULT_IMPORT_RECORDER, NoopImportRewriter} from '../../imports'; export class Context { constructor(readonly isStatement: boolean) {} @@ -40,8 +39,7 @@ const BINARY_OPERATORS = new Map([ ]); export class ImportManager { - private nonDefaultImports = new Map(); - private defaultImports = new Map(); + private specifierToIdentifier = new Map(); private nextIndex = 0; constructor(protected rewriter: ImportRewriter = new NoopImportRewriter(), private prefix = 'i') { @@ -61,47 +59,36 @@ export class ImportManager { // If not, this symbol will be imported. Allocate a prefix for the imported module if needed. - 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++}`); + if (!this.specifierToIdentifier.has(moduleName)) { + this.specifierToIdentifier.set(moduleName, `${this.prefix}${this.nextIndex++}`); } - const moduleImport = trackingMap.get(moduleName) !; + const moduleImport = this.specifierToIdentifier.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}; - } + return {moduleImport, symbol}; } - getAllImports(contextPath: string): {specifier: string, qualifier: string, isDefault: boolean}[] { - const imports: {specifier: string, qualifier: string, isDefault: boolean}[] = []; - this.nonDefaultImports.forEach((qualifier, specifier) => { + getAllImports(contextPath: string): {specifier: string, qualifier: string}[] { + const imports: {specifier: string, qualifier: string}[] = []; + this.specifierToIdentifier.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}); + imports.push({specifier, qualifier}); }); return imports; } } -export function translateExpression(expression: Expression, imports: ImportManager): ts.Expression { - return expression.visitExpression(new ExpressionTranslatorVisitor(imports), new Context(false)); +export function translateExpression( + expression: Expression, imports: ImportManager, + defaultImportRecorder: DefaultImportRecorder): ts.Expression { + return expression.visitExpression( + new ExpressionTranslatorVisitor(imports, defaultImportRecorder), new Context(false)); } -export function translateStatement(statement: Statement, imports: ImportManager): ts.Statement { - return statement.visitStatement(new ExpressionTranslatorVisitor(imports), new Context(true)); +export function translateStatement( + statement: Statement, imports: ImportManager, + defaultImportRecorder: DefaultImportRecorder): ts.Statement { + return statement.visitStatement( + new ExpressionTranslatorVisitor(imports, defaultImportRecorder), new Context(true)); } export function translateType(type: Type, imports: ImportManager): ts.TypeNode { @@ -110,7 +97,8 @@ export function translateType(type: Type, imports: ImportManager): ts.TypeNode { class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor { private externalSourceFiles = new Map(); - constructor(private imports: ImportManager) {} + constructor( + private imports: ImportManager, private defaultImportRecorder: DefaultImportRecorder) {} visitDeclareVarStmt(stmt: DeclareVarStmt, context: Context): ts.VariableStatement { const nodeFlags = stmt.hasModifier(StmtModifier.Final) ? ts.NodeFlags.Const : ts.NodeFlags.None; @@ -324,7 +312,12 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor throw new Error('Method not implemented.'); } - visitWrappedNodeExpr(ast: WrappedNodeExpr, context: Context): any { return ast.node; } + visitWrappedNodeExpr(ast: WrappedNodeExpr, context: Context): any { + if (ts.isIdentifier(ast.node)) { + this.defaultImportRecorder.recordUsedIdentifier(ast.node); + } + return ast.node; + } visitTypeofExpr(ast: TypeofExpr, context: Context): ts.TypeOfExpression { return ts.createTypeOf(ast.expr.visitExpression(this, context)); @@ -496,7 +489,7 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor { } visitTypeofExpr(ast: TypeofExpr, context: Context): ts.TypeQueryNode { - let expr = translateExpression(ast.expr, this.imports); + let expr = translateExpression(ast.expr, this.imports, NOOP_DEFAULT_IMPORT_RECORDER); return ts.createTypeQueryNode(expr as ts.Identifier); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index 02f160e036..582b68442e 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -135,13 +135,7 @@ 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 => { - if (!i.isDefault) { - return `import * as ${i.qualifier} from '${i.specifier}';`; - } else { - return `import ${i.qualifier} from '${i.specifier}';`; - } - }) + .map(i => `import * as ${i.qualifier} from '${i.specifier}';`) .join('\n'); code = imports + '\n' + code; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index a928f16898..270ce22dbb 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -9,7 +9,7 @@ import {AST, BindingType, BoundTarget, ImplicitReceiver, PropertyRead, TmplAstBoundAttribute, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; import * as ts from 'typescript'; -import {Reference, ReferenceEmitter} from '../../imports'; +import {NOOP_DEFAULT_IMPORT_RECORDER, Reference, ReferenceEmitter} from '../../imports'; import {ImportManager, translateExpression} from '../../translator'; import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta} from './api'; @@ -83,7 +83,7 @@ class Context { } // Use `translateExpression` to convert the `Expression` into a `ts.Expression`. - return translateExpression(ngExpr, this.importManager); + return translateExpression(ngExpr, this.importManager, NOOP_DEFAULT_IMPORT_RECORDER); } } diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 35924de0ab..cdacd28426 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -2121,9 +2121,9 @@ describe('ngtsc behavioral tests', () => { env.driveMain(); const jsContents = trim(env.getContents('test.js')); - expect(jsContents).toContain(`import * as types from './types';`); - expect(jsContents).toMatch(setClassMetadataRegExp('type: i\\d\\.MyTypeA')); - expect(jsContents).toMatch(setClassMetadataRegExp('type: i\\d\\.MyTypeB')); + expect(jsContents).toContain(`import * as i1 from "./types";`); + expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.MyTypeA')); + expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.MyTypeB')); }); it('should use default-imported types if they can be represented as values', () => { @@ -2146,12 +2146,12 @@ describe('ngtsc behavioral tests', () => { 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')); + expect(jsContents).toContain(`import Default from './types';`); + expect(jsContents).toContain(`import * as i1 from "./types";`); + expect(jsContents).toContain('i0.ɵdirectiveInject(Default)'); + expect(jsContents).toContain('i0.ɵdirectiveInject(i1.Other)'); + expect(jsContents).toMatch(setClassMetadataRegExp('type: Default')); + expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.Other')); }); it('should use `undefined` in setClassMetadata if types can\'t be represented as values', () => {