Skip to content

Commit

Permalink
refactor(compiler-cli): introduce new implementation of `ImportManage…
Browse files Browse the repository at this point in the history
…r` (#54983)

This commit introduces a new implementation of `ImportManager` that has
numerous benefits:

- It allows efficient re-use of original source file imports.
  * either fully re-using original imports if matching
  * updating existing import declarations to include new symbols.
- It allows efficient re-use of previous generated imports.
- The manager can be used for schematics and migrations.

The implementation is a rework of the import manager that we originally
built for schematics in Angular Material, but this commit improved it
to be more flexible, more readable, and "correct".

In follow-ups we can use this for schematics/migrations.

PR Close #54983
  • Loading branch information
devversion authored and dylhunn committed Mar 27, 2024
1 parent 365fd50 commit 3253576
Show file tree
Hide file tree
Showing 13 changed files with 1,464 additions and 27 deletions.
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngtsc/imports/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export {DeferredSymbolTracker} from './src/deferred_symbol_tracker';
export {AbsoluteModuleStrategy, assertSuccessfulReferenceEmit, EmittedReference, FailedEmitResult, ImportedFile, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitKind, ReferenceEmitResult, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesStrategy} from './src/emitter';
export {ImportedSymbolsTracker} from './src/imported_symbols_tracker';
export {LocalCompilationExtraImportsTracker} from './src/local_compilation_extra_imports_tracker';
export {isAliasImportDeclaration, loadIsReferencedAliasDeclarationPatch} from './src/patch_alias_reference_resolution';
export {AliasImportDeclaration, isAliasImportDeclaration, loadIsReferencedAliasDeclarationPatch} from './src/patch_alias_reference_resolution';
export {Reexport} from './src/reexport';
export {OwningModule, Reference} from './src/references';
export {ModuleResolver} from './src/resolver';
16 changes: 0 additions & 16 deletions packages/compiler-cli/src/ngtsc/imports/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,6 @@ import {relativePathBetween} from '../../util/src/path';
* Rewrites imports of symbols being written into generated code.
*/
export interface ImportRewriter {
/**
* Should the given symbol be imported at all?
*
* If `true`, the symbol should be imported from the given specifier. If `false`, the symbol
* should be referenced directly, without an import.
*/
shouldImportSymbol(symbol: string, specifier: string): boolean;

/**
* Optionally rewrite a reference to an imported symbol, changing either the binding prefix or the
* symbol name itself.
Expand All @@ -36,10 +28,6 @@ export interface ImportRewriter {
* `ImportRewriter` that does no rewriting.
*/
export class NoopImportRewriter implements ImportRewriter {
shouldImportSymbol(symbol: string, specifier: string): boolean {
return true;
}

rewriteSymbol(symbol: string, specifier: string): string {
return symbol;
}
Expand Down Expand Up @@ -78,10 +66,6 @@ const CORE_MODULE = '@angular/core';
export class R3SymbolsImportRewriter implements ImportRewriter {
constructor(private r3SymbolsPath: string) {}

shouldImportSymbol(symbol: string, specifier: string): boolean {
return true;
}

rewriteSymbol(symbol: string, specifier: string): string {
if (specifier !== CORE_MODULE) {
// This import isn't from core, so ignore it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

import ts from 'typescript';

/** Possible alias import declarations */
export type AliasImportDeclaration = ts.ImportSpecifier|ts.NamespaceImport|ts.ImportClause;

/**
* Describes a TypeScript transformation context with the internal emit
* resolver exposed. There are requests upstream in TypeScript to expose
Expand All @@ -22,7 +25,7 @@ const patchedReferencedAliasesSymbol = Symbol('patchedReferencedAliases');
/** Describes a subset of the TypeScript internal emit resolver. */
interface EmitResolver {
isReferencedAliasDeclaration?(node: ts.Node, ...args: unknown[]): void;
[patchedReferencedAliasesSymbol]?: Set<ts.Declaration>;
[patchedReferencedAliasesSymbol]?: Set<AliasImportDeclaration>;
}

/**
Expand Down Expand Up @@ -95,9 +98,9 @@ export function loadIsReferencedAliasDeclarationPatch(context: ts.Transformation
throwIncompatibleTransformationContextError();
}

const referencedAliases = new Set<ts.Declaration>();
const referencedAliases = new Set<AliasImportDeclaration>();
emitResolver.isReferencedAliasDeclaration = function(node, ...args) {
if (isAliasImportDeclaration(node) && referencedAliases.has(node)) {
if (isAliasImportDeclaration(node) && (referencedAliases as Set<ts.Node>).has(node)) {
return true;
}
return originalIsReferencedAliasDeclaration.call(emitResolver, node, ...args);
Expand All @@ -110,8 +113,7 @@ export function loadIsReferencedAliasDeclarationPatch(context: ts.Transformation
* declarations can be import specifiers, namespace imports or import clauses
* as these do not declare an actual symbol but just point to a target declaration.
*/
export function isAliasImportDeclaration(node: ts.Node): node is ts.ImportSpecifier|
ts.NamespaceImport|ts.ImportClause {
export function isAliasImportDeclaration(node: ts.Node): node is AliasImportDeclaration {
return ts.isImportSpecifier(node) || ts.isNamespaceImport(node) || ts.isImportClause(node);
}

Expand Down
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/translator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
*/

export {AstFactory, BinaryOperator, LeadingComment, ObjectLiteralProperty, SourceMapLocation, SourceMapRange, TemplateElement, TemplateLiteral, UnaryOperator, VariableDeclarationType} from './src/api/ast_factory';
export {ImportGenerator, NamedImport} from './src/api/import_generator';
export {ImportGenerator, ImportRequest, NamedImport} from './src/api/import_generator';
export {Context} from './src/context';
export {Import, ImportManager, NamespaceImport, SideEffectImport} from './src/import_manager';
export {ImportManagerConfig, ImportManagerV2, presetImportManagerForceNamespaceImports} from './src/import_manager/import_manager';
export {ExpressionTranslatorVisitor, RecordWrappedNodeFn, TranslatorOptions} from './src/translator';
export {canEmitType, TypeEmitter, TypeReferenceTranslator} from './src/type_emitter';
export {translateType} from './src/type_translator';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,32 @@ export interface NamedImport<TExpression> {
symbol: string;
}

/**
* A request to import a given symbol from the given module.
*/
export interface ImportRequest<TFile> {
/**
* Name of the export to be imported.
* May be `null` if a namespace import is requested.
*/
exportSymbolName: string|null;

/**
* Module specifier to be imported.
* May be a module name, or a file-relative path.
*/
exportModuleSpecifier: string;

/**
* File for which the import is requested for. This may
* be used by import generators to re-use existing imports.
*
* Import managers may also allow this to be nullable if
* imports are never re-used. E.g. in the linker generator.
*/
requestedFile: TFile;
}

/**
* Generate import information based on the context of the code being generated.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import ts from 'typescript';

import {ImportRewriter, NoopImportRewriter} from '../../imports';

import {ImportGenerator, NamedImport} from './api/import_generator';

/**
* Information about a namespace import that has been added to a module.
*/
Expand Down Expand Up @@ -41,7 +39,7 @@ export interface SideEffectImport {
*/
export type Import = NamespaceImport|SideEffectImport;

export class ImportManager implements ImportGenerator<ts.Identifier> {
export class ImportManager {
private specifierToIdentifier = new Map<string, ts.Identifier|null>();
private nextIndex = 0;

Expand All @@ -60,7 +58,7 @@ export class ImportManager implements ImportGenerator<ts.Identifier> {
return this.specifierToIdentifier.get(moduleName)!;
}

generateNamedImport(moduleName: string, originalSymbol: string): NamedImport<ts.Identifier> {
generateNamedImport(moduleName: string, originalSymbol: string): any {
// First, rewrite the symbol name.
const symbol = this.rewriter.rewriteSymbol(originalSymbol, moduleName);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* @license
* Copyright Google LLC 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 ts from 'typescript';

import type {ImportManagerConfig} from './import_manager';

/** Extension of `ts.SourceFile` with metadata fields that are marked as internal. */
interface SourceFileWithIdentifiers extends ts.SourceFile {
/** List of all identifiers encountered while parsing the source file. */
identifiers?: Map<string, string>;
}

/**
* Generates a helper for `ImportManagerConfig` to generate unique identifiers
* for a given source file.
*/
export function createGenerateUniqueIdentifierHelper():
ImportManagerConfig['generateUniqueIdentifier'] {
const generatedIdentifiers = new Set<string>();

return (sourceFile: ts.SourceFile, symbolName: string) => {
const sf = sourceFile as SourceFileWithIdentifiers;
if (sf.identifiers === undefined) {
throw new Error('Source file unexpectedly lacks map of parsed `identifiers`.');
}

const isUniqueIdentifier = (name: string) =>
!sf.identifiers!.has(name) && !generatedIdentifiers.has(name);

if (isUniqueIdentifier(symbolName)) {
generatedIdentifiers.add(symbolName);
return null;
}

let name = null;
let counter = 1;
do {
name = `${symbolName}_${counter++}`;
} while (!isUniqueIdentifier(name));

generatedIdentifiers.add(name);
return ts.factory.createUniqueName(name, ts.GeneratedIdentifierFlags.Optimistic);
};
}
Loading

0 comments on commit 3253576

Please sign in to comment.