From 5b9c96b9b8d7b607617a8f1d87579e817f0ae7bc Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Thu, 19 Dec 2019 15:38:04 -0800 Subject: [PATCH] refactor(ivy): change ImportMode enum to ImportFlags (#34649) Previously, `ReferenceEmitter.emit()` took an `ImportMode` enum value, where one value of the enum allowed forcing new imports to be generated when emitting a reference to some value or type. This commit refactors `ImportMode` to be an `ImportFlags` value instead. Using a bit field of flags will allow future customization of reference emitting. PR Close #34649 --- .../src/ngtsc/annotations/src/util.ts | 6 ++-- .../compiler-cli/src/ngtsc/imports/index.ts | 2 +- .../src/ngtsc/imports/src/alias.ts | 8 +++-- .../src/ngtsc/imports/src/emitter.ts | 32 +++++++++++++------ .../src/ngtsc/imports/src/references.ts | 5 --- .../ngtsc/modulewithproviders/src/scanner.ts | 4 +-- 6 files changed, 34 insertions(+), 23 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts index 5bce8ff698..4a7bbc7c4e 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts @@ -10,7 +10,7 @@ import {Expression, ExternalExpr, R3DependencyMetadata, R3Reference, R3ResolvedD import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic} from '../../diagnostics'; -import {DefaultImportRecorder, ImportMode, Reference, ReferenceEmitter} from '../../imports'; +import {DefaultImportRecorder, ImportFlags, Reference, ReferenceEmitter} from '../../imports'; import {ForeignFunctionResolver, PartialEvaluator} from '../../partial_evaluator'; import {ClassDeclaration, CtorParameter, Decorator, Import, ReflectionHost, TypeValueReference, isNamedClassDeclaration} from '../../reflection'; import {DeclarationData} from '../../scope'; @@ -189,8 +189,8 @@ export function validateConstructorDependencies( export function toR3Reference( valueRef: Reference, typeRef: Reference, valueContext: ts.SourceFile, typeContext: ts.SourceFile, refEmitter: ReferenceEmitter): R3Reference { - const value = refEmitter.emit(valueRef, valueContext, ImportMode.UseExistingImport); - const type = refEmitter.emit(typeRef, typeContext, ImportMode.ForceNewImport); + const value = refEmitter.emit(valueRef, valueContext); + const type = refEmitter.emit(typeRef, typeContext, ImportFlags.ForceNewImport); if (value === null || type === null) { throw new Error(`Could not refer to ${ts.SyntaxKind[valueRef.node.kind]}`); } diff --git a/packages/compiler-cli/src/ngtsc/imports/index.ts b/packages/compiler-cli/src/ngtsc/imports/index.ts index 446aa54cc7..43f63e7c28 100644 --- a/packages/compiler-cli/src/ngtsc/imports/index.ts +++ b/packages/compiler-cli/src/ngtsc/imports/index.ts @@ -11,5 +11,5 @@ export {ImportRewriter, NoopImportRewriter, R3SymbolsImportRewriter, validateAnd export {DefaultImportRecorder, DefaultImportTracker, NOOP_DEFAULT_IMPORT_RECORDER} from './src/default'; export {AbsoluteModuleStrategy, FileToModuleHost, FileToModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy} from './src/emitter'; export {Reexport} from './src/reexport'; -export {ImportMode, OwningModule, Reference} from './src/references'; +export {ImportFlags, OwningModule, Reference} from './src/references'; export {ModuleResolver} from './src/resolver'; diff --git a/packages/compiler-cli/src/ngtsc/imports/src/alias.ts b/packages/compiler-cli/src/ngtsc/imports/src/alias.ts index 79ba0f668b..d28c53d8a5 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/alias.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/alias.ts @@ -10,8 +10,10 @@ import {Expression, ExternalExpr} from '@angular/compiler'; import * as ts from 'typescript'; import {ClassDeclaration, ReflectionHost, isNamedClassDeclaration} from '../../reflection'; -import {FileToModuleHost, ReferenceEmitStrategy} from './emitter'; -import {ImportMode, Reference} from './references'; + +import {FileToModuleHost, ImportFlags, ReferenceEmitStrategy} from './emitter'; +import {Reference} from './references'; + // Escape anything that isn't alphanumeric, '/' or '_'. const CHARS_TO_ESCAPE = /[^a-zA-Z0-9/_]/g; @@ -206,7 +208,7 @@ export class PrivateExportAliasingHost implements AliasingHost { * directive or pipe, if it exists. */ export class AliasStrategy implements ReferenceEmitStrategy { - emit(ref: Reference, context: ts.SourceFile, importMode: ImportMode): Expression|null { + emit(ref: Reference, context: ts.SourceFile, importMode: ImportFlags): Expression|null { return ref.alias; } } diff --git a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts index 4606d49ec9..4f5c12f98b 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts @@ -14,7 +14,7 @@ import {ReflectionHost} from '../../reflection'; import {getSourceFile, isDeclaration, nodeNameForError} from '../../util/src/typescript'; import {findExportedNameOfNode} from './find_export'; -import {ImportMode, Reference} from './references'; +import {Reference} from './references'; import {ModuleResolver} from './resolver'; @@ -29,6 +29,21 @@ export interface FileToModuleHost { fileNameToModuleName(importedFilePath: string, containingFilePath: string): string; } +/** + * Flags which alter the imports generated by the `ReferenceEmitter`. + */ +export enum ImportFlags { + None = 0x00, + + /** + * Force the generation of a new import when generating a reference, even if an identifier already + * exists in the target file which could be used instead. + * + * This is sometimes required if there's a risk TypeScript might remove imports during emit. + */ + ForceNewImport = 0x01, +} + /** * A particular strategy for generating an expression which refers to a `Reference`. * @@ -51,7 +66,7 @@ export interface ReferenceEmitStrategy { * @param importMode a flag which controls whether imports should be generated or not * @returns an `Expression` which refers to the `Reference`, or `null` if none can be generated */ - emit(ref: Reference, context: ts.SourceFile, importMode: ImportMode): Expression|null; + emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): Expression|null; } /** @@ -63,11 +78,10 @@ export interface ReferenceEmitStrategy { export class ReferenceEmitter { constructor(private strategies: ReferenceEmitStrategy[]) {} - emit( - ref: Reference, context: ts.SourceFile, - importMode: ImportMode = ImportMode.UseExistingImport): Expression { + emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags = ImportFlags.None): + Expression { for (const strategy of this.strategies) { - const emitted = strategy.emit(ref, context, importMode); + const emitted = strategy.emit(ref, context, importFlags); if (emitted !== null) { return emitted; } @@ -82,10 +96,10 @@ export class ReferenceEmitter { * such identifiers are available. */ export class LocalIdentifierStrategy implements ReferenceEmitStrategy { - emit(ref: Reference, context: ts.SourceFile, importMode: ImportMode): Expression|null { + emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): Expression|null { // If the emitter has specified ForceNewImport, then LocalIdentifierStrategy should not use a // local identifier at all, *except* in the source file where the node is actually declared. - if (importMode === ImportMode.ForceNewImport && + if (importFlags & ImportFlags.ForceNewImport && getSourceFile(ref.node) !== getSourceFile(context)) { return null; } @@ -121,7 +135,7 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy { protected program: ts.Program, protected checker: ts.TypeChecker, protected moduleResolver: ModuleResolver, private reflectionHost: ReflectionHost) {} - emit(ref: Reference, context: ts.SourceFile, importMode: ImportMode): Expression|null { + emit(ref: Reference, context: ts.SourceFile): Expression|null { if (ref.bestGuessOwningModule === null) { // There is no module name available for this Reference, meaning it was arrived at via a // relative path. diff --git a/packages/compiler-cli/src/ngtsc/imports/src/references.ts b/packages/compiler-cli/src/ngtsc/imports/src/references.ts index f2432d9c3f..8b1df442cc 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/references.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/references.ts @@ -11,11 +11,6 @@ import * as ts from 'typescript'; import {identifierOfNode} from '../../util/src/typescript'; -export enum ImportMode { - UseExistingImport, - ForceNewImport, -} - export interface OwningModule { specifier: string; resolutionContext: string; diff --git a/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts b/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts index bb85d24aa4..137151ed5b 100644 --- a/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts +++ b/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts @@ -9,7 +9,7 @@ import {ExpressionType, ExternalExpr, R3Identifiers as Identifiers, Type} from '@angular/compiler'; import * as ts from 'typescript'; -import {ImportMode, Reference, ReferenceEmitter} from '../../imports'; +import {ImportFlags, Reference, ReferenceEmitter} from '../../imports'; import {PartialEvaluator, ResolvedValueMap} from '../../partial_evaluator'; import {ReflectionHost} from '../../reflection'; @@ -98,7 +98,7 @@ export class ModuleWithProvidersScanner { } const ngModuleExpr = - this.emitter.emit(ngModule, decl.getSourceFile(), ImportMode.ForceNewImport); + this.emitter.emit(ngModule, decl.getSourceFile(), ImportFlags.ForceNewImport); const ngModuleType = new ExpressionType(ngModuleExpr); const mwpNgType = new ExpressionType( new ExternalExpr(Identifiers.ModuleWithProviders), /* modifiers */ null, [ngModuleType]);