From 74b7a355aee40568d0da7858372d29ed5bc1abda Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 11 Mar 2024 14:31:30 +0000 Subject: [PATCH 01/10] refactor(compiler-cli): introduce new implementation of `ImportManager` 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. --- .../compiler-cli/src/ngtsc/imports/index.ts | 2 +- .../src/ngtsc/imports/src/core.ts | 16 - .../src/patch_alias_reference_resolution.ts | 12 +- .../src/ngtsc/translator/index.ts | 3 +- .../translator/src/api/import_generator.ts | 26 + .../ngtsc/translator/src/import_manager.ts | 6 +- .../check_unique_identifier_name.ts | 50 ++ .../src/import_manager/import_manager.ts | 330 ++++++++ .../import_typescript_transform.ts | 117 +++ .../import_manager/reuse_generated_imports.ts | 75 ++ .../reuse_source_file_imports.ts | 132 ++++ .../src/ngtsc/translator/test/BUILD.bazel | 3 + .../translator/test/import_manager_spec.ts | 719 ++++++++++++++++++ 13 files changed, 1464 insertions(+), 27 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/translator/src/import_manager/check_unique_identifier_name.ts create mode 100644 packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_manager.ts create mode 100644 packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_typescript_transform.ts create mode 100644 packages/compiler-cli/src/ngtsc/translator/src/import_manager/reuse_generated_imports.ts create mode 100644 packages/compiler-cli/src/ngtsc/translator/src/import_manager/reuse_source_file_imports.ts create mode 100644 packages/compiler-cli/src/ngtsc/translator/test/import_manager_spec.ts diff --git a/packages/compiler-cli/src/ngtsc/imports/index.ts b/packages/compiler-cli/src/ngtsc/imports/index.ts index cd1fdc93324ce..1f6ea639b9f6b 100644 --- a/packages/compiler-cli/src/ngtsc/imports/index.ts +++ b/packages/compiler-cli/src/ngtsc/imports/index.ts @@ -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'; diff --git a/packages/compiler-cli/src/ngtsc/imports/src/core.ts b/packages/compiler-cli/src/ngtsc/imports/src/core.ts index 5b67e66e905e1..1b3996394b044 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/core.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/core.ts @@ -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. @@ -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; } @@ -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. diff --git a/packages/compiler-cli/src/ngtsc/imports/src/patch_alias_reference_resolution.ts b/packages/compiler-cli/src/ngtsc/imports/src/patch_alias_reference_resolution.ts index c78e045f67804..182fdc81e88dd 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/patch_alias_reference_resolution.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/patch_alias_reference_resolution.ts @@ -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 @@ -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; + [patchedReferencedAliasesSymbol]?: Set; } /** @@ -95,9 +98,9 @@ export function loadIsReferencedAliasDeclarationPatch(context: ts.Transformation throwIncompatibleTransformationContextError(); } - const referencedAliases = new Set(); + const referencedAliases = new Set(); emitResolver.isReferencedAliasDeclaration = function(node, ...args) { - if (isAliasImportDeclaration(node) && referencedAliases.has(node)) { + if (isAliasImportDeclaration(node) && (referencedAliases as Set).has(node)) { return true; } return originalIsReferencedAliasDeclaration.call(emitResolver, node, ...args); @@ -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); } diff --git a/packages/compiler-cli/src/ngtsc/translator/index.ts b/packages/compiler-cli/src/ngtsc/translator/index.ts index 52258bd511661..8cc33918c8f02 100644 --- a/packages/compiler-cli/src/ngtsc/translator/index.ts +++ b/packages/compiler-cli/src/ngtsc/translator/index.ts @@ -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'; diff --git a/packages/compiler-cli/src/ngtsc/translator/src/api/import_generator.ts b/packages/compiler-cli/src/ngtsc/translator/src/api/import_generator.ts index 07a1bd22f5ffd..ec45efa763ef2 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/api/import_generator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/api/import_generator.ts @@ -17,6 +17,32 @@ export interface NamedImport { symbol: string; } +/** + * A request to import a given symbol from the given module. + */ +export interface ImportRequest { + /** + * 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. * diff --git a/packages/compiler-cli/src/ngtsc/translator/src/import_manager.ts b/packages/compiler-cli/src/ngtsc/translator/src/import_manager.ts index 0b1998d1167eb..72fca8de18be9 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/import_manager.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/import_manager.ts @@ -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. */ @@ -41,7 +39,7 @@ export interface SideEffectImport { */ export type Import = NamespaceImport|SideEffectImport; -export class ImportManager implements ImportGenerator { +export class ImportManager { private specifierToIdentifier = new Map(); private nextIndex = 0; @@ -60,7 +58,7 @@ export class ImportManager implements ImportGenerator { return this.specifierToIdentifier.get(moduleName)!; } - generateNamedImport(moduleName: string, originalSymbol: string): NamedImport { + generateNamedImport(moduleName: string, originalSymbol: string): any { // First, rewrite the symbol name. const symbol = this.rewriter.rewriteSymbol(originalSymbol, moduleName); diff --git a/packages/compiler-cli/src/ngtsc/translator/src/import_manager/check_unique_identifier_name.ts b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/check_unique_identifier_name.ts new file mode 100644 index 0000000000000..66aa37c14439d --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/check_unique_identifier_name.ts @@ -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; +} + +/** + * Generates a helper for `ImportManagerConfig` to generate unique identifiers + * for a given source file. + */ +export function createGenerateUniqueIdentifierHelper(): + ImportManagerConfig['generateUniqueIdentifier'] { + const generatedIdentifiers = new Set(); + + 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); + }; +} diff --git a/packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_manager.ts b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_manager.ts new file mode 100644 index 0000000000000..5c917c0bcb5c7 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_manager.ts @@ -0,0 +1,330 @@ +/** + * @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 {AliasImportDeclaration, ImportRewriter} from '../../../imports'; +import {ImportGenerator, ImportRequest} from '../api/import_generator'; + +import {createGenerateUniqueIdentifierHelper} from './check_unique_identifier_name'; +import {createTsTransformForImportManager} from './import_typescript_transform'; +import {attemptToReuseGeneratedImports, captureGeneratedImport, ReuseGeneratedImportsTracker} from './reuse_generated_imports'; +import {attemptToReuseExistingSourceFileImports, ReuseExistingSourceFileImportsTracker} from './reuse_source_file_imports'; + +/** Configuration for the import manager. */ +export interface ImportManagerConfig { + generateUniqueIdentifier(file: ts.SourceFile, baseName: string): ts.Identifier|null; + shouldUseSingleQuotes(file: ts.SourceFile): boolean; + rewriter: ImportRewriter|null; + namespaceImportPrefix: string; + disableOriginalSourceFileReuse: boolean; + forceGenerateNamespacesForNewImports: boolean; +} + +/** + * Preset configuration for forcing namespace imports. + * + * This preset is commonly used to avoid test differences to previous + * versions of the `ImportManager`. + */ +export const presetImportManagerForceNamespaceImports: Partial = { + // Forcing namespace imports also means no-reuse. + // Re-using would otherwise become more complicated and we don't + // expect re-usable namespace imports. + disableOriginalSourceFileReuse: true, + forceGenerateNamespacesForNewImports: true, +}; + +/** Branded string to identify a module name. */ +export type ModuleName = string&{__moduleName: boolean}; + +/** + * Import manager that can be used to conveniently and efficiently generate + * imports It efficiently re-uses existing source file imports, or previous + * generated imports. + * + * These capabilities are important for efficient TypeScript transforms that + * minimize structural changes to the dependency graph of source files, enabling + * as much incremental re-use as possible. + * + * Those imports may be inserted via a TypeScript transform, or via manual string + * manipulation using e.g. `magic-string`. + */ +export class ImportManagerV2 implements + ImportGenerator { + /** List of new imports that will be inserted into given source files. */ + private newImports: Map, + namedImports: Map, + sideEffectImports: Set, + }> = new Map(); + + private nextUniqueIndex = 0; + private config: ImportManagerConfig = { + shouldUseSingleQuotes: () => false, + rewriter: null, + disableOriginalSourceFileReuse: false, + forceGenerateNamespacesForNewImports: false, + namespaceImportPrefix: 'i', + generateUniqueIdentifier: this._config.generateUniqueIdentifier ?? + createGenerateUniqueIdentifierHelper(), + ...this._config, + }; + + private reuseSourceFileImportsTracker: ReuseExistingSourceFileImportsTracker = { + generateUniqueIdentifier: this.config.generateUniqueIdentifier, + reusedAliasDeclarations: new Set(), + updatedImports: new Map(), + }; + private reuseGeneratedImportsTracker: ReuseGeneratedImportsTracker = { + directReuseCache: new Map(), + namespaceImportReuseCache: new Map(), + }; + + constructor(private _config: Partial = {}) {} + + /** Adds a side-effect import for the given module. */ + addSideEffectImport(requestedFile: ts.SourceFile, moduleSpecifier: string) { + if (this.config.rewriter !== null) { + moduleSpecifier = + this.config.rewriter.rewriteSpecifier(moduleSpecifier, requestedFile.fileName); + } + + this._getNewImportsTrackerForFile(requestedFile) + .sideEffectImports.add(moduleSpecifier as ModuleName); + } + + /** + * Adds an import to the given source-file and returns a TypeScript + * expression that can be used to access the newly imported symbol. + */ + addImport(request: ImportRequest&{asTypeReference: true}): ts.Identifier + |ts.QualifiedName; + addImport(request: ImportRequest&{asTypeReference?: undefined}): ts.Identifier + |ts.PropertyAccessExpression; + addImport(request: ImportRequest&{asTypeReference?: boolean}): ts.Identifier + |ts.PropertyAccessExpression|ts.QualifiedName { + if (this.config.rewriter !== null) { + if (request.exportSymbolName !== null) { + request.exportSymbolName = this.config.rewriter.rewriteSymbol( + request.exportSymbolName, request.exportModuleSpecifier); + } + + request.exportModuleSpecifier = this.config.rewriter.rewriteSpecifier( + request.exportModuleSpecifier, request.requestedFile.fileName); + } + + // Attempt to re-use previous identical import requests. + const previousGeneratedImportRef = + attemptToReuseGeneratedImports(this.reuseGeneratedImportsTracker, request); + if (previousGeneratedImportRef !== null) { + return createImportReference(!!request.asTypeReference, previousGeneratedImportRef); + } + + // Generate a new one, and cache it. + const resultImportRef = this._generateNewImport(request); + captureGeneratedImport(request, this.reuseGeneratedImportsTracker, resultImportRef); + return createImportReference(!!request.asTypeReference, resultImportRef); + } + + private _generateNewImport(request: ImportRequest): ts.Identifier + |[ts.Identifier, ts.Identifier] { + const {requestedFile: sourceFile} = request; + const disableOriginalSourceFileReuse = this.config.disableOriginalSourceFileReuse; + const forceGenerateNamespacesForNewImports = this.config.forceGenerateNamespacesForNewImports; + + // If desired, attempt to re-use original source file imports as a base, or as much as possible. + // This may involve updates to existing import named bindings. + if (!disableOriginalSourceFileReuse) { + const reuseResult = attemptToReuseExistingSourceFileImports( + this.reuseSourceFileImportsTracker, sourceFile, request); + if (reuseResult !== null) { + return reuseResult; + } + } + + // A new import needs to be generated. + // No candidate existing import was found. + const {namedImports, namespaceImports} = this._getNewImportsTrackerForFile(sourceFile); + + // If a namespace import is requested, or the symbol should be forcibly + // imported through namespace imports: + if (request.exportSymbolName === null || forceGenerateNamespacesForNewImports) { + const namespaceImportName = `${this.config.namespaceImportPrefix}${this.nextUniqueIndex++}`; + const namespaceImport = ts.factory.createNamespaceImport( + this.config.generateUniqueIdentifier(sourceFile, namespaceImportName) ?? + ts.factory.createIdentifier(namespaceImportName)); + + namespaceImports.set(request.exportModuleSpecifier as ModuleName, namespaceImport); + + // Capture the generated namespace import alone, to allow re-use. + captureGeneratedImport( + {...request, exportSymbolName: null}, this.reuseGeneratedImportsTracker, + namespaceImport.name); + + if (request.exportSymbolName !== null) { + return [namespaceImport.name, ts.factory.createIdentifier(request.exportSymbolName)]; + } + return namespaceImport.name; + } + + // Otherwise, an individual named import is requested. + if (!namedImports.has(request.exportModuleSpecifier as ModuleName)) { + namedImports.set(request.exportModuleSpecifier as ModuleName, []); + } + + const exportSymbolName = ts.factory.createIdentifier(request.exportSymbolName); + const fileUniqueName = + this.config.generateUniqueIdentifier(sourceFile, request.exportSymbolName); + const needsAlias = fileUniqueName !== null; + const specifierName = needsAlias ? fileUniqueName : exportSymbolName; + + namedImports.get(request.exportModuleSpecifier as ModuleName)!.push( + ts.factory.createImportSpecifier( + false, needsAlias ? exportSymbolName : undefined, specifierName)); + + return specifierName; + } + + /** + * Finalizes the import manager by computing all necessary import changes + * and returning them. + * + * Changes are collected once at the end, after all imports are requested, + * because this simplifies building up changes to existing imports that need + * to be updated, and allows more trivial re-use of previous generated imports. + */ + finalize(): { + affectedFiles: Set, + updatedImports: Map, + newImports: Map, + reusedOriginalAliasDeclarations: Set, + } { + const affectedFiles = new Set(); + const updatedImportsResult = new Map(); + const newImportsResult = new Map(); + + const addNewImport = (fileName: string, importDecl: ts.ImportDeclaration) => { + affectedFiles.add(fileName); + if (newImportsResult.has(fileName)) { + newImportsResult.get(fileName)!.push(importDecl); + } else { + newImportsResult.set(fileName, [importDecl]); + } + }; + + // Collect original source file imports that need to be updated. + this.reuseSourceFileImportsTracker.updatedImports.forEach((expressions, importDecl) => { + const namedBindings = importDecl.importClause!.namedBindings as ts.NamedImports; + const newNamedBindings = ts.factory.updateNamedImports( + namedBindings, + namedBindings.elements.concat(expressions.map( + ({propertyName, fileUniqueAlias}) => ts.factory.createImportSpecifier( + false, fileUniqueAlias !== null ? propertyName : undefined, + fileUniqueAlias ?? propertyName)))); + + affectedFiles.add(importDecl.getSourceFile().fileName); + updatedImportsResult.set(namedBindings, newNamedBindings); + }); + + // Collect all new imports to be added. Named imports, namespace imports or side-effects. + this.newImports.forEach(({namedImports, namespaceImports, sideEffectImports}, sourceFile) => { + const useSingleQuotes = this.config.shouldUseSingleQuotes(sourceFile); + const fileName = sourceFile.fileName; + + sideEffectImports.forEach(moduleName => { + addNewImport( + fileName, + ts.factory.createImportDeclaration( + undefined, undefined, ts.factory.createStringLiteral(moduleName))); + }); + + namespaceImports.forEach((namespaceImport, moduleName) => { + const newImport = ts.factory.createImportDeclaration( + undefined, ts.factory.createImportClause(false, undefined, namespaceImport), + ts.factory.createStringLiteral(moduleName, useSingleQuotes)); + + // IMPORTANT: Set the original TS node to the `ts.ImportDeclaration`. This allows + // downstream transforms such as tsickle to properly process references to this import. + // + // This operation is load-bearing in g3 as some imported modules contain special metadata + // generated by clutz, which tsickle uses to transform imports and references to those + // imports. See: `google3: node_modules/tsickle/src/googmodule.ts;l=637-640;rcl=615418148` + ts.setOriginalNode(namespaceImport.name, newImport); + + addNewImport(fileName, newImport); + }); + + namedImports.forEach((specifiers, moduleName) => { + const newImport = ts.factory.createImportDeclaration( + undefined, + ts.factory.createImportClause( + false, undefined, ts.factory.createNamedImports(specifiers)), + ts.factory.createStringLiteral(moduleName, useSingleQuotes)); + + addNewImport(fileName, newImport); + }); + }); + + return { + affectedFiles, + newImports: newImportsResult, + updatedImports: updatedImportsResult, + reusedOriginalAliasDeclarations: this.reuseSourceFileImportsTracker.reusedAliasDeclarations, + }; + } + + /** + * Gets a TypeScript transform for the import manager. + * + * @param extraStatementsMap Additional set of statements to be inserted + * for given source files after their imports. E.g. top-level constants. + */ + toTsTransform(extraStatementsMap?: Map): + ts.TransformerFactory { + return createTsTransformForImportManager(this, extraStatementsMap); + } + + /** + * Transforms a single file as a shorthand, using {@link toTsTransform}. + * + * @param extraStatementsMap Additional set of statements to be inserted + * for given source files after their imports. E.g. top-level constants. + */ + transformTsFile( + ctx: ts.TransformationContext, file: ts.SourceFile, + extraStatementsAfterImports?: ts.Statement[]): ts.SourceFile { + const extraStatementsMap = extraStatementsAfterImports ? + new Map([[file.fileName, extraStatementsAfterImports]]) : + undefined; + return this.toTsTransform(extraStatementsMap)(ctx)(file); + } + + private _getNewImportsTrackerForFile(file: ts.SourceFile): + NonNullable> { + if (!this.newImports.has(file)) { + this.newImports.set(file, { + namespaceImports: new Map(), + namedImports: new Map(), + sideEffectImports: new Set(), + }); + } + return this.newImports.get(file)!; + } +} + +/** Creates an import reference based on the given identifier, or nested access. */ +function createImportReference( + asTypeReference: boolean, ref: ts.Identifier|[ts.Identifier, ts.Identifier]): ts.Identifier| + ts.QualifiedName|ts.PropertyAccessExpression { + if (asTypeReference) { + return Array.isArray(ref) ? ts.factory.createQualifiedName(ref[0], ref[1]) : ref; + } else { + return Array.isArray(ref) ? ts.factory.createPropertyAccessExpression(ref[0], ref[1]) : ref; + } +} diff --git a/packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_typescript_transform.ts b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_typescript_transform.ts new file mode 100644 index 0000000000000..441c4bf6920af --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_typescript_transform.ts @@ -0,0 +1,117 @@ +/** + * @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 {loadIsReferencedAliasDeclarationPatch} from '../../../imports'; + +import type {ImportManagerV2} from './import_manager'; + +/** + * Creates a TypeScript transform for the given import manager. + * + * - The transform updates existing imports with new symbols to be added. + * - The transform adds new necessary imports. + * - The transform inserts additional optional statements after imports. + */ +export function createTsTransformForImportManager( + manager: ImportManagerV2, + extraStatementsForFiles?: Map): ts.TransformerFactory { + return (ctx) => { + const {affectedFiles, newImports, updatedImports, reusedOriginalAliasDeclarations} = + manager.finalize(); + + // If we re-used existing source file alias declarations, mark those as referenced so TypeScript + // doesn't drop these thinking they are unused. + if (reusedOriginalAliasDeclarations.size > 0) { + const referencedAliasDeclarations = loadIsReferencedAliasDeclarationPatch(ctx); + reusedOriginalAliasDeclarations.forEach( + aliasDecl => referencedAliasDeclarations.add(aliasDecl)); + } + + // Update the set of affected files to include files that need extra statements to be inserted. + if (extraStatementsForFiles !== undefined) { + for (const [fileName, statements] of extraStatementsForFiles.entries()) { + if (statements.length > 0) { + affectedFiles.add(fileName); + } + } + } + + const visitStatement: ts.Visitor = (node) => { + if (!ts.isImportDeclaration(node) || node.importClause === undefined || + !ts.isImportClause(node.importClause)) { + return node; + } + + const clause = node.importClause; + if (clause.namedBindings === undefined || !ts.isNamedImports(clause.namedBindings) || + !updatedImports.has(clause.namedBindings)) { + return node; + } + + const newClause = ctx.factory.updateImportClause( + clause, clause.isTypeOnly, clause.name, updatedImports.get(clause.namedBindings)); + const newImport = ctx.factory.updateImportDeclaration( + node, node.modifiers, newClause, node.moduleSpecifier, node.attributes); + + // This tricks TypeScript into thinking that the `importClause` is still optimizable. + // By default, TS assumes, no specifiers are elide-able if the clause of the "original + // node" has changed. google3: + // typescript/unstable/src/compiler/transformers/ts.ts;l=456;rcl=611254538. + ts.setOriginalNode( + newImport, + {importClause: newClause, kind: newImport.kind} as Partialas any); + + return newImport; + }; + + return sourceFile => { + if (!affectedFiles.has(sourceFile.fileName)) { + return sourceFile; + } + + sourceFile = ts.visitEachChild(sourceFile, visitStatement, ctx); + + // Filter out the existing imports and the source file body. + // All new statements will be inserted between them. + const extraStatements = extraStatementsForFiles?.get(sourceFile.fileName) ?? []; + const existingImports: ts.Statement[] = []; + const body: ts.Statement[] = []; + + for (const statement of sourceFile.statements) { + if (isImportStatement(statement)) { + existingImports.push(statement); + } else { + body.push(statement); + } + } + + return ctx.factory.updateSourceFile( + sourceFile, + [ + ...existingImports, + ...(newImports.get(sourceFile.fileName) ?? []), + ...extraStatements, + ...body, + ], + sourceFile.isDeclarationFile, + sourceFile.referencedFiles, + sourceFile.typeReferenceDirectives, + sourceFile.hasNoDefaultLib, + sourceFile.libReferenceDirectives, + ); + }; + }; +} + +/** Whether the given statement is an import statement. */ +function isImportStatement(stmt: ts.Statement): boolean { + return ts.isImportDeclaration(stmt) || ts.isImportEqualsDeclaration(stmt) || + ts.isNamespaceImport(stmt); +} diff --git a/packages/compiler-cli/src/ngtsc/translator/src/import_manager/reuse_generated_imports.ts b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/reuse_generated_imports.ts new file mode 100644 index 0000000000000..48cb07340fb0e --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/reuse_generated_imports.ts @@ -0,0 +1,75 @@ +/** + * @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 {ImportRequest} from '../api/import_generator'; + +import type {ModuleName} from './import_manager'; + +/** Branded string identifying a hashed {@link ImportRequest}. */ +type ImportRequestHash = string&{__importHash: string}; + +/** Tracker capturing re-used generated imports. */ +export interface ReuseGeneratedImportsTracker { + /** + * Map of previously resolved symbol imports. Cache can be re-used to return + * the same identifier without checking the source-file again. + */ + directReuseCache: Map; + + /** + * Map of module names and their potential namespace import + * identifiers. Allows efficient re-using of namespace imports. + */ + namespaceImportReuseCache: Map; +} + +/** Attempts to efficiently re-use previous generated import requests. */ +export function attemptToReuseGeneratedImports( + tracker: ReuseGeneratedImportsTracker, + request: ImportRequest): ts.Identifier|[ts.Identifier, ts.Identifier]|null { + const requestHash = hashImportRequest(request); + + // In case the given import has been already generated previously, we just return + // the previous generated identifier in order to avoid duplicate generated imports. + const existingExactImport = tracker.directReuseCache.get(requestHash); + if (existingExactImport !== undefined) { + return existingExactImport; + } + + const potentialNamespaceImport = + tracker.namespaceImportReuseCache.get(request.exportModuleSpecifier as ModuleName); + if (potentialNamespaceImport === undefined) { + return null; + } + + if (request.exportSymbolName === null) { + return potentialNamespaceImport; + } + + return [potentialNamespaceImport, ts.factory.createIdentifier(request.exportSymbolName)]; +} + +/** Captures the given import request and its generated reference node/path for future re-use. */ +export function captureGeneratedImport( + request: ImportRequest, tracker: ReuseGeneratedImportsTracker, + referenceNode: ts.Identifier|[ts.Identifier, ts.Identifier]) { + tracker.directReuseCache.set(hashImportRequest(request), referenceNode); + + if (request.exportSymbolName === null && !Array.isArray(referenceNode)) { + tracker.namespaceImportReuseCache.set( + request.exportModuleSpecifier as ModuleName, referenceNode); + } +} + +/** Generates a unique hash for the given import request. */ +function hashImportRequest(req: ImportRequest): ImportRequestHash { + return `${req.requestedFile.fileName}:${req.exportModuleSpecifier}:${req.exportSymbolName}` as + ImportRequestHash; +} diff --git a/packages/compiler-cli/src/ngtsc/translator/src/import_manager/reuse_source_file_imports.ts b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/reuse_source_file_imports.ts new file mode 100644 index 0000000000000..b12dde8259fd1 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/reuse_source_file_imports.ts @@ -0,0 +1,132 @@ +/** + * @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 {AliasImportDeclaration} from '../../../imports'; +import {ImportRequest} from '../api/import_generator'; + +/** + * Tracker necessary for enabling re-use of original source file imports. + * + * The tracker holds information about original source file imports that + * need to be updated, or import declarations/specifiers that are now + * referenced due to the import manager. + */ +export interface ReuseExistingSourceFileImportsTracker { + /** + * Map of import declarations that need to be updated to include the + * given symbols. + */ + updatedImports: + Map; + + /** + * Set of re-used alias import declarations. + * + * These are captured so that we can tell TypeScript to not elide these source file + * imports as we introduced references to them. More details: {@link AliasImportDeclaration}. + */ + reusedAliasDeclarations: Set; + + /** Generates a unique identifier for a name in the given file. */ + generateUniqueIdentifier(file: ts.SourceFile, symbolName: string): ts.Identifier|null; +} + +/** Attempts to re-use original source file imports for the given request. */ +export function attemptToReuseExistingSourceFileImports( + tracker: ReuseExistingSourceFileImportsTracker, sourceFile: ts.SourceFile, + request: ImportRequest): ts.Identifier|[ts.Identifier, ts.Identifier]|null { + // Walk through all source-file top-level statements and search for import declarations + // that already match the specified "moduleName" and can be updated to import the + // given symbol. If no matching import can be found, the last import in the source-file + // will be used as starting point for a new import that will be generated. + let candidateImportToBeUpdated: ts.ImportDeclaration|null = null; + + for (let i = sourceFile.statements.length - 1; i >= 0; i--) { + const statement = sourceFile.statements[i]; + + if (!ts.isImportDeclaration(statement) || !ts.isStringLiteral(statement.moduleSpecifier)) { + continue; + } + + // Side-effect imports are ignored, or type-only imports. + // TODO: Consider re-using type-only imports efficiently. + if (!statement.importClause || statement.importClause.isTypeOnly) { + continue; + } + + const moduleSpecifier = statement.moduleSpecifier.text; + // If the import does not match the module name, or requested target file, continue. + // Note: In the future, we may consider performing better analysis here. E.g. resolve paths, + // or try to detect re-usable symbols via type-checking. + if (moduleSpecifier !== request.exportModuleSpecifier) { + continue; + } + + if (statement.importClause.namedBindings) { + const namedBindings = statement.importClause.namedBindings; + + // A namespace import can be reused. + if (ts.isNamespaceImport(namedBindings)) { + tracker.reusedAliasDeclarations.add(namedBindings); + + if (request.exportSymbolName === null) { + return namedBindings.name; + } + + return [namedBindings.name, ts.factory.createIdentifier(request.exportSymbolName)]; + } + + // Named imports can be re-used if a specific symbol is requested. + if (ts.isNamedImports(namedBindings) && request.exportSymbolName !== null) { + const existingElement = namedBindings.elements.find(e => { + // TODO: Consider re-using type-only imports efficiently. + return !e.isTypeOnly && + (e.propertyName ? e.propertyName.text === request.exportSymbolName : + e.name.text === request.exportSymbolName); + }); + + if (existingElement !== undefined) { + tracker.reusedAliasDeclarations.add(existingElement); + return existingElement.name; + } + + // In case the symbol could not be found in an existing import, we + // keep track of the import declaration as it can be updated to include + // the specified symbol name without having to create a new import. + candidateImportToBeUpdated = statement; + } + } + } + + if (candidateImportToBeUpdated === null || request.exportSymbolName === null) { + return null; + } + + // We have a candidate import. Update it to import what we need. + if (!tracker.updatedImports.has(candidateImportToBeUpdated)) { + tracker.updatedImports.set(candidateImportToBeUpdated, []); + } + const symbolsToBeImported = tracker.updatedImports.get(candidateImportToBeUpdated)!; + const propertyName = ts.factory.createIdentifier(request.exportSymbolName); + const fileUniqueAlias = tracker.generateUniqueIdentifier(sourceFile, request.exportSymbolName); + + // Since it can happen that multiple classes need to be imported within the + // specified source file and we want to add the identifiers to the existing + // import declaration, we need to keep track of the updated import declarations. + // We can't directly update the import declaration for each identifier as this + // would not be reflected in the AST— or would throw of update recording offsets. + symbolsToBeImported.push({ + propertyName, + fileUniqueAlias, + }); + + return fileUniqueAlias ?? propertyName; +} diff --git a/packages/compiler-cli/src/ngtsc/translator/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/translator/test/BUILD.bazel index d03795a861fac..58e79860f380b 100644 --- a/packages/compiler-cli/src/ngtsc/translator/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/translator/test/BUILD.bazel @@ -11,6 +11,9 @@ ts_library( deps = [ "//packages:types", "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/file_system", + "//packages/compiler-cli/src/ngtsc/file_system/testing", + "//packages/compiler-cli/src/ngtsc/testing", "//packages/compiler-cli/src/ngtsc/translator", "@npm//typescript", ], diff --git a/packages/compiler-cli/src/ngtsc/translator/test/import_manager_spec.ts b/packages/compiler-cli/src/ngtsc/translator/test/import_manager_spec.ts new file mode 100644 index 0000000000000..6e848c657ce4c --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/translator/test/import_manager_spec.ts @@ -0,0 +1,719 @@ +/** + * @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 {absoluteFrom} from '../../file_system'; +import {initMockFileSystem} from '../../file_system/testing'; +import {NgtscTestCompilerHost} from '../../testing'; +import {ImportManagerV2} from '../src/import_manager/import_manager'; + +describe('import manager', () => { + it('should be possible to import a symbol', () => { + const {testFile, emit} = createTestProgram(''); + const manager = new ImportManagerV2(); + const ref = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(ref), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { input } from "@angular/core"; + input; + `)); + }); + + it('should be possible to import a namespace', () => { + const {testFile, emit} = createTestProgram(''); + const manager = new ImportManagerV2(); + + const ref = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: null, + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(ref), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import * as i0 from "@angular/core"; + i0; + `)); + }); + + it('should be possible to import multiple symbols', () => { + const {testFile, emit} = createTestProgram(''); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const outputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'output', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ts.factory.createExpressionStatement(outputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { input, output } from "@angular/core"; + input; + output; + `)); + }); + + it('should be possible to import multiple namespaces', () => { + const {testFile, emit} = createTestProgram(''); + const manager = new ImportManagerV2(); + + const coreNamespace = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: null, + requestedFile: testFile, + }); + + const interopNamespace = manager.addImport({ + exportModuleSpecifier: '@angular/core/rxjs-interop', + exportSymbolName: null, + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(coreNamespace), + ts.factory.createExpressionStatement(interopNamespace), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import * as i0 from "@angular/core"; + import * as i1 from "@angular/core/rxjs-interop"; + i0; + i1; + `)); + }); + + it('should be possible to generate a namespace import and re-use it for future symbols', () => { + const {testFile, emit} = createTestProgram(''); + const manager = new ImportManagerV2(); + + const coreNamespace = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: null, + requestedFile: testFile, + }); + + const outputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'output', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(coreNamespace), + ts.factory.createExpressionStatement(outputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import * as i0 from "@angular/core"; + i0; + i0.output; + `)); + }); + + it('should always generate a new namespace import if there is only a named import', () => { + const {testFile, emit} = createTestProgram(''); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const coreNamespace = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: null, + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ts.factory.createExpressionStatement(coreNamespace), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import * as i0 from "@angular/core"; + import { input } from "@angular/core"; + input; + i0; + `)); + }); + + it('should be able to re-use existing source file namespace imports for symbols', () => { + const {testFile, emit} = createTestProgram(` + import * as existingImport from '@angular/core'; + `); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import * as existingImport from '@angular/core'; + existingImport.input; + `)); + }); + + it('should re-use existing source file namespace imports for a namespace request', () => { + const {testFile, emit} = createTestProgram(` + import * as existingImport from '@angular/core'; + `); + const manager = new ImportManagerV2(); + + const coreRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: null, + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(coreRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import * as existingImport from '@angular/core'; + existingImport; + `)); + }); + + it('should be able to re-use existing source named bindings', () => { + const {testFile, emit} = createTestProgram(` + import {input} from '@angular/core'; + `); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { input } from '@angular/core'; + input; + `)); + }); + + it('should be able to add symbols to an existing source file named import', () => { + const {testFile, emit} = createTestProgram(` + import {input} from '@angular/core'; + + const x = input(); + `); + const manager = new ImportManagerV2(); + + const outputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'output', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(outputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { input, output } from '@angular/core'; + output; + const x = input(); + `)); + }); + + it('should be able to add symbols to an existing source file named import, ' + + 'while still eliding unused specifiers of the updated import', + () => { + const {testFile, emit} = createTestProgram(` + import {input} from '@angular/core'; + `); + const manager = new ImportManagerV2(); + + const outputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'output', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(outputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { output } from '@angular/core'; + output; + `)); + }); + + it('should not re-use an original file import if re-use is disabled', () => { + const {testFile, emit} = createTestProgram(` + import {input} from '@angular/core'; + `); + const manager = new ImportManagerV2({ + disableOriginalSourceFileReuse: true, + }); + + const outputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'output', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(outputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { output } from "@angular/core"; + output; + `)); + }); + + it('should not re-use an original namespace import if re-use is disabled', () => { + const {testFile, emit} = createTestProgram(` + import * as existingCore from '@angular/core'; + `); + const manager = new ImportManagerV2({ + disableOriginalSourceFileReuse: true, + }); + + const outputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'output', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(outputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { output } from "@angular/core"; + output; + `)); + }); + + it('should be able to always prefer namespace imports for new imports', () => { + const {testFile, emit} = createTestProgram(``); + const manager = new ImportManagerV2({ + forceGenerateNamespacesForNewImports: true, + }); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const outputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'output', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ts.factory.createExpressionStatement(outputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import * as i0 from "@angular/core"; + i0.input; + i0.output; + `)); + }); + + it('should be able to always prefer namespace imports for new imports, ' + + 'but still re-use source file namespace imports', + () => { + const {testFile, emit} = createTestProgram(` + import * as existingNamespace from '@angular/core'; + `); + const manager = new ImportManagerV2({ + forceGenerateNamespacesForNewImports: true, + }); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const outputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'output', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ts.factory.createExpressionStatement(outputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import * as existingNamespace from '@angular/core'; + existingNamespace.input; + existingNamespace.output; + `)); + }); + + it('should be able to always prefer namespace imports for new imports, ' + + 'but still re-use source file individual imports', + () => { + const {testFile, emit} = createTestProgram(` + import {Dir} from 'bla'; + + const x = new Dir(); + `); + const manager = new ImportManagerV2({ + forceGenerateNamespacesForNewImports: true, + }); + + const blaRef = manager.addImport({ + exportModuleSpecifier: 'bla', + exportSymbolName: 'Dir', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(blaRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { Dir } from 'bla'; + Dir; + const x = new Dir(); + `)); + }); + + it('should not change existing unrelated imports', () => { + const {testFile, emit} = createTestProgram(` + import {MyComp} from './bla'; + + console.log(MyComp); + `); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { MyComp } from './bla'; + import { input } from "@angular/core"; + input; + console.log(MyComp); + `)); + }); + + it('should be able to add a side effect import', () => { + const {testFile, emit} = createTestProgram(``); + const manager = new ImportManagerV2(); + + manager.addSideEffectImport(testFile, '@angular/core'); + + const res = emit(manager, []); + + expect(res).toBe(omitLeadingWhitespace(` + import "@angular/core"; + `)); + }); + + it('should avoid conflicts with existing top-level identifiers', () => { + const {testFile, emit} = createTestProgram(` + const input = 1; + `); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { input as input_1 } from "@angular/core"; + input_1; + const input = 1; + `)); + }); + + it('should avoid conflicts with existing deep identifiers', () => { + const {testFile, emit} = createTestProgram(` + function x() { + const p = () => { + const input = 1; + }; + } + `); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { input as input_1 } from "@angular/core"; + input_1; + function x() { + const p = () => { + const input = 1; + }; + } + `)); + }); + + it('should avoid an import alias specifier if identifier is free to use', () => { + const {testFile, emit} = createTestProgram(``); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { input } from "@angular/core"; + input; + `)); + }); + + it('should avoid collisions with generated identifiers', () => { + const {testFile, emit} = createTestProgram(``); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const inputRef2 = manager.addImport({ + exportModuleSpecifier: '@angular/core2', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ts.factory.createExpressionStatement(inputRef2), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { input } from "@angular/core"; + import { input as input_1 } from "@angular/core2"; + input; + input_1; + `)); + }); + + it('should avoid collisions with generated identifiers', () => { + const {testFile, emit} = createTestProgram(``); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const inputRef2 = manager.addImport({ + exportModuleSpecifier: '@angular/core2', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ts.factory.createExpressionStatement(inputRef2), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { input } from "@angular/core"; + import { input as input_1 } from "@angular/core2"; + input; + input_1; + `)); + }); + + it('should re-use previous similar generated imports', () => { + const {testFile, emit} = createTestProgram(``); + const manager = new ImportManagerV2(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const inputRef2 = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ts.factory.createExpressionStatement(inputRef2), + ]); + + expect(inputRef).toBe(inputRef2); + expect(res).toBe(omitLeadingWhitespace(` + import { input } from "@angular/core"; + input; + input; + `)); + }); + + it('should not re-use original source file type-only imports', () => { + const {testFile, emit} = createTestProgram(` + import type {input} from '@angular/core'; + `); + const manager = new ImportManager(); + + const ref = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'bla', + requestedFile: testFile, + }); + const res = emit(manager, [ + ts.factory.createExpressionStatement(ref), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { bla } from "@angular/core"; + bla; + `)); + }); + + it('should not re-use original source file type-only import specifiers', () => { + const {testFile, emit} = createTestProgram(` + import {type input} from '@angular/core'; // existing. + `); + const manager = new ImportManager(); + + const ref = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'bla', + requestedFile: testFile, + }); + const res = emit(manager, [ + ts.factory.createExpressionStatement(ref), + ]); + + expect(res).toBe(omitLeadingWhitespace(` + import { bla } from '@angular/core'; // existing. + bla; + `)); + }); +}); + +function createTestProgram(text: string): { + testFile: ts.SourceFile, + emit: (manager: ImportManagerV2, extraStatements: ts.Statement[]) => string, +} { + const fs = initMockFileSystem('Native'); + const options: ts.CompilerOptions = { + rootDir: '/', + module: ts.ModuleKind.ESNext, + target: ts.ScriptTarget.ESNext, + skipLibCheck: true, + types: [], + }; + + fs.ensureDir(absoluteFrom('/')); + fs.writeFile(absoluteFrom('/test.ts'), text); + + const program = ts.createProgram({ + rootNames: ['/test.ts'], + options, + host: new NgtscTestCompilerHost(fs, options), + }); + + const testFile = program.getSourceFile('/test.ts'); + if (testFile === undefined) { + throw new Error('Could not get test source file from program.'); + } + + const emit = (manager: ImportManagerV2, newStatements: ts.Statement[]) => { + const transformer = manager.toTsTransform(new Map([[testFile.fileName, newStatements]])); + + let emitResult: string|null = null; + const {emitSkipped} = program.emit(testFile, (fileName, resultText) => { + if (fileName === '/test.js') { + emitResult = resultText; + } + }, undefined, undefined, {before: [transformer]}); + + if (emitSkipped || emitResult === null) { + throw new Error(`Unexpected emit failure when emitting test file.`); + } + + return omitLeadingWhitespace(emitResult); + }; + + return {testFile, emit}; +} + +/** Omits the leading whitespace for each line of the given text. */ +function omitLeadingWhitespace(text: string): string { + return text.replace(/^\s+/gm, ''); +} From ec8831ad0767bb4729142d22111870f491640fa3 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 11 Mar 2024 14:41:48 +0000 Subject: [PATCH 02/10] refactor(compiler-cli): update `ImportGenerator` abstraction for new manager `ImportGenerator` is the abstraction used by the translator functions to insert imports for `ExternalExpr` in an AST-agnostic way. This was built specifically for the linker which does not use any of the complex import managers- but rather re-uses `ngImport` or uses `ngImport.Bla`. This commit also switches the linker AST-agnostic generator to follow the new signatures. This was rather trivial. --- .../src/file_linker/emit_scopes/emit_scope.ts | 6 +-- .../linker/src/file_linker/translator.ts | 10 ++-- .../linker/src/linker_import_generator.ts | 22 ++++---- .../test/file_linker/translator_spec.ts | 28 +++++----- .../test/linker_import_generator_spec.ts | 51 ++++++++++++++----- .../annotations/common/test/metadata_spec.ts | 2 +- .../src/ngtsc/transform/src/transform.ts | 12 +++-- .../translator/src/api/import_generator.ts | 16 +----- .../src/ngtsc/translator/src/translator.ts | 36 ++++++------- .../ngtsc/translator/src/type_translator.ts | 18 +++---- .../translator/src/typescript_translator.ts | 16 +++--- .../src/ngtsc/typecheck/src/environment.ts | 2 +- .../src/reference_emit_environment.ts | 4 +- 13 files changed, 125 insertions(+), 98 deletions(-) diff --git a/packages/compiler-cli/linker/src/file_linker/emit_scopes/emit_scope.ts b/packages/compiler-cli/linker/src/file_linker/emit_scopes/emit_scope.ts index 061ed6aa72afe..dd22af73defed 100644 --- a/packages/compiler-cli/linker/src/file_linker/emit_scopes/emit_scope.ts +++ b/packages/compiler-cli/linker/src/file_linker/emit_scopes/emit_scope.ts @@ -36,7 +36,7 @@ export class EmitScope { */ translateDefinition(definition: LinkedDefinition): TExpression { const expression = this.translator.translateExpression( - definition.expression, new LinkerImportGenerator(this.ngImport)); + definition.expression, new LinkerImportGenerator(this.factory, this.ngImport)); if (definition.statements.length > 0) { // Definition statements must be emitted "after" the declaration for which the definition is @@ -44,7 +44,7 @@ export class EmitScope { // insert statements after definitions. To work around this, the linker transforms the // definition into an IIFE which executes the definition statements before returning the // definition expression. - const importGenerator = new LinkerImportGenerator(this.ngImport); + const importGenerator = new LinkerImportGenerator(this.factory, this.ngImport); return this.wrapInIifeWithStatements( expression, definition.statements.map( @@ -59,7 +59,7 @@ export class EmitScope { * Return any constant statements that are shared between all uses of this `EmitScope`. */ getConstantStatements(): TStatement[] { - const importGenerator = new LinkerImportGenerator(this.ngImport); + const importGenerator = new LinkerImportGenerator(this.factory, this.ngImport); return this.constantPool.statements.map( statement => this.translator.translateStatement(statement, importGenerator)); } diff --git a/packages/compiler-cli/linker/src/file_linker/translator.ts b/packages/compiler-cli/linker/src/file_linker/translator.ts index 60fc9a33ebfb8..2ddc078a05bf5 100644 --- a/packages/compiler-cli/linker/src/file_linker/translator.ts +++ b/packages/compiler-cli/linker/src/file_linker/translator.ts @@ -20,10 +20,11 @@ export class Translator { * Translate the given output AST in the context of an expression. */ translateExpression( - expression: o.Expression, imports: ImportGenerator, + expression: o.Expression, imports: ImportGenerator, options: TranslatorOptions = {}): TExpression { return expression.visitExpression( - new ExpressionTranslatorVisitor(this.factory, imports, options), + new ExpressionTranslatorVisitor( + this.factory, imports, null, options), new Context(false)); } @@ -31,10 +32,11 @@ export class Translator { * Translate the given output AST in the context of a statement. */ translateStatement( - statement: o.Statement, imports: ImportGenerator, + statement: o.Statement, imports: ImportGenerator, options: TranslatorOptions = {}): TStatement { return statement.visitStatement( - new ExpressionTranslatorVisitor(this.factory, imports, options), + new ExpressionTranslatorVisitor( + this.factory, imports, null, options), new Context(true)); } } diff --git a/packages/compiler-cli/linker/src/linker_import_generator.ts b/packages/compiler-cli/linker/src/linker_import_generator.ts index af477dc555752..32ef19f546b55 100644 --- a/packages/compiler-cli/linker/src/linker_import_generator.ts +++ b/packages/compiler-cli/linker/src/linker_import_generator.ts @@ -5,7 +5,7 @@ * 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 {ImportGenerator, NamedImport} from '../../src/ngtsc/translator'; +import {AstFactory, ImportGenerator, ImportRequest} from '../../src/ngtsc/translator'; import {FatalLinkerError} from './fatal_linker_error'; @@ -17,17 +17,19 @@ import {FatalLinkerError} from './fatal_linker_error'; * must be achieved by property access on an `ng` namespace identifier, which is passed in via the * constructor. */ -export class LinkerImportGenerator implements ImportGenerator { - constructor(private ngImport: TExpression) {} - - generateNamespaceImport(moduleName: string): TExpression { - this.assertModuleName(moduleName); - return this.ngImport; +export class LinkerImportGenerator implements + ImportGenerator { + constructor(private factory: AstFactory, private ngImport: TExpression) { } - generateNamedImport(moduleName: string, originalSymbol: string): NamedImport { - this.assertModuleName(moduleName); - return {moduleImport: this.ngImport, symbol: originalSymbol}; + addImport(request: ImportRequest): TExpression { + this.assertModuleName(request.exportModuleSpecifier); + + if (request.exportSymbolName === null) { + return this.ngImport; + } + + return this.factory.createPropertyAccess(this.ngImport, request.exportSymbolName); } private assertModuleName(moduleName: string): void { diff --git a/packages/compiler-cli/linker/test/file_linker/translator_spec.ts b/packages/compiler-cli/linker/test/file_linker/translator_spec.ts index 30f35b0c63728..3fd9478722ac8 100644 --- a/packages/compiler-cli/linker/test/file_linker/translator_spec.ts +++ b/packages/compiler-cli/linker/test/file_linker/translator_spec.ts @@ -6,21 +6,30 @@ * found in the LICENSE file at https://angular.io/license */ import * as o from '@angular/compiler'; -import {ImportGenerator, NamedImport, TypeScriptAstFactory} from '@angular/compiler-cli/src/ngtsc/translator'; +import {TypeScriptAstFactory} from '@angular/compiler-cli/src/ngtsc/translator'; import ts from 'typescript'; import {Translator} from '../../src/file_linker/translator'; +import {LinkerImportGenerator} from '../../src/linker_import_generator'; + import {generate} from './helpers'; +const ngImport = ts.factory.createIdentifier('ngImport'); + describe('Translator', () => { let factory: TypeScriptAstFactory; - beforeEach(() => factory = new TypeScriptAstFactory(/* annotateForClosureCompiler */ false)); + let importGenerator: LinkerImportGenerator; + + beforeEach(() => { + factory = new TypeScriptAstFactory(/* annotateForClosureCompiler */ false); + importGenerator = new LinkerImportGenerator(factory, ngImport); + }); describe('translateExpression()', () => { it('should generate expression specific output', () => { const translator = new Translator(factory); const outputAst = new o.WriteVarExpr('foo', new o.LiteralExpr(42)); - const translated = translator.translateExpression(outputAst, new MockImportGenerator()); + const translated = translator.translateExpression(outputAst, importGenerator); expect(generate(translated)).toEqual('(foo = 42)'); }); }); @@ -29,19 +38,8 @@ describe('Translator', () => { it('should generate statement specific output', () => { const translator = new Translator(factory); const outputAst = new o.ExpressionStatement(new o.WriteVarExpr('foo', new o.LiteralExpr(42))); - const translated = translator.translateStatement(outputAst, new MockImportGenerator()); + const translated = translator.translateStatement(outputAst, importGenerator); expect(generate(translated)).toEqual('foo = 42;'); }); }); - class MockImportGenerator implements ImportGenerator { - generateNamespaceImport(moduleName: string): ts.Expression { - return factory.createLiteral(moduleName); - } - generateNamedImport(moduleName: string, originalSymbol: string): NamedImport { - return { - moduleImport: factory.createLiteral(moduleName), - symbol: originalSymbol, - }; - } - } }); diff --git a/packages/compiler-cli/linker/test/linker_import_generator_spec.ts b/packages/compiler-cli/linker/test/linker_import_generator_spec.ts index 3202ada682d09..ac81dd16fee5e 100644 --- a/packages/compiler-cli/linker/test/linker_import_generator_spec.ts +++ b/packages/compiler-cli/linker/test/linker_import_generator_spec.ts @@ -5,37 +5,62 @@ * 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 {TypeScriptAstFactory} from '../../src/ngtsc/translator'; import {LinkerImportGenerator} from '../src/linker_import_generator'; -const ngImport = { - ngImport: true -}; +const ngImport = ts.factory.createIdentifier('ngImport'); describe('LinkerImportGenerator', () => { describe('generateNamespaceImport()', () => { it('should error if the import is not `@angular/core`', () => { - const generator = new LinkerImportGenerator(ngImport); - expect(() => generator.generateNamespaceImport('other/import')) + const generator = new LinkerImportGenerator( + new TypeScriptAstFactory(false), ngImport); + + expect( + () => generator.addImport( + {exportModuleSpecifier: 'other/import', exportSymbolName: null, requestedFile: null})) .toThrowError(`Unable to import from anything other than '@angular/core'`); }); it('should return the ngImport expression for `@angular/core`', () => { - const generator = new LinkerImportGenerator(ngImport); - expect(generator.generateNamespaceImport('@angular/core')).toBe(ngImport); + const generator = new LinkerImportGenerator( + new TypeScriptAstFactory(false), ngImport); + + expect(generator.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: null, + requestedFile: null + })).toBe(ngImport); }); }); describe('generateNamedImport()', () => { it('should error if the import is not `@angular/core`', () => { - const generator = new LinkerImportGenerator(ngImport); - expect(() => generator.generateNamedImport('other/import', 'someSymbol')) - .toThrowError(`Unable to import from anything other than '@angular/core'`); + const generator = new LinkerImportGenerator( + new TypeScriptAstFactory(false), ngImport); + + expect(() => generator.addImport({ + exportModuleSpecifier: 'other/import', + exportSymbolName: 'someSymbol', + requestedFile: null, + })).toThrowError(`Unable to import from anything other than '@angular/core'`); }); it('should return a `NamedImport` object containing the ngImport expression', () => { - const generator = new LinkerImportGenerator(ngImport); - expect(generator.generateNamedImport('@angular/core', 'someSymbol')) - .toEqual({moduleImport: ngImport, symbol: 'someSymbol'}); + const generator = new LinkerImportGenerator( + new TypeScriptAstFactory(false), ngImport); + + const result = generator.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'someSymbol', + requestedFile: null, + }); + + expect(ts.isPropertyAccessExpression(result)).toBe(true); + expect((result as ts.PropertyAccessExpression).name.text).toBe('someSymbol'); + expect((result as ts.PropertyAccessExpression).expression).toBe(ngImport); }); }); }); diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts index d36c76b89f2de..2623952c3cdd7 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts @@ -135,7 +135,7 @@ runInEachFileSystem(() => { const sf = getSourceFileOrError(program, _('/index.ts')); const im = new ImportManager(new NoopImportRewriter(), 'i'); const stmt = compileClassMetadata(call).toStmt(); - const tsStatement = translateStatement(stmt, im); + const tsStatement = translateStatement(sf, stmt, im); const res = ts.createPrinter().printNode(ts.EmitHint.Unspecified, tsStatement, sf); return res.replace(/\s+/g, ' '); } diff --git a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts index 82bd44831b801..fc934d87a09a6 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts @@ -118,6 +118,10 @@ class IvyTransformationVisitor extends Visitor { const statements: ts.Statement[] = []; const members = [...node.members]; + // Note: Class may be already transformed by e.g. Tsickle and + // not have a direct reference to the source file. + const sourceFile = ts.getOriginalNode(node).getSourceFile(); + for (const field of this.classCompilationMap.get(node)!) { // Type-only member. if (field.initializer === null) { @@ -125,7 +129,8 @@ class IvyTransformationVisitor extends Visitor { } // Translate the initializer for the field into TS nodes. - const exprNode = translateExpression(field.initializer, this.importManager, translateOptions); + const exprNode = + translateExpression(sourceFile, field.initializer, this.importManager, translateOptions); // Create a static property declaration for the new field. const property = ts.factory.createPropertyDeclaration( @@ -142,7 +147,8 @@ class IvyTransformationVisitor extends Visitor { /* hasTrailingNewLine */ false); } - field.statements.map(stmt => translateStatement(stmt, this.importManager, translateOptions)) + field.statements + .map(stmt => translateStatement(sourceFile, stmt, this.importManager, translateOptions)) .forEach(stmt => statements.push(stmt)); members.push(property); @@ -313,7 +319,7 @@ function transformIvySourceFile( // to the ImportManager. const downlevelTranslatedCode = getLocalizeCompileTarget(context) < ts.ScriptTarget.ES2015; const constants = - constantPool.statements.map(stmt => translateStatement(stmt, importManager, { + constantPool.statements.map(stmt => translateStatement(file, stmt, importManager, { recordWrappedNode, downlevelTaggedTemplates: downlevelTranslatedCode, downlevelVariableDeclarations: downlevelTranslatedCode, diff --git a/packages/compiler-cli/src/ngtsc/translator/src/api/import_generator.ts b/packages/compiler-cli/src/ngtsc/translator/src/api/import_generator.ts index ec45efa763ef2..4714beb192e1f 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/api/import_generator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/api/import_generator.ts @@ -6,17 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -/** - * The symbol name and import namespace of an imported symbol, - * which has been registered through the ImportGenerator. - */ -export interface NamedImport { - /** The import namespace containing this imported symbol. */ - moduleImport: TExpression|null; - /** The (possibly rewritten) name of the imported symbol. */ - symbol: string; -} - /** * A request to import a given symbol from the given module. */ @@ -49,7 +38,6 @@ export interface ImportRequest { * Implementations of these methods return a specific identifier that corresponds to the imported * module. */ -export interface ImportGenerator { - generateNamespaceImport(moduleName: string): TExpression; - generateNamedImport(moduleName: string, originalSymbol: string): NamedImport; +export interface ImportGenerator { + addImport(request: ImportRequest): TExpression; } diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index d5bcc4fedf526..f0f5dae9a4c42 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -46,15 +46,16 @@ export interface TranslatorOptions { annotateForClosureCompiler?: boolean; } -export class ExpressionTranslatorVisitor implements o.ExpressionVisitor, - o.StatementVisitor { +export class ExpressionTranslatorVisitor implements + o.ExpressionVisitor, o.StatementVisitor { private downlevelTaggedTemplates: boolean; private downlevelVariableDeclarations: boolean; private recordWrappedNode: RecordWrappedNodeFn; constructor( private factory: AstFactory, - private imports: ImportGenerator, options: TranslatorOptions) { + private imports: ImportGenerator, private contextFile: TFile, + options: TranslatorOptions) { this.downlevelTaggedTemplates = options.downlevelTaggedTemplates === true; this.downlevelVariableDeclarations = options.downlevelVariableDeclarations === true; this.recordWrappedNode = options.recordWrappedNode || (() => {}); @@ -210,11 +211,11 @@ export class ExpressionTranslatorVisitor implements o.E private createES5TaggedTemplateFunctionCall( tagHandler: TExpression, {elements, expressions}: TemplateLiteral): TExpression { // Ensure that the `__makeTemplateObject()` helper has been imported. - const {moduleImport, symbol} = - this.imports.generateNamedImport('tslib', '__makeTemplateObject'); - const __makeTemplateObjectHelper = (moduleImport === null) ? - this.factory.createIdentifier(symbol) : - this.factory.createPropertyAccess(moduleImport, symbol); + const __makeTemplateObjectHelper = this.imports.addImport({ + exportModuleSpecifier: 'tslib', + exportSymbolName: '__makeTemplateObject', + requestedFile: this.contextFile, + }); // Collect up the cooked and raw strings into two separate arrays. const cooked: TExpression[] = []; @@ -244,20 +245,21 @@ export class ExpressionTranslatorVisitor implements o.E if (ast.value.moduleName === null) { throw new Error('Invalid import without name nor moduleName'); } - return this.imports.generateNamespaceImport(ast.value.moduleName); + return this.imports.addImport({ + exportModuleSpecifier: ast.value.moduleName, + exportSymbolName: null, + requestedFile: this.contextFile, + }); } // If a moduleName is specified, this is a normal import. If there's no module name, it's a // reference to a global/ambient symbol. if (ast.value.moduleName !== null) { // This is a normal import. Find the imported module. - const {moduleImport, symbol} = - this.imports.generateNamedImport(ast.value.moduleName, ast.value.name); - if (moduleImport === null) { - // The symbol was ambient after all. - return this.factory.createIdentifier(symbol); - } else { - return this.factory.createPropertyAccess(moduleImport, symbol); - } + return this.imports.addImport({ + exportModuleSpecifier: ast.value.moduleName, + exportSymbolName: ast.value.name, + requestedFile: this.contextFile, + }); } else { // The symbol is ambient, so just reference it. return this.factory.createIdentifier(ast.value.name); diff --git a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts index 6ae8412b69b5c..4041e101d5bcf 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts @@ -13,21 +13,21 @@ import {assertSuccessfulReferenceEmit, ImportFlags, OwningModule, Reference, Ref import {AmbientImport, ReflectionHost} from '../../reflection'; import {Context} from './context'; -import {ImportManager} from './import_manager'; +import {ImportManagerV2} from './import_manager/import_manager'; import {tsNumericExpression} from './ts_util'; import {TypeEmitter} from './type_emitter'; export function translateType( type: o.Type, contextFile: ts.SourceFile, reflector: ReflectionHost, - refEmitter: ReferenceEmitter, imports: ImportManager): ts.TypeNode { + refEmitter: ReferenceEmitter, imports: ImportManagerV2): ts.TypeNode { return type.visitType( new TypeTranslatorVisitor(imports, contextFile, reflector, refEmitter), new Context(false)); } class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor { constructor( - private imports: ImportManager, private contextFile: ts.SourceFile, + private imports: ImportManagerV2, private contextFile: ts.SourceFile, private reflector: ReflectionHost, private refEmitter: ReferenceEmitter) {} visitBuiltinType(type: o.BuiltinType, context: Context): ts.KeywordTypeNode { @@ -148,12 +148,12 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor { if (ast.value.moduleName === null || ast.value.name === null) { throw new Error(`Import unknown module or symbol`); } - const {moduleImport, symbol} = - this.imports.generateNamedImport(ast.value.moduleName, ast.value.name); - const symbolIdentifier = ts.factory.createIdentifier(symbol); - - const typeName = moduleImport ? ts.factory.createQualifiedName(moduleImport, symbolIdentifier) : - symbolIdentifier; + const typeName = this.imports.addImport({ + exportModuleSpecifier: ast.value.moduleName, + exportSymbolName: ast.value.name, + requestedFile: this.contextFile, + asTypeReference: true + }); const typeArguments = ast.typeParams !== null ? ast.typeParams.map(type => this.translateType(type, context)) : diff --git a/packages/compiler-cli/src/ngtsc/translator/src/typescript_translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/typescript_translator.ts index 8e8f932bfd302..5e6e4a8f4cef0 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/typescript_translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/typescript_translator.ts @@ -15,19 +15,23 @@ import {ExpressionTranslatorVisitor, TranslatorOptions} from './translator'; import {TypeScriptAstFactory} from './typescript_ast_factory'; export function translateExpression( - expression: o.Expression, imports: ImportGenerator, + contextFile: ts.SourceFile, expression: o.Expression, + imports: ImportGenerator, options: TranslatorOptions = {}): ts.Expression { return expression.visitExpression( - new ExpressionTranslatorVisitor( - new TypeScriptAstFactory(options.annotateForClosureCompiler === true), imports, options), + new ExpressionTranslatorVisitor( + new TypeScriptAstFactory(options.annotateForClosureCompiler === true), imports, + contextFile, options), new Context(false)); } export function translateStatement( - statement: o.Statement, imports: ImportGenerator, + contextFile: ts.SourceFile, statement: o.Statement, + imports: ImportGenerator, options: TranslatorOptions = {}): ts.Statement { return statement.visitStatement( - new ExpressionTranslatorVisitor( - new TypeScriptAstFactory(options.annotateForClosureCompiler === true), imports, options), + new ExpressionTranslatorVisitor( + new TypeScriptAstFactory(options.annotateForClosureCompiler === true), imports, + contextFile, options), new Context(true)); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index e77cf955cc756..d1b184d49bcb2 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -123,7 +123,7 @@ export class Environment extends ReferenceEmitEnvironment { assertSuccessfulReferenceEmit(ngExpr, this.contextFile, 'class'); // Use `translateExpression` to convert the `Expression` into a `ts.Expression`. - return translateExpression(ngExpr.expression, this.importManager); + return translateExpression(this.contextFile, ngExpr.expression, this.importManager); } private emitTypeParameters(declaration: ClassDeclaration): diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts index ea336d57bb711..1a70753c2883c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts @@ -22,7 +22,7 @@ import {ImportManager, translateExpression, translateType} from '../../translato export class ReferenceEmitEnvironment { constructor( readonly importManager: ImportManager, protected refEmitter: ReferenceEmitter, - readonly reflector: ReflectionHost, protected contextFile: ts.SourceFile) {} + readonly reflector: ReflectionHost, public contextFile: ts.SourceFile) {} canReferenceType( ref: Reference, @@ -57,7 +57,7 @@ export class ReferenceEmitEnvironment { */ referenceExternalSymbol(moduleName: string, name: string): ts.Expression { const external = new ExternalExpr({moduleName, name}); - return translateExpression(external, this.importManager); + return translateExpression(this.contextFile, external, this.importManager); } /** From 93ab0c09c805768cef594c6faf76512295eafff5 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 11 Mar 2024 14:48:32 +0000 Subject: [PATCH 03/10] refactor(compiler-cli): use new import manager for ngtsc transforms This commit switches ngtsc's JS and DTS transform to use the new import manager. This is a drop-in replacement as we've updated the translator helpers in the previous commit to align with the new API suggested by the `ImportManagerV2` (to be renamed then). --- .../annotations/common/test/metadata_spec.ts | 5 ++-- .../src/ngtsc/transform/src/api.ts | 8 +++--- .../src/ngtsc/transform/src/declaration.ts | 26 +++++++++---------- .../src/ngtsc/transform/src/transform.ts | 12 ++++----- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts index 2623952c3cdd7..8fa09b359e680 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts @@ -10,10 +10,9 @@ import ts from 'typescript'; import {absoluteFrom, getSourceFileOrError} from '../../../file_system'; import {runInEachFileSystem, TestFile} from '../../../file_system/testing'; -import {NoopImportRewriter} from '../../../imports'; import {TypeScriptReflectionHost} from '../../../reflection'; import {getDeclaration, makeProgram} from '../../../testing'; -import {ImportManager, translateStatement} from '../../../translator'; +import {ImportManagerV2, presetImportManagerForceNamespaceImports, translateStatement} from '../../../translator'; import {extractClassMetadata} from '../src/metadata'; runInEachFileSystem(() => { @@ -133,7 +132,7 @@ runInEachFileSystem(() => { return ''; } const sf = getSourceFileOrError(program, _('/index.ts')); - const im = new ImportManager(new NoopImportRewriter(), 'i'); + const im = new ImportManagerV2(presetImportManagerForceNamespaceImports); const stmt = compileClassMetadata(call).toStmt(); const tsStatement = translateStatement(sf, stmt, im); const res = ts.createPrinter().printNode(ts.EmitHint.Unspecified, tsStatement, sf); diff --git a/packages/compiler-cli/src/ngtsc/transform/src/api.ts b/packages/compiler-cli/src/ngtsc/transform/src/api.ts index aeadd6e3810f7..83fe799962e8a 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/api.ts @@ -13,7 +13,7 @@ import {Reexport, ReferenceEmitter} from '../../imports'; import {SemanticSymbol} from '../../incremental/semantic_graph'; import {IndexingContext} from '../../indexer'; import {ClassDeclaration, Decorator, ReflectionHost} from '../../reflection'; -import {ImportManager} from '../../translator'; +import {ImportManagerV2} from '../../translator'; import {TypeCheckContext} from '../../typecheck/api'; import {ExtendedTemplateChecker} from '../../typecheck/extended/api'; import {TemplateSemanticsChecker} from '../../typecheck/template_semantics/api/api'; @@ -264,11 +264,11 @@ export interface ResolveResult { } export interface DtsTransform { - transformClassElement?(element: ts.ClassElement, imports: ImportManager): ts.ClassElement; + transformClassElement?(element: ts.ClassElement, imports: ImportManagerV2): ts.ClassElement; transformFunctionDeclaration? - (element: ts.FunctionDeclaration, imports: ImportManager): ts.FunctionDeclaration; + (element: ts.FunctionDeclaration, imports: ImportManagerV2): ts.FunctionDeclaration; transformClass? (clazz: ts.ClassDeclaration, elements: ReadonlyArray, reflector: ReflectionHost, refEmitter: ReferenceEmitter, - imports: ImportManager): ts.ClassDeclaration; + imports: ImportManagerV2): ts.ClassDeclaration; } diff --git a/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts b/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts index 35a37301e293a..3f4f4ac5206c4 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts @@ -11,10 +11,9 @@ import ts from 'typescript'; import {ImportRewriter, ReferenceEmitter} from '../../imports'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManager, translateType} from '../../translator'; +import {ImportManagerV2, presetImportManagerForceNamespaceImports, translateType} from '../../translator'; import {DtsTransform} from './api'; -import {addImports} from './utils'; /** * Keeps track of `DtsTransform`s per source file, so that it is known which source files need to @@ -55,11 +54,10 @@ export class DtsTransformRegistry { export function declarationTransformFactory( transformRegistry: DtsTransformRegistry, reflector: ReflectionHost, - refEmitter: ReferenceEmitter, importRewriter: ImportRewriter, - importPrefix?: string): ts.TransformerFactory { + refEmitter: ReferenceEmitter, + importRewriter: ImportRewriter): ts.TransformerFactory { return (context: ts.TransformationContext) => { - const transformer = - new DtsTransformer(context, reflector, refEmitter, importRewriter, importPrefix); + const transformer = new DtsTransformer(context, reflector, refEmitter, importRewriter); return (fileOrBundle) => { if (ts.isBundle(fileOrBundle)) { // Only attempt to transform source files. @@ -80,14 +78,14 @@ export function declarationTransformFactory( class DtsTransformer { constructor( private ctx: ts.TransformationContext, private reflector: ReflectionHost, - private refEmitter: ReferenceEmitter, private importRewriter: ImportRewriter, - private importPrefix?: string) {} + private refEmitter: ReferenceEmitter, private importRewriter: ImportRewriter) {} /** * Transform the declaration file and add any declarations which were recorded. */ transform(sf: ts.SourceFile, transforms: DtsTransform[]): ts.SourceFile { - const imports = new ImportManager(this.importRewriter, this.importPrefix); + const imports = new ImportManagerV2( + {...presetImportManagerForceNamespaceImports, rewriter: this.importRewriter}); const visitor: ts.Visitor = (node: ts.Node): ts.VisitResult => { if (ts.isClassDeclaration(node)) { @@ -103,13 +101,13 @@ class DtsTransformer { // Recursively scan through the AST and process all nodes as desired. sf = ts.visitNode(sf, visitor, ts.isSourceFile) || sf; - // Add new imports for this file. - return addImports(this.ctx.factory, imports, sf); + // Update/insert needed imports. + return imports.transformTsFile(this.ctx, sf); } private transformClassDeclaration( clazz: ts.ClassDeclaration, transforms: DtsTransform[], - imports: ImportManager): ts.ClassDeclaration { + imports: ImportManagerV2): ts.ClassDeclaration { let elements: ts.ClassElement[]|ReadonlyArray = clazz.members; let elementsChanged = false; @@ -158,7 +156,7 @@ class DtsTransformer { private transformFunctionDeclaration( declaration: ts.FunctionDeclaration, transforms: DtsTransform[], - imports: ImportManager): ts.FunctionDeclaration { + imports: ImportManagerV2): ts.FunctionDeclaration { let newDecl = declaration; for (const transform of transforms) { @@ -186,7 +184,7 @@ export class IvyDeclarationDtsTransform implements DtsTransform { transformClass( clazz: ts.ClassDeclaration, members: ReadonlyArray, reflector: ReflectionHost, refEmitter: ReferenceEmitter, - imports: ImportManager): ts.ClassDeclaration { + imports: ImportManagerV2): ts.ClassDeclaration { const original = ts.getOriginalNode(clazz) as ClassDeclaration; if (!this.declarationFields.has(original)) { diff --git a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts index fc934d87a09a6..c595e5341f1d3 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts @@ -13,12 +13,11 @@ import {DefaultImportTracker, ImportRewriter, LocalCompilationExtraImportsTracke import {getDefaultImportDeclaration} from '../../imports/src/default'; import {PerfPhase, PerfRecorder} from '../../perf'; import {Decorator, ReflectionHost} from '../../reflection'; -import {ImportManager, RecordWrappedNodeFn, translateExpression, translateStatement, TranslatorOptions} from '../../translator'; +import {ImportManagerV2, presetImportManagerForceNamespaceImports, RecordWrappedNodeFn, translateExpression, translateStatement, TranslatorOptions} from '../../translator'; import {visit, VisitListEntryResult, Visitor} from '../../util/src/visitor'; import {CompileResult} from './api'; import {TraitCompiler} from './compilation'; -import {addImports} from './utils'; const NO_DECORATORS = new Set(); @@ -94,7 +93,7 @@ class IvyTransformationVisitor extends Visitor { constructor( private compilation: TraitCompiler, private classCompilationMap: Map, - private reflector: ReflectionHost, private importManager: ImportManager, + private reflector: ReflectionHost, private importManager: ImportManagerV2, private recordWrappedNodeExpr: RecordWrappedNodeFn, private isClosureCompilerEnabled: boolean, private isCore: boolean, private deferrableImports: Set) { @@ -292,7 +291,8 @@ function transformIvySourceFile( file: ts.SourceFile, isCore: boolean, isClosureCompilerEnabled: boolean, recordWrappedNode: RecordWrappedNodeFn): ts.SourceFile { const constantPool = new ConstantPool(isClosureCompilerEnabled); - const importManager = new ImportManager(importRewriter); + const importManager = + new ImportManagerV2({...presetImportManagerForceNamespaceImports, rewriter: importRewriter}); // The transformation process consists of 2 steps: // @@ -333,12 +333,12 @@ function transformIvySourceFile( // Add extra imports. if (localCompilationExtraImportsTracker !== null) { for (const moduleName of localCompilationExtraImportsTracker.getImportsForFile(sf)) { - importManager.generateSideEffectImport(moduleName); + importManager.addSideEffectImport(sf, moduleName); } } // Add new imports for this file. - sf = addImports(context.factory, importManager, sf, constants); + sf = importManager.transformTsFile(context, sf, constants); if (fileOverviewMeta !== null) { setFileOverviewComment(sf, fileOverviewMeta); From 5de349160496c8d36448c44639e0d4b9f11bb609 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 11 Mar 2024 14:50:36 +0000 Subject: [PATCH 04/10] refactor(compiler-cli): update type check generation code to use new import manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updates the type-check block generation code (also for inline type check blocks) to use the new import manager. This is now a requirement because the translator utilities from the reference emit environment expect an import manager that follows the new contract established via `ImportGenerator`. For type check files, we can simply print new imports as we don't expect existing imports to be updated. That is because type check files do not have any _original_ source files (or in practice— those are empty). For type check blocks inline, or constructors, imports _may_ be re-used. This is great as it helps fixing some incrementality bugs that we were seeing in the type check code. That is, sometimes the type check block code may generate imports conditionally for e.g. `TemplateRef`, or animations. Those then **prevent** incremental re-use if TCB code switches between those continously. We tried to account for that with signal inputs by always pre-generating such imports. This fixed the issue for type-check files, but for inline type check blocks this is different as we would introduce new imports in user code that would then be changed back in subsequential edit iterations. See: https://github.com/angular/angular/pull/53521#pullrequestreview-1778130879. In practice, the assumption was that we would be fine since user code is most likely containing imports to `@angular/core` already. That is a true assumption, but unfortunately it doesn't help with incremental re-use because TypeScript's structural change detection does not dedupe and expects 1:1 exact imports from their old source files. https://github.com/microsoft/TypeScript/pull/56845 To improve incremental re-use for the type check integration, we should re-use original source file imports when possible. This commit enables this. To update imports and execute inline operations, we are now uisng `magic-string` (which is then bundled) as it simplifies the string manipulatuons. --- package.json | 2 +- .../src/ngtsc/typecheck/BUILD.bazel | 1 + .../src/ngtsc/typecheck/src/context.ts | 97 ++++++++++++------- .../src/ngtsc/typecheck/src/environment.ts | 4 +- .../src/reference_emit_environment.ts | 4 +- .../src/ngtsc/typecheck/src/tcb_util.ts | 20 ++-- .../src/ngtsc/typecheck/src/ts_util.ts | 11 --- .../ngtsc/typecheck/src/type_check_file.ts | 32 ++++-- yarn.lock | 15 ++- 9 files changed, 118 insertions(+), 68 deletions(-) diff --git a/package.json b/package.json index 44cc30765e7c1..c85d821597d27 100644 --- a/package.json +++ b/package.json @@ -118,7 +118,7 @@ "karma-jasmine": "^5.0.0", "karma-requirejs": "^1.1.0", "karma-sourcemap-loader": "^0.4.0", - "magic-string": "0.30.7", + "magic-string": "^0.30.8", "memo-decorator": "^2.0.1", "ngx-flamegraph": "0.0.12", "ngx-progressbar": "^11.1.0", diff --git a/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel index 47a64194b22dd..e070f16a35fc2 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel @@ -26,6 +26,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/typecheck/diagnostics", "//packages/compiler-cli/src/ngtsc/util", "@npm//@types/node", + "@npm//magic-string", "@npm//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index c216b3e520403..5f947c5843ac2 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -7,16 +7,17 @@ */ import {BoundTarget, ParseError, ParseSourceFile, R3TargetBinder, SchemaMetadata, TmplAstNode} from '@angular/compiler'; +import MagicString from 'magic-string'; import ts from 'typescript'; import {ErrorCode, ngErrorCode} from '../../../../src/ngtsc/diagnostics'; import {absoluteFromSourceFile, AbsoluteFsPath} from '../../file_system'; -import {NoopImportRewriter, Reference, ReferenceEmitter} from '../../imports'; +import {Reference, ReferenceEmitter} from '../../imports'; import {PipeMeta} from '../../metadata'; import {PerfEvent, PerfRecorder} from '../../perf'; import {FileUpdate} from '../../program_driver'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManager} from '../../translator'; +import {ImportManagerV2} from '../../translator'; import {TemplateDiagnostic, TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api'; import {makeTemplateDiagnostic} from '../diagnostics'; @@ -27,7 +28,6 @@ import {ReferenceEmitEnvironment} from './reference_emit_environment'; import {TypeCheckShimGenerator} from './shim'; import {TemplateSourceManager} from './source'; import {requiresInlineTypeCheckBlock, TcbInliningRequirement} from './tcb_util'; -import {getImportString} from './ts_util'; import {generateTypeCheckBlock, TcbGenericContextBehavior} from './type_check_block'; import {TypeCheckFile} from './type_check_file'; import {generateInlineTypeCtor, requiresInlineTypeCtor} from './type_constructor'; @@ -358,34 +358,61 @@ export class TypeCheckContextImpl implements TypeCheckContext { return null; } - // Imports may need to be added to the file to support type-checking of directives used in the - // template within it. - const importManager = new ImportManager(new NoopImportRewriter(), '_i'); - - // Each Op has a splitPoint index into the text where it needs to be inserted. Split the - // original source text into chunks at these split points, where code will be inserted between - // the chunks. - const ops = this.opMap.get(sf)!.sort(orderOps); - const textParts = splitStringAtPoints(sf.text, ops.map(op => op.splitPoint)); - // Use a `ts.Printer` to generate source code. const printer = ts.createPrinter({omitTrailingSemicolon: true}); - // Begin with the initial section of the code text. - let code = textParts[0]; - - // Process each operation and use the printer to generate source code for it, inserting it into - // the source code in between the original chunks. - ops.forEach((op, idx) => { - const text = op.execute(importManager, sf, this.refEmitter, printer); - code += '\n\n' + text + textParts[idx + 1]; + // Imports may need to be added to the file to support type-checking of directives + // used in the template within it. + const importManager = new ImportManagerV2({ + // This minimizes noticeable changes with older versions of `ImportManager`. + forceGenerateNamespacesForNewImports: true, + // Type check block code affects code completion and fix suggestions. + // We want to encourage single quotes for now, like we always did. + shouldUseSingleQuotes: () => true, }); - // Write out the imports that need to be added to the beginning of the file. - let imports = importManager.getAllImports(sf.fileName).map(getImportString).join('\n'); - code = imports + '\n' + code; + // Execute ops. + // Each Op has a splitPoint index into the text where it needs to be inserted. + const updates: {pos: number, deletePos?: number, text: string}[] = + this.opMap.get(sf)!.map(op => { + return { + pos: op.splitPoint, + text: op.execute(importManager, sf, this.refEmitter, printer), + }; + }); + + const {newImports, updatedImports} = importManager.finalize(); + + // Capture new imports + if (newImports.has(sf.fileName)) { + newImports.get(sf.fileName)!.forEach(newImport => { + updates.push({ + pos: 0, + text: printer.printNode(ts.EmitHint.Unspecified, newImport, sf), + }); + }); + } + + // Capture updated imports + for (const [oldBindings, newBindings] of updatedImports.entries()) { + if (oldBindings.getSourceFile() !== sf) { + throw new Error('Unexpected updates to unrelated source files.'); + } + updates.push({ + pos: oldBindings.getStart(), + deletePos: oldBindings.getEnd(), + text: printer.printNode(ts.EmitHint.Unspecified, newBindings, sf), + }); + } - return code; + const result = new MagicString(sf.text, {filename: sf.fileName}); + for (const update of updates) { + if (update.deletePos !== undefined) { + result.remove(update.pos, update.deletePos); + } + result.appendLeft(update.pos, update.text); + } + return result.toString(); } finalize(): Map { @@ -510,8 +537,9 @@ interface Op { /** * Execute the operation and return the generated code as text. */ - execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer): - string; + execute( + im: ImportManagerV2, sf: ts.SourceFile, refEmitter: ReferenceEmitter, + printer: ts.Printer): string; } /** @@ -531,16 +559,18 @@ class InlineTcbOp implements Op { return this.ref.node.end + 1; } - execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer): - string { + execute( + im: ImportManagerV2, sf: ts.SourceFile, refEmitter: ReferenceEmitter, + printer: ts.Printer): string { const env = new Environment(this.config, im, refEmitter, this.reflector, sf); const fnName = ts.factory.createIdentifier(`_tcb_${this.ref.node.pos}`); - // Inline TCBs should copy any generic type parameter nodes directly, as the TCB code is inlined - // into the class in a context where that will always be legal. + // Inline TCBs should copy any generic type parameter nodes directly, as the TCB code is + // inlined into the class in a context where that will always be legal. const fn = generateTypeCheckBlock( env, this.ref, fnName, this.meta, this.domSchemaChecker, this.oobRecorder, TcbGenericContextBehavior.CopyClassNodes); + return printer.printNode(ts.EmitHint.Unspecified, fn, sf); } } @@ -560,8 +590,9 @@ class TypeCtorOp implements Op { return this.ref.node.end - 1; } - execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer): - string { + execute( + im: ImportManagerV2, sf: ts.SourceFile, refEmitter: ReferenceEmitter, + printer: ts.Printer): string { const emitEnv = new ReferenceEmitEnvironment(im, refEmitter, this.reflector, sf); const tcb = generateInlineTypeCtor(emitEnv, this.ref.node, this.meta); return printer.printNode(ts.EmitHint.Unspecified, tcb, sf); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index d1b184d49bcb2..ce5acb42e1d09 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -10,7 +10,7 @@ import ts from 'typescript'; import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../imports'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManager, translateExpression} from '../../translator'; +import {ImportManagerV2, translateExpression} from '../../translator'; import {TypeCheckableDirectiveMeta, TypeCheckingConfig, TypeCtorMetadata} from '../api'; import {ReferenceEmitEnvironment} from './reference_emit_environment'; @@ -42,7 +42,7 @@ export class Environment extends ReferenceEmitEnvironment { protected pipeInstStatements: ts.Statement[] = []; constructor( - readonly config: TypeCheckingConfig, importManager: ImportManager, + readonly config: TypeCheckingConfig, importManager: ImportManagerV2, refEmitter: ReferenceEmitter, reflector: ReflectionHost, contextFile: ts.SourceFile) { super(importManager, refEmitter, reflector, contextFile); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts index 1a70753c2883c..45d984a39079c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts @@ -11,7 +11,7 @@ import ts from 'typescript'; import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitKind, ReferenceEmitter} from '../../imports'; import {ReflectionHost} from '../../reflection'; -import {ImportManager, translateExpression, translateType} from '../../translator'; +import {ImportManagerV2, translateExpression, translateType} from '../../translator'; /** * An environment for a given source file that can be used to emit references. @@ -21,7 +21,7 @@ import {ImportManager, translateExpression, translateType} from '../../translato */ export class ReferenceEmitEnvironment { constructor( - readonly importManager: ImportManager, protected refEmitter: ReferenceEmitter, + readonly importManager: ImportManagerV2, protected refEmitter: ReferenceEmitter, readonly reflector: ReflectionHost, public contextFile: ts.SourceFile) {} canReferenceType( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts index 07100b3ac3302..12814821dc28b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts @@ -19,17 +19,21 @@ import {ReferenceEmitEnvironment} from './reference_emit_environment'; import {TypeParameterEmitter} from './type_parameter_emitter'; /** - * External modules that always should exist for type check blocks and - * file hosting inline type constructors. + * External modules/identifiers that always should exist for type check + * block files. * * Importing the modules in preparation helps ensuring a stable import graph * that would not degrade TypeScript's incremental program structure re-use. + * + * Note: For inline type check blocks, or type constructors, we cannot add preparation + * imports, but ideally the required modules are already imported and can be re-used + * to not incur a structural TypeScript program re-use discarding. */ -const TCB_FILE_IMPORT_GRAPH_PREPARE_MODULES = [ +const TCB_FILE_IMPORT_GRAPH_PREPARE_IDENTIFIERS = [ // Imports may be added for signal input checking. We wouldn't want to change the // import graph for incremental compilations when suddenly a signal input is used, // or removed. - R3Identifiers.InputSignalBrandWriteType.moduleName, + R3Identifiers.InputSignalBrandWriteType, ]; /** @@ -194,8 +198,12 @@ function getTemplateId( * import graph changes whenever e.g. a signal input is introduced in user code. */ export function ensureTypeCheckFilePreparationImports(env: ReferenceEmitEnvironment): void { - for (const moduleName of TCB_FILE_IMPORT_GRAPH_PREPARE_MODULES) { - env.importManager.generateNamespaceImport(moduleName); + for (const identifier of TCB_FILE_IMPORT_GRAPH_PREPARE_IDENTIFIERS) { + env.importManager.addImport({ + exportModuleSpecifier: identifier.moduleName, + exportSymbolName: identifier.name, + requestedFile: env.contextFile, + }); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts index a943050d8a16f..33aee30fb4112 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts @@ -8,10 +8,7 @@ import ts from 'typescript'; -import {Import} from '../../translator'; - import {addExpressionIdentifier, ExpressionIdentifier} from './comments'; -import {wrapForTypeChecker} from './diagnostics'; @@ -163,11 +160,3 @@ export function tsNumericExpression(value: number): ts.NumericLiteral|ts.PrefixU return ts.factory.createNumericLiteral(value); } - -export function getImportString(imp: Import): string { - if (imp.qualifier === null) { - return `import from '${imp.specifier}';`; - } else { - return `import * as ${imp.qualifier.text} from '${imp.specifier}';`; - } -} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts index 6f46600fb0b05..257100b91fd59 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts @@ -8,16 +8,15 @@ import ts from 'typescript'; import {AbsoluteFsPath, join} from '../../file_system'; -import {NoopImportRewriter, Reference, ReferenceEmitter} from '../../imports'; +import {Reference, ReferenceEmitter} from '../../imports'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManager} from '../../translator'; +import {ImportManagerV2} from '../../translator'; import {TypeCheckBlockMetadata, TypeCheckingConfig} from '../api'; import {DomSchemaChecker} from './dom'; import {Environment} from './environment'; import {OutOfBandDiagnosticRecorder} from './oob'; import {ensureTypeCheckFilePreparationImports} from './tcb_util'; -import {getImportString} from './ts_util'; import {generateTypeCheckBlock, TcbGenericContextBehavior} from './type_check_block'; @@ -38,7 +37,14 @@ export class TypeCheckFile extends Environment { readonly fileName: AbsoluteFsPath, config: TypeCheckingConfig, refEmitter: ReferenceEmitter, reflector: ReflectionHost, compilerHost: Pick) { super( - config, new ImportManager(new NoopImportRewriter(), 'i'), refEmitter, reflector, + config, new ImportManagerV2({ + // This minimizes noticeable changes with older versions of `ImportManager`. + forceGenerateNamespacesForNewImports: true, + // Type check block code affects code completion and fix suggestions. + // We want to encourage single quotes for now, like we always did. + shouldUseSingleQuotes: () => true + }), + refEmitter, reflector, ts.createSourceFile( compilerHost.getCanonicalFileName(fileName), '', ts.ScriptTarget.Latest, true)); } @@ -60,11 +66,21 @@ export class TypeCheckFile extends Environment { // import to e.g. `@angular/core` always exists to guarantee a stable graph. ensureTypeCheckFilePreparationImports(this); - let source: string = this.importManager.getAllImports(this.contextFile.fileName) - .map(getImportString) - .join('\n') + - '\n\n'; + const importChanges = this.importManager.finalize(); + if (importChanges.updatedImports.size > 0) { + throw new Error( + 'AssertionError: Expected no imports to be updated for a new type check file.'); + } + const printer = ts.createPrinter({removeComments}); + let source = ''; + + const newImports = importChanges.newImports.get(this.contextFile.fileName); + if (newImports !== undefined) { + source += newImports.map(i => printer.printNode(ts.EmitHint.Unspecified, i, this.contextFile)) + .join('\n'); + } + source += '\n'; for (const stmt of this.pipeInstStatements) { source += printer.printNode(ts.EmitHint.Unspecified, stmt, this.contextFile) + '\n'; diff --git a/yarn.lock b/yarn.lock index 75a944222f85c..10a3e9d640bd8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -386,7 +386,6 @@ "@angular/build-tooling@https://github.com/angular/dev-infra-private-build-tooling-builds.git#65d002a534a74daa3c9bd26bdea5a092cbd519df": version "0.0.0-5774b71c01a55c4c998f858ee37d3b77ae704c31" - uid "65d002a534a74daa3c9bd26bdea5a092cbd519df" resolved "https://github.com/angular/dev-infra-private-build-tooling-builds.git#65d002a534a74daa3c9bd26bdea5a092cbd519df" dependencies: "@angular-devkit/build-angular" "17.2.0-rc.0" @@ -495,7 +494,6 @@ "@angular/docs@https://github.com/angular/dev-infra-private-docs-builds.git#82c4573f5c9d4fb864271a1c74259fc251457e2f": version "0.0.0-5774b71c01a55c4c998f858ee37d3b77ae704c31" - uid "82c4573f5c9d4fb864271a1c74259fc251457e2f" resolved "https://github.com/angular/dev-infra-private-docs-builds.git#82c4573f5c9d4fb864271a1c74259fc251457e2f" dependencies: "@angular/cdk" "17.3.0-next.0" @@ -4876,8 +4874,7 @@ "@types/glob" "*" "@types/node" "*" -"@types/selenium-webdriver4@npm:@types/selenium-webdriver@4.1.21", "@types/selenium-webdriver@^4.1.21": - name "@types/selenium-webdriver4" +"@types/selenium-webdriver4@npm:@types/selenium-webdriver@4.1.21": version "4.1.21" resolved "https://registry.yarnpkg.com/@types/selenium-webdriver/-/selenium-webdriver-4.1.21.tgz#79fe31faf9953a4143c3e32944d98d5146bbe185" integrity sha512-QGURnImvxYlIQz5DVhvHdqpYNLBjhJ2Vm+cnQI2G9QZzkWlZm0LkLcvDcHp+qE6N2KBz4CeuvXgPO7W3XQ0Tyw== @@ -4894,6 +4891,14 @@ resolved "https://registry.yarnpkg.com/@types/selenium-webdriver/-/selenium-webdriver-3.0.25.tgz#7982bf5add9539b4e953512354055545e8508cff" integrity sha512-gkb8rx0kjHuaVUzIfKa1tVVjnEmjYLufDmoOHKPsaoRraTcNjV9W9ruFWi3Xv821c7M5gZVmrGOT8BJYsY/acQ== +"@types/selenium-webdriver@^4.1.21": + name "@types/selenium-webdriver4" + version "4.1.22" + resolved "https://registry.yarnpkg.com/@types/selenium-webdriver/-/selenium-webdriver-4.1.22.tgz#344519b90727eb713e1ce6d2e0198eb0b4f8f316" + integrity sha512-MCL4l7q8dwxejr2Q2NXLyNwHWMPdlWE0Kpn6fFwJtvkJF7PTkG5jkvbH/X1IAAQxgt/L1dA8u2GtDeekvSKvOA== + dependencies: + "@types/ws" "*" + "@types/semver@^7.3.4": version "7.5.4" resolved "https://registry.yarnpkg.com/@types/semver/-/semver-7.5.4.tgz#0a41252ad431c473158b22f9bfb9a63df7541cff" @@ -12480,7 +12485,7 @@ magic-string@0.30.7: dependencies: "@jridgewell/sourcemap-codec" "^1.4.15" -magic-string@0.30.8: +magic-string@0.30.8, magic-string@^0.30.8: version "0.30.8" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.30.8.tgz#14e8624246d2bedba70d5462aa99ac9681844613" integrity sha512-ISQTe55T2ao7XtlAStud6qwYPZjE4GK1S/BeVPus4jrq6JuOnQ00YKQC581RWhR122W7msZV263KzVeLoqidyQ== From dee04abadd4e8ff82b8b796de20defc3dc6c9cb5 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 11 Mar 2024 15:45:51 +0000 Subject: [PATCH 05/10] refactor(compiler-cli): switch jit transforms to use new import manager Switches the JIT transforms to use the new import manager. --- .../input_function.ts | 3 +- .../model_function.ts | 28 ++++++++----------- .../output_function.ts | 3 +- .../query_functions.ts | 3 +- .../initializer_api_transforms/transform.ts | 14 +++------- .../transform_api.ts | 14 ++++++---- .../signal_queries_metadata_transform_spec.ts | 4 +-- 7 files changed, 32 insertions(+), 37 deletions(-) diff --git a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/input_function.ts b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/input_function.ts index 0d258d443d932..15827243dcf93 100644 --- a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/input_function.ts +++ b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/input_function.ts @@ -54,10 +54,11 @@ export const signalInputsTransform: PropertyTransform = ( 'transform': factory.createIdentifier('undefined'), }; + const sourceFile = member.getSourceFile(); const newDecorator = factory.createDecorator( factory.createCallExpression( createSyntheticAngularCoreDecoratorAccess( - factory, importManager, classDecorator, 'Input'), + factory, importManager, classDecorator, sourceFile, 'Input'), undefined, [ // Cast to `any` because `isSignal` will be private, and in case this diff --git a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/model_function.ts b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/model_function.ts index ad7c197bd1511..d006e2d5dba64 100644 --- a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/model_function.ts +++ b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/model_function.ts @@ -6,12 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ +import {Decorator} from '@angular/compiler-cli/src/ngtsc/reflection'; import ts from 'typescript'; import {isAngularDecorator, tryParseSignalModelMapping} from '../../../ngtsc/annotations'; -import {ImportManager} from '../../../ngtsc/translator'; +import {ImportManagerV2} from '../../../ngtsc/translator'; -import {PropertyTransform} from './transform_api'; +import {createSyntheticAngularCoreDecoratorAccess, PropertyTransform} from './transform_api'; /** * Transform that automatically adds `@Input` and `@Output` to members initialized as `model()`. @@ -23,7 +24,7 @@ export const signalModelTransform: PropertyTransform = ( factory, importTracker, importManager, - decorator, + classDecorator, isCore, ) => { if (host.getDecoratorsOfDeclaration(member)?.some(d => { @@ -42,10 +43,6 @@ export const signalModelTransform: PropertyTransform = ( return member; } - const classDecoratorIdentifier = ts.isIdentifier(decorator.identifier) ? - decorator.identifier : - decorator.identifier.expression; - const inputConfig = factory.createObjectLiteralExpression([ factory.createPropertyAssignment( 'isSignal', modelMapping.input.isSignal ? factory.createTrue() : factory.createFalse()), @@ -55,6 +52,7 @@ export const signalModelTransform: PropertyTransform = ( 'required', modelMapping.input.required ? factory.createTrue() : factory.createFalse()), ]); + const sourceFile = member.getSourceFile(); const inputDecorator = createDecorator( 'Input', // Config is cast to `any` because `isSignal` will be private, and in case this @@ -62,11 +60,11 @@ export const signalModelTransform: PropertyTransform = ( // not fail. It is already validated now due to us parsing the input metadata. factory.createAsExpression( inputConfig, factory.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)), - classDecoratorIdentifier, factory, importManager); + classDecorator, factory, sourceFile, importManager); const outputDecorator = createDecorator( 'Output', factory.createStringLiteral(modelMapping.output.bindingPropertyName), - classDecoratorIdentifier, factory, importManager); + classDecorator, factory, sourceFile, importManager); return factory.updatePropertyDeclaration( member, @@ -79,14 +77,10 @@ export const signalModelTransform: PropertyTransform = ( }; function createDecorator( - name: string, config: ts.Expression, classDecoratorIdentifier: ts.Identifier, - factory: ts.NodeFactory, importManager: ImportManager): ts.Decorator { - const callTarget = factory.createPropertyAccessExpression( - importManager.generateNamespaceImport('@angular/core'), - // The synthetic identifier may be checked later by the downlevel decorators - // transform to resolve to an Angular import using `getSymbolAtLocation`. We trick - // the transform to think it's not synthetic and comes from Angular core. - ts.setOriginalNode(factory.createIdentifier(name), classDecoratorIdentifier)); + name: string, config: ts.Expression, classDecorator: Decorator, factory: ts.NodeFactory, + sourceFile: ts.SourceFile, importManager: ImportManagerV2): ts.Decorator { + const callTarget = createSyntheticAngularCoreDecoratorAccess( + factory, importManager, classDecorator, sourceFile, name); return factory.createDecorator(factory.createCallExpression(callTarget, undefined, [config])); } diff --git a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/output_function.ts b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/output_function.ts index 5453e8ec4adf6..6d1cda62c6793 100644 --- a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/output_function.ts +++ b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/output_function.ts @@ -43,10 +43,11 @@ export const initializerApiOutputTransform: PropertyTransform = ( return member; } + const sourceFile = member.getSourceFile(); const newDecorator = factory.createDecorator( factory.createCallExpression( createSyntheticAngularCoreDecoratorAccess( - factory, importManager, classDecorator, 'Output'), + factory, importManager, classDecorator, sourceFile, 'Output'), undefined, [factory.createStringLiteral(output.metadata.bindingPropertyName)]), ); diff --git a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/query_functions.ts b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/query_functions.ts index 605679d01cfd3..e5f1693464005 100644 --- a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/query_functions.ts +++ b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/query_functions.ts @@ -56,11 +56,12 @@ export const queryFunctionsTransforms: PropertyTransform = ( return member; } + const sourceFile = member.getSourceFile(); const callArgs = queryDefinition.call.arguments; const newDecorator = factory.createDecorator( factory.createCallExpression( createSyntheticAngularCoreDecoratorAccess( - factory, importManager, classDecorator, + factory, importManager, classDecorator, sourceFile, queryFunctionToDecorator[queryDefinition.name]), undefined, // All positional arguments of the query functions can be mostly re-used as is diff --git a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform.ts b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform.ts index cf2a30eb10bbb..6c309b35bdb4c 100644 --- a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform.ts +++ b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform.ts @@ -11,8 +11,7 @@ import ts from 'typescript'; import {isAngularDecorator} from '../../../ngtsc/annotations'; import {ImportedSymbolsTracker} from '../../../ngtsc/imports'; import {ReflectionHost} from '../../../ngtsc/reflection'; -import {addImports} from '../../../ngtsc/transform'; -import {ImportManager} from '../../../ngtsc/translator'; +import {ImportManagerV2} from '../../../ngtsc/translator'; import {signalInputsTransform} from './input_function'; import {signalModelTransform} from './model_function'; @@ -48,7 +47,7 @@ export function getInitializerApiJitTransform( ): ts.TransformerFactory { return ctx => { return sourceFile => { - const importManager = new ImportManager(undefined, undefined, ctx.factory); + const importManager = new ImportManagerV2(); sourceFile = ts.visitNode( sourceFile, @@ -56,12 +55,7 @@ export function getInitializerApiJitTransform( ts.isSourceFile, ); - const newImports = importManager.getAllImports(sourceFile.fileName); - if (newImports.length > 0) { - sourceFile = addImports(ctx.factory, importManager, sourceFile); - } - - return sourceFile; + return importManager.transformTsFile(ctx, sourceFile); }; }; } @@ -69,7 +63,7 @@ export function getInitializerApiJitTransform( function createTransformVisitor( ctx: ts.TransformationContext, host: ReflectionHost, - importManager: ImportManager, + importManager: ImportManagerV2, importTracker: ImportedSymbolsTracker, isCore: boolean, ): ts.Visitor { diff --git a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform_api.ts b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform_api.ts index bad537e6acbfd..55db779fdcca6 100644 --- a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform_api.ts +++ b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform_api.ts @@ -10,13 +10,13 @@ import ts from 'typescript'; import {ImportedSymbolsTracker} from '../../../ngtsc/imports'; import {Decorator, ReflectionHost} from '../../../ngtsc/reflection'; -import {ImportManager} from '../../../ngtsc/translator'; +import {ImportManagerV2} from '../../../ngtsc/translator'; /** Function that can be used to transform class properties. */ export type PropertyTransform = (node: ts.PropertyDeclaration&{name: ts.Identifier | ts.StringLiteralLike}, host: ReflectionHost, factory: ts.NodeFactory, importTracker: ImportedSymbolsTracker, - importManager: ImportManager, classDecorator: Decorator, isCore: boolean) => + importManager: ImportManagerV2, classDecorator: Decorator, isCore: boolean) => ts.PropertyDeclaration; /** @@ -26,14 +26,18 @@ export type PropertyTransform = * decorator downlevel transform. */ export function createSyntheticAngularCoreDecoratorAccess( - factory: ts.NodeFactory, importManager: ImportManager, ngClassDecorator: Decorator, - decoratorName: string): ts.PropertyAccessExpression { + factory: ts.NodeFactory, importManager: ImportManagerV2, ngClassDecorator: Decorator, + sourceFile: ts.SourceFile, decoratorName: string): ts.PropertyAccessExpression { const classDecoratorIdentifier = ts.isIdentifier(ngClassDecorator.identifier) ? ngClassDecorator.identifier : ngClassDecorator.identifier.expression; return factory.createPropertyAccessExpression( - importManager.generateNamespaceImport('@angular/core'), + importManager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: null, + requestedFile: sourceFile, + }), // The synthetic identifier may be checked later by the downlevel decorators // transform to resolve to an Angular import using `getSymbolAtLocation`. We trick // the transform to think it's not synthetic and comes from Angular core. diff --git a/packages/compiler-cli/test/signal_queries_metadata_transform_spec.ts b/packages/compiler-cli/test/signal_queries_metadata_transform_spec.ts index e5ccc24cd7d16..0e50e21175f6a 100644 --- a/packages/compiler-cli/test/signal_queries_metadata_transform_spec.ts +++ b/packages/compiler-cli/test/signal_queries_metadata_transform_spec.ts @@ -137,12 +137,12 @@ describe('signal queries metadata transform', () => { expect(result).toContain(omitLeadingWhitespace(` __decorate([ - i0.ViewChild('el', { ...{ read: SomeToken }, isSignal: true }) + bla.ViewChild('el', { ...{ read: SomeToken }, isSignal: true }) ], MyDir.prototype, "el", void 0); `)); expect(result).toContain(omitLeadingWhitespace(` __decorate([ - i0.ViewChild('el', { ...{ read: bla.Component }, isSignal: true }) + bla.ViewChild('el', { ...{ read: bla.Component }, isSignal: true }) ], MyDir.prototype, "el2", void 0); `)); }); From 372e6046403ac1f1774758d5450bb5fc807a753f Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 11 Mar 2024 14:27:27 +0000 Subject: [PATCH 06/10] test(compiler-cli): enable incremental re-use type checking with signal inputs Enables the incremental type-checking test that we never enabled when we landed signal inputs. Now that we fixed incremental re-use by re-using the existing user imports for inline type check blocks, the test is passing and can be enabled. --- .../test/ngtsc/incremental_typecheck_spec.ts | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/compiler-cli/test/ngtsc/incremental_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/incremental_typecheck_spec.ts index 6d114d3bbb10e..5512fc5036ca3 100644 --- a/packages/compiler-cli/test/ngtsc/incremental_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/incremental_typecheck_spec.ts @@ -1601,11 +1601,10 @@ runInEachFileSystem(() => { expectCompleteReuse(env.getReuseTsProgram()); }); - // TODO(devversion): look into fixing this for inline TCB and inline type ctors. - xit('should completely re-use structure when an inline constructor generic directive starts using input signals', - () => { - env.tsconfig({strictTemplates: true}); - env.write('dir.ts', ` + it('should completely re-use structure when an inline constructor generic directive starts using input signals', + () => { + env.tsconfig({strictTemplates: true}); + env.write('dir.ts', ` import {Directive, Input} from '@angular/core'; class SomeNonExportedClass {} @@ -1617,7 +1616,7 @@ runInEachFileSystem(() => { @Input() dir: T|undefined; } `); - env.write('cmp.ts', ` + env.write('cmp.ts', ` import {Component} from '@angular/core'; @Component({ @@ -1628,7 +1627,7 @@ runInEachFileSystem(() => { foo = 'foo'; } `); - env.write('mod.ts', ` + env.write('mod.ts', ` import {NgModule} from '@angular/core'; import {Cmp} from './cmp'; import {Dir} from './dir'; @@ -1638,10 +1637,10 @@ runInEachFileSystem(() => { }) export class Mod {} `); - env.driveMain(); + env.driveMain(); - // turn the input into a signal input- causing a new import. - env.write('dir.ts', ` + // turn the input into a signal input- causing a new import. + env.write('dir.ts', ` import {Directive, input} from '@angular/core'; class SomeNonExportedClass {} @@ -1653,11 +1652,11 @@ runInEachFileSystem(() => { dir = input.required(); } `); - env.driveMain(); + env.driveMain(); - expectCompleteReuse(env.getTsProgram()); - expectCompleteReuse(env.getReuseTsProgram()); - }); + expectCompleteReuse(env.getTsProgram()); + expectCompleteReuse(env.getReuseTsProgram()); + }); }); }); }); From fae49744e91fdcbb4070102bde7ace765ed0293e Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 11 Mar 2024 15:50:00 +0000 Subject: [PATCH 07/10] refactor(compiler-cli): delete old unused `ImportManager` This commit deletes the older and now unused `ImportManager`. --- .../compiler-cli/src/ngtsc/transform/index.ts | 1 - .../src/ngtsc/transform/src/utils.ts | 80 ---------------- .../src/ngtsc/translator/index.ts | 3 +- .../ngtsc/translator/src/import_manager.ts | 95 ------------------- 4 files changed, 1 insertion(+), 178 deletions(-) delete mode 100644 packages/compiler-cli/src/ngtsc/transform/src/utils.ts delete mode 100644 packages/compiler-cli/src/ngtsc/translator/src/import_manager.ts diff --git a/packages/compiler-cli/src/ngtsc/transform/index.ts b/packages/compiler-cli/src/ngtsc/transform/index.ts index 33a4e5a68c29f..e3da19c3b15f3 100644 --- a/packages/compiler-cli/src/ngtsc/transform/index.ts +++ b/packages/compiler-cli/src/ngtsc/transform/index.ts @@ -12,4 +12,3 @@ export {ClassRecord, TraitCompiler} from './src/compilation'; export {declarationTransformFactory, DtsTransformRegistry, IvyDeclarationDtsTransform} from './src/declaration'; export {AnalyzedTrait, PendingTrait, ResolvedTrait, SkippedTrait, Trait, TraitState} from './src/trait'; export {ivyTransformFactory} from './src/transform'; -export {addImports} from './src/utils'; diff --git a/packages/compiler-cli/src/ngtsc/transform/src/utils.ts b/packages/compiler-cli/src/ngtsc/transform/src/utils.ts deleted file mode 100644 index 58e61a5859d8b..0000000000000 --- a/packages/compiler-cli/src/ngtsc/transform/src/utils.ts +++ /dev/null @@ -1,80 +0,0 @@ -/** - * @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 {ImportManager, NamespaceImport, SideEffectImport} from '../../translator'; - -/** - * Adds extra imports in the import manage for this source file, after the existing imports - * and before the module body. - * Can optionally add extra statements (e.g. new constants) before the body as well. - */ -export function addImports( - factory = ts.factory, importManager: ImportManager, sf: ts.SourceFile, - extraStatements: ts.Statement[] = []): ts.SourceFile { - // Generate the import statements to prepend. - const addedImports = importManager.getAllImports(sf.fileName) - .map( - i => i.qualifier !== null ? createNamespaceImportDecl(i, factory) : - createSideEffectImportDecl(i, factory)); - - // Filter out the existing imports and the source file body. All new statements - // will be inserted between them. - const existingImports = sf.statements.filter(stmt => isImportStatement(stmt)); - const body = sf.statements.filter(stmt => !isImportStatement(stmt)); - // Prepend imports if needed. - if (addedImports.length > 0) { - // If we prepend imports, we also prepend NotEmittedStatement to use it as an anchor - // for @fileoverview Closure annotation. If there is no @fileoverview annotations, this - // statement would be a noop. - const fileoverviewAnchorStmt = factory.createNotEmittedStatement(sf); - return factory.updateSourceFile(sf, factory.createNodeArray([ - fileoverviewAnchorStmt, ...existingImports, ...addedImports, ...extraStatements, ...body - ])); - } - - return sf; -} - -function createNamespaceImportDecl( - i: NamespaceImport, factory: ts.NodeFactory): ts.ImportDeclaration { - const qualifier = factory.createIdentifier(i.qualifier.text); - const importClause = factory.createImportClause( - /* isTypeOnly */ false, - /* name */ undefined, - /* namedBindings */ factory.createNamespaceImport(qualifier)); - const decl = factory.createImportDeclaration( - /* modifiers */ undefined, - /* importClause */ importClause, - /* moduleSpecifier */ factory.createStringLiteral(i.specifier)); - - // Set the qualifier's original TS node to the `ts.ImportDeclaration`. This allows downstream - // transforms such as tsickle to properly process references to this import. - // - // This operation is load-bearing in g3 as some imported modules contain special metadata - // generated by clutz, which tsickle uses to transform imports and references to those imports. - // - // TODO(alxhub): add a test for this when tsickle is updated externally to depend on this - // behavior. - ts.setOriginalNode(i.qualifier, decl); - - return decl; -} - -function createSideEffectImportDecl( - i: SideEffectImport, factory: ts.NodeFactory): ts.ImportDeclaration { - return factory.createImportDeclaration( - /* modifiers */ undefined, - /* importClause */ undefined, - /* moduleSpecifier */ ts.factory.createStringLiteral(i.specifier)); -} - -function isImportStatement(stmt: ts.Statement): boolean { - return ts.isImportDeclaration(stmt) || ts.isImportEqualsDeclaration(stmt) || - ts.isNamespaceImport(stmt); -} diff --git a/packages/compiler-cli/src/ngtsc/translator/index.ts b/packages/compiler-cli/src/ngtsc/translator/index.ts index 8cc33918c8f02..8c2d75f168bef 100644 --- a/packages/compiler-cli/src/ngtsc/translator/index.ts +++ b/packages/compiler-cli/src/ngtsc/translator/index.ts @@ -7,9 +7,8 @@ */ export {AstFactory, BinaryOperator, LeadingComment, ObjectLiteralProperty, SourceMapLocation, SourceMapRange, TemplateElement, TemplateLiteral, UnaryOperator, VariableDeclarationType} from './src/api/ast_factory'; -export {ImportGenerator, ImportRequest, NamedImport} from './src/api/import_generator'; +export {ImportGenerator, ImportRequest} 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'; diff --git a/packages/compiler-cli/src/ngtsc/translator/src/import_manager.ts b/packages/compiler-cli/src/ngtsc/translator/src/import_manager.ts deleted file mode 100644 index 72fca8de18be9..0000000000000 --- a/packages/compiler-cli/src/ngtsc/translator/src/import_manager.ts +++ /dev/null @@ -1,95 +0,0 @@ -/** - * @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 {ImportRewriter, NoopImportRewriter} from '../../imports'; - -/** - * Information about a namespace import that has been added to a module. - */ -export interface NamespaceImport { - /** The name of the module that has been imported. */ - specifier: string; - - /** The `ts.Identifier` by which the imported module is known. */ - qualifier: ts.Identifier; -} - -/** - * Information about a side effect import that has been added to a module. - */ -export interface SideEffectImport { - /** The name of the module that has been imported. */ - specifier: string; - - /** - * The qualifier of a side effect import is always non-existent, and that can be used to check - * whether the import is side effect or not. - */ - qualifier: null; -} - -/** - * Information about an import that has been added to a module. - */ -export type Import = NamespaceImport|SideEffectImport; - -export class ImportManager { - private specifierToIdentifier = new Map(); - private nextIndex = 0; - - constructor( - protected rewriter: ImportRewriter = new NoopImportRewriter(), private prefix = 'i', - private factory = ts.factory) {} - - generateNamespaceImport(moduleName: string): ts.Identifier { - // The case `specifierToIdentifier.get(moduleName) === null` is also considered to overwrite the - // side effect import since namedspace import is enough. - if (!this.specifierToIdentifier.has(moduleName) || - this.specifierToIdentifier.get(moduleName) === null) { - this.specifierToIdentifier.set( - moduleName, this.factory.createIdentifier(`${this.prefix}${this.nextIndex++}`)); - } - return this.specifierToIdentifier.get(moduleName)!; - } - - generateNamedImport(moduleName: string, originalSymbol: string): any { - // First, rewrite the symbol name. - const symbol = this.rewriter.rewriteSymbol(originalSymbol, moduleName); - - // Ask the rewriter if this symbol should be imported at all. If not, it can be referenced - // directly (moduleImport: null). - if (!this.rewriter.shouldImportSymbol(symbol, moduleName)) { - // The symbol should be referenced directly. - return {moduleImport: null, symbol}; - } - - // If not, this symbol will be imported using a generated namespace import. - const moduleImport = this.generateNamespaceImport(moduleName); - - return {moduleImport, symbol}; - } - - generateSideEffectImport(moduleName: string) { - if (!this.specifierToIdentifier.has(moduleName)) { - this.specifierToIdentifier.set(moduleName, null); - } - } - - getAllImports(contextPath: string): Import[] { - const imports: Import[] = []; - for (const [originalSpecifier, qualifier] of this.specifierToIdentifier) { - const specifier = this.rewriter.rewriteSpecifier(originalSpecifier, contextPath); - imports.push({ - specifier, - qualifier, - }); - } - return imports; - } -} From 5abb0dbb363edb8de4fa45fb8eb76d2b02aa5fdf Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 11 Mar 2024 15:58:06 +0000 Subject: [PATCH 08/10] refactor(compiler-cli): rename `ImportManagerV2` to `ImportManager` To ease review and to allow for both instances to co-exist, `ImportManagerV2` was introduced. This commit renames it to `ImportManager` now that we deleted the older one. --- .../annotations/common/test/metadata_spec.ts | 4 +- .../src/ngtsc/transform/src/api.ts | 8 +-- .../src/ngtsc/transform/src/declaration.ts | 10 ++-- .../src/ngtsc/transform/src/transform.ts | 6 +-- .../src/ngtsc/translator/index.ts | 2 +- .../src/import_manager/import_manager.ts | 2 +- .../import_typescript_transform.ts | 4 +- .../ngtsc/translator/src/type_translator.ts | 6 +-- .../translator/test/import_manager_spec.ts | 54 +++++++++---------- .../src/ngtsc/typecheck/src/context.ts | 19 +++---- .../src/ngtsc/typecheck/src/environment.ts | 4 +- .../src/reference_emit_environment.ts | 4 +- .../ngtsc/typecheck/src/type_check_file.ts | 4 +- .../model_function.ts | 4 +- .../initializer_api_transforms/transform.ts | 6 +-- .../transform_api.ts | 6 +-- 16 files changed, 70 insertions(+), 73 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts index 8fa09b359e680..17cdbdf6ab38b 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts @@ -12,7 +12,7 @@ import {absoluteFrom, getSourceFileOrError} from '../../../file_system'; import {runInEachFileSystem, TestFile} from '../../../file_system/testing'; import {TypeScriptReflectionHost} from '../../../reflection'; import {getDeclaration, makeProgram} from '../../../testing'; -import {ImportManagerV2, presetImportManagerForceNamespaceImports, translateStatement} from '../../../translator'; +import {ImportManager, presetImportManagerForceNamespaceImports, translateStatement} from '../../../translator'; import {extractClassMetadata} from '../src/metadata'; runInEachFileSystem(() => { @@ -132,7 +132,7 @@ runInEachFileSystem(() => { return ''; } const sf = getSourceFileOrError(program, _('/index.ts')); - const im = new ImportManagerV2(presetImportManagerForceNamespaceImports); + const im = new ImportManager(presetImportManagerForceNamespaceImports); const stmt = compileClassMetadata(call).toStmt(); const tsStatement = translateStatement(sf, stmt, im); const res = ts.createPrinter().printNode(ts.EmitHint.Unspecified, tsStatement, sf); diff --git a/packages/compiler-cli/src/ngtsc/transform/src/api.ts b/packages/compiler-cli/src/ngtsc/transform/src/api.ts index 83fe799962e8a..aeadd6e3810f7 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/api.ts @@ -13,7 +13,7 @@ import {Reexport, ReferenceEmitter} from '../../imports'; import {SemanticSymbol} from '../../incremental/semantic_graph'; import {IndexingContext} from '../../indexer'; import {ClassDeclaration, Decorator, ReflectionHost} from '../../reflection'; -import {ImportManagerV2} from '../../translator'; +import {ImportManager} from '../../translator'; import {TypeCheckContext} from '../../typecheck/api'; import {ExtendedTemplateChecker} from '../../typecheck/extended/api'; import {TemplateSemanticsChecker} from '../../typecheck/template_semantics/api/api'; @@ -264,11 +264,11 @@ export interface ResolveResult { } export interface DtsTransform { - transformClassElement?(element: ts.ClassElement, imports: ImportManagerV2): ts.ClassElement; + transformClassElement?(element: ts.ClassElement, imports: ImportManager): ts.ClassElement; transformFunctionDeclaration? - (element: ts.FunctionDeclaration, imports: ImportManagerV2): ts.FunctionDeclaration; + (element: ts.FunctionDeclaration, imports: ImportManager): ts.FunctionDeclaration; transformClass? (clazz: ts.ClassDeclaration, elements: ReadonlyArray, reflector: ReflectionHost, refEmitter: ReferenceEmitter, - imports: ImportManagerV2): ts.ClassDeclaration; + imports: ImportManager): ts.ClassDeclaration; } diff --git a/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts b/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts index 3f4f4ac5206c4..61cbd443ed2f6 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts @@ -11,7 +11,7 @@ import ts from 'typescript'; import {ImportRewriter, ReferenceEmitter} from '../../imports'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManagerV2, presetImportManagerForceNamespaceImports, translateType} from '../../translator'; +import {ImportManager, presetImportManagerForceNamespaceImports, translateType} from '../../translator'; import {DtsTransform} from './api'; @@ -84,7 +84,7 @@ class DtsTransformer { * Transform the declaration file and add any declarations which were recorded. */ transform(sf: ts.SourceFile, transforms: DtsTransform[]): ts.SourceFile { - const imports = new ImportManagerV2( + const imports = new ImportManager( {...presetImportManagerForceNamespaceImports, rewriter: this.importRewriter}); const visitor: ts.Visitor = (node: ts.Node): ts.VisitResult => { @@ -107,7 +107,7 @@ class DtsTransformer { private transformClassDeclaration( clazz: ts.ClassDeclaration, transforms: DtsTransform[], - imports: ImportManagerV2): ts.ClassDeclaration { + imports: ImportManager): ts.ClassDeclaration { let elements: ts.ClassElement[]|ReadonlyArray = clazz.members; let elementsChanged = false; @@ -156,7 +156,7 @@ class DtsTransformer { private transformFunctionDeclaration( declaration: ts.FunctionDeclaration, transforms: DtsTransform[], - imports: ImportManagerV2): ts.FunctionDeclaration { + imports: ImportManager): ts.FunctionDeclaration { let newDecl = declaration; for (const transform of transforms) { @@ -184,7 +184,7 @@ export class IvyDeclarationDtsTransform implements DtsTransform { transformClass( clazz: ts.ClassDeclaration, members: ReadonlyArray, reflector: ReflectionHost, refEmitter: ReferenceEmitter, - imports: ImportManagerV2): ts.ClassDeclaration { + imports: ImportManager): ts.ClassDeclaration { const original = ts.getOriginalNode(clazz) as ClassDeclaration; if (!this.declarationFields.has(original)) { diff --git a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts index c595e5341f1d3..954d3260f610b 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts @@ -13,7 +13,7 @@ import {DefaultImportTracker, ImportRewriter, LocalCompilationExtraImportsTracke import {getDefaultImportDeclaration} from '../../imports/src/default'; import {PerfPhase, PerfRecorder} from '../../perf'; import {Decorator, ReflectionHost} from '../../reflection'; -import {ImportManagerV2, presetImportManagerForceNamespaceImports, RecordWrappedNodeFn, translateExpression, translateStatement, TranslatorOptions} from '../../translator'; +import {ImportManager, presetImportManagerForceNamespaceImports, RecordWrappedNodeFn, translateExpression, translateStatement, TranslatorOptions} from '../../translator'; import {visit, VisitListEntryResult, Visitor} from '../../util/src/visitor'; import {CompileResult} from './api'; @@ -93,7 +93,7 @@ class IvyTransformationVisitor extends Visitor { constructor( private compilation: TraitCompiler, private classCompilationMap: Map, - private reflector: ReflectionHost, private importManager: ImportManagerV2, + private reflector: ReflectionHost, private importManager: ImportManager, private recordWrappedNodeExpr: RecordWrappedNodeFn, private isClosureCompilerEnabled: boolean, private isCore: boolean, private deferrableImports: Set) { @@ -292,7 +292,7 @@ function transformIvySourceFile( recordWrappedNode: RecordWrappedNodeFn): ts.SourceFile { const constantPool = new ConstantPool(isClosureCompilerEnabled); const importManager = - new ImportManagerV2({...presetImportManagerForceNamespaceImports, rewriter: importRewriter}); + new ImportManager({...presetImportManagerForceNamespaceImports, rewriter: importRewriter}); // The transformation process consists of 2 steps: // diff --git a/packages/compiler-cli/src/ngtsc/translator/index.ts b/packages/compiler-cli/src/ngtsc/translator/index.ts index 8c2d75f168bef..45edeedbb7b50 100644 --- a/packages/compiler-cli/src/ngtsc/translator/index.ts +++ b/packages/compiler-cli/src/ngtsc/translator/index.ts @@ -9,7 +9,7 @@ export {AstFactory, BinaryOperator, LeadingComment, ObjectLiteralProperty, SourceMapLocation, SourceMapRange, TemplateElement, TemplateLiteral, UnaryOperator, VariableDeclarationType} from './src/api/ast_factory'; export {ImportGenerator, ImportRequest} from './src/api/import_generator'; export {Context} from './src/context'; -export {ImportManagerConfig, ImportManagerV2, presetImportManagerForceNamespaceImports} from './src/import_manager/import_manager'; +export {ImportManager, ImportManagerConfig, 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'; diff --git a/packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_manager.ts b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_manager.ts index 5c917c0bcb5c7..f5c0ab66e074c 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_manager.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/import_manager/import_manager.ts @@ -55,7 +55,7 @@ export type ModuleName = string&{__moduleName: boolean}; * Those imports may be inserted via a TypeScript transform, or via manual string * manipulation using e.g. `magic-string`. */ -export class ImportManagerV2 implements +export class ImportManager implements ImportGenerator { /** List of new imports that will be inserted into given source files. */ private newImports: Map): ts.TransformerFactory { return (ctx) => { const {affectedFiles, newImports, updatedImports, reusedOriginalAliasDeclarations} = diff --git a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts index 4041e101d5bcf..c678a2701569f 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts @@ -13,21 +13,21 @@ import {assertSuccessfulReferenceEmit, ImportFlags, OwningModule, Reference, Ref import {AmbientImport, ReflectionHost} from '../../reflection'; import {Context} from './context'; -import {ImportManagerV2} from './import_manager/import_manager'; +import {ImportManager} from './import_manager/import_manager'; import {tsNumericExpression} from './ts_util'; import {TypeEmitter} from './type_emitter'; export function translateType( type: o.Type, contextFile: ts.SourceFile, reflector: ReflectionHost, - refEmitter: ReferenceEmitter, imports: ImportManagerV2): ts.TypeNode { + refEmitter: ReferenceEmitter, imports: ImportManager): ts.TypeNode { return type.visitType( new TypeTranslatorVisitor(imports, contextFile, reflector, refEmitter), new Context(false)); } class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor { constructor( - private imports: ImportManagerV2, private contextFile: ts.SourceFile, + private imports: ImportManager, private contextFile: ts.SourceFile, private reflector: ReflectionHost, private refEmitter: ReferenceEmitter) {} visitBuiltinType(type: o.BuiltinType, context: Context): ts.KeywordTypeNode { diff --git a/packages/compiler-cli/src/ngtsc/translator/test/import_manager_spec.ts b/packages/compiler-cli/src/ngtsc/translator/test/import_manager_spec.ts index 6e848c657ce4c..7f87d1e58c324 100644 --- a/packages/compiler-cli/src/ngtsc/translator/test/import_manager_spec.ts +++ b/packages/compiler-cli/src/ngtsc/translator/test/import_manager_spec.ts @@ -11,12 +11,12 @@ import ts from 'typescript'; import {absoluteFrom} from '../../file_system'; import {initMockFileSystem} from '../../file_system/testing'; import {NgtscTestCompilerHost} from '../../testing'; -import {ImportManagerV2} from '../src/import_manager/import_manager'; +import {ImportManager} from '../src/import_manager/import_manager'; describe('import manager', () => { it('should be possible to import a symbol', () => { const {testFile, emit} = createTestProgram(''); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const ref = manager.addImport({ exportModuleSpecifier: '@angular/core', exportSymbolName: 'input', @@ -35,7 +35,7 @@ describe('import manager', () => { it('should be possible to import a namespace', () => { const {testFile, emit} = createTestProgram(''); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const ref = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -55,7 +55,7 @@ describe('import manager', () => { it('should be possible to import multiple symbols', () => { const {testFile, emit} = createTestProgram(''); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -83,7 +83,7 @@ describe('import manager', () => { it('should be possible to import multiple namespaces', () => { const {testFile, emit} = createTestProgram(''); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const coreNamespace = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -112,7 +112,7 @@ describe('import manager', () => { it('should be possible to generate a namespace import and re-use it for future symbols', () => { const {testFile, emit} = createTestProgram(''); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const coreNamespace = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -140,7 +140,7 @@ describe('import manager', () => { it('should always generate a new namespace import if there is only a named import', () => { const {testFile, emit} = createTestProgram(''); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -171,7 +171,7 @@ describe('import manager', () => { const {testFile, emit} = createTestProgram(` import * as existingImport from '@angular/core'; `); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -193,7 +193,7 @@ describe('import manager', () => { const {testFile, emit} = createTestProgram(` import * as existingImport from '@angular/core'; `); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const coreRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -215,7 +215,7 @@ describe('import manager', () => { const {testFile, emit} = createTestProgram(` import {input} from '@angular/core'; `); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -239,7 +239,7 @@ describe('import manager', () => { const x = input(); `); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const outputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -264,7 +264,7 @@ describe('import manager', () => { const {testFile, emit} = createTestProgram(` import {input} from '@angular/core'; `); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const outputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -286,7 +286,7 @@ describe('import manager', () => { const {testFile, emit} = createTestProgram(` import {input} from '@angular/core'; `); - const manager = new ImportManagerV2({ + const manager = new ImportManager({ disableOriginalSourceFileReuse: true, }); @@ -310,7 +310,7 @@ describe('import manager', () => { const {testFile, emit} = createTestProgram(` import * as existingCore from '@angular/core'; `); - const manager = new ImportManagerV2({ + const manager = new ImportManager({ disableOriginalSourceFileReuse: true, }); @@ -332,7 +332,7 @@ describe('import manager', () => { it('should be able to always prefer namespace imports for new imports', () => { const {testFile, emit} = createTestProgram(``); - const manager = new ImportManagerV2({ + const manager = new ImportManager({ forceGenerateNamespacesForNewImports: true, }); @@ -366,7 +366,7 @@ describe('import manager', () => { const {testFile, emit} = createTestProgram(` import * as existingNamespace from '@angular/core'; `); - const manager = new ImportManagerV2({ + const manager = new ImportManager({ forceGenerateNamespacesForNewImports: true, }); @@ -402,7 +402,7 @@ describe('import manager', () => { const x = new Dir(); `); - const manager = new ImportManagerV2({ + const manager = new ImportManager({ forceGenerateNamespacesForNewImports: true, }); @@ -429,7 +429,7 @@ describe('import manager', () => { console.log(MyComp); `); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -451,7 +451,7 @@ describe('import manager', () => { it('should be able to add a side effect import', () => { const {testFile, emit} = createTestProgram(``); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); manager.addSideEffectImport(testFile, '@angular/core'); @@ -466,7 +466,7 @@ describe('import manager', () => { const {testFile, emit} = createTestProgram(` const input = 1; `); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -493,7 +493,7 @@ describe('import manager', () => { }; } `); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -518,7 +518,7 @@ describe('import manager', () => { it('should avoid an import alias specifier if identifier is free to use', () => { const {testFile, emit} = createTestProgram(``); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -538,7 +538,7 @@ describe('import manager', () => { it('should avoid collisions with generated identifiers', () => { const {testFile, emit} = createTestProgram(``); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -567,7 +567,7 @@ describe('import manager', () => { it('should avoid collisions with generated identifiers', () => { const {testFile, emit} = createTestProgram(``); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -596,7 +596,7 @@ describe('import manager', () => { it('should re-use previous similar generated imports', () => { const {testFile, emit} = createTestProgram(``); - const manager = new ImportManagerV2(); + const manager = new ImportManager(); const inputRef = manager.addImport({ exportModuleSpecifier: '@angular/core', @@ -668,7 +668,7 @@ describe('import manager', () => { function createTestProgram(text: string): { testFile: ts.SourceFile, - emit: (manager: ImportManagerV2, extraStatements: ts.Statement[]) => string, + emit: (manager: ImportManager, extraStatements: ts.Statement[]) => string, } { const fs = initMockFileSystem('Native'); const options: ts.CompilerOptions = { @@ -693,7 +693,7 @@ function createTestProgram(text: string): { throw new Error('Could not get test source file from program.'); } - const emit = (manager: ImportManagerV2, newStatements: ts.Statement[]) => { + const emit = (manager: ImportManager, newStatements: ts.Statement[]) => { const transformer = manager.toTsTransform(new Map([[testFile.fileName, newStatements]])); let emitResult: string|null = null; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index 5f947c5843ac2..cf743b18ec211 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -17,7 +17,7 @@ import {PipeMeta} from '../../metadata'; import {PerfEvent, PerfRecorder} from '../../perf'; import {FileUpdate} from '../../program_driver'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManagerV2} from '../../translator'; +import {ImportManager} from '../../translator'; import {TemplateDiagnostic, TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api'; import {makeTemplateDiagnostic} from '../diagnostics'; @@ -363,7 +363,7 @@ export class TypeCheckContextImpl implements TypeCheckContext { // Imports may need to be added to the file to support type-checking of directives // used in the template within it. - const importManager = new ImportManagerV2({ + const importManager = new ImportManager({ // This minimizes noticeable changes with older versions of `ImportManager`. forceGenerateNamespacesForNewImports: true, // Type check block code affects code completion and fix suggestions. @@ -537,9 +537,8 @@ interface Op { /** * Execute the operation and return the generated code as text. */ - execute( - im: ImportManagerV2, sf: ts.SourceFile, refEmitter: ReferenceEmitter, - printer: ts.Printer): string; + execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer): + string; } /** @@ -559,9 +558,8 @@ class InlineTcbOp implements Op { return this.ref.node.end + 1; } - execute( - im: ImportManagerV2, sf: ts.SourceFile, refEmitter: ReferenceEmitter, - printer: ts.Printer): string { + execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer): + string { const env = new Environment(this.config, im, refEmitter, this.reflector, sf); const fnName = ts.factory.createIdentifier(`_tcb_${this.ref.node.pos}`); @@ -590,9 +588,8 @@ class TypeCtorOp implements Op { return this.ref.node.end - 1; } - execute( - im: ImportManagerV2, sf: ts.SourceFile, refEmitter: ReferenceEmitter, - printer: ts.Printer): string { + execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer): + string { const emitEnv = new ReferenceEmitEnvironment(im, refEmitter, this.reflector, sf); const tcb = generateInlineTypeCtor(emitEnv, this.ref.node, this.meta); return printer.printNode(ts.EmitHint.Unspecified, tcb, sf); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index ce5acb42e1d09..d1b184d49bcb2 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -10,7 +10,7 @@ import ts from 'typescript'; import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../imports'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManagerV2, translateExpression} from '../../translator'; +import {ImportManager, translateExpression} from '../../translator'; import {TypeCheckableDirectiveMeta, TypeCheckingConfig, TypeCtorMetadata} from '../api'; import {ReferenceEmitEnvironment} from './reference_emit_environment'; @@ -42,7 +42,7 @@ export class Environment extends ReferenceEmitEnvironment { protected pipeInstStatements: ts.Statement[] = []; constructor( - readonly config: TypeCheckingConfig, importManager: ImportManagerV2, + readonly config: TypeCheckingConfig, importManager: ImportManager, refEmitter: ReferenceEmitter, reflector: ReflectionHost, contextFile: ts.SourceFile) { super(importManager, refEmitter, reflector, contextFile); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts index 45d984a39079c..1a70753c2883c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/reference_emit_environment.ts @@ -11,7 +11,7 @@ import ts from 'typescript'; import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitKind, ReferenceEmitter} from '../../imports'; import {ReflectionHost} from '../../reflection'; -import {ImportManagerV2, translateExpression, translateType} from '../../translator'; +import {ImportManager, translateExpression, translateType} from '../../translator'; /** * An environment for a given source file that can be used to emit references. @@ -21,7 +21,7 @@ import {ImportManagerV2, translateExpression, translateType} from '../../transla */ export class ReferenceEmitEnvironment { constructor( - readonly importManager: ImportManagerV2, protected refEmitter: ReferenceEmitter, + readonly importManager: ImportManager, protected refEmitter: ReferenceEmitter, readonly reflector: ReflectionHost, public contextFile: ts.SourceFile) {} canReferenceType( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts index 257100b91fd59..d343137949976 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts @@ -10,7 +10,7 @@ import ts from 'typescript'; import {AbsoluteFsPath, join} from '../../file_system'; import {Reference, ReferenceEmitter} from '../../imports'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportManagerV2} from '../../translator'; +import {ImportManager} from '../../translator'; import {TypeCheckBlockMetadata, TypeCheckingConfig} from '../api'; import {DomSchemaChecker} from './dom'; @@ -37,7 +37,7 @@ export class TypeCheckFile extends Environment { readonly fileName: AbsoluteFsPath, config: TypeCheckingConfig, refEmitter: ReferenceEmitter, reflector: ReflectionHost, compilerHost: Pick) { super( - config, new ImportManagerV2({ + config, new ImportManager({ // This minimizes noticeable changes with older versions of `ImportManager`. forceGenerateNamespacesForNewImports: true, // Type check block code affects code completion and fix suggestions. diff --git a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/model_function.ts b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/model_function.ts index d006e2d5dba64..8abd956c51abd 100644 --- a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/model_function.ts +++ b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/model_function.ts @@ -10,7 +10,7 @@ import {Decorator} from '@angular/compiler-cli/src/ngtsc/reflection'; import ts from 'typescript'; import {isAngularDecorator, tryParseSignalModelMapping} from '../../../ngtsc/annotations'; -import {ImportManagerV2} from '../../../ngtsc/translator'; +import {ImportManager} from '../../../ngtsc/translator'; import {createSyntheticAngularCoreDecoratorAccess, PropertyTransform} from './transform_api'; @@ -78,7 +78,7 @@ export const signalModelTransform: PropertyTransform = ( function createDecorator( name: string, config: ts.Expression, classDecorator: Decorator, factory: ts.NodeFactory, - sourceFile: ts.SourceFile, importManager: ImportManagerV2): ts.Decorator { + sourceFile: ts.SourceFile, importManager: ImportManager): ts.Decorator { const callTarget = createSyntheticAngularCoreDecoratorAccess( factory, importManager, classDecorator, sourceFile, name); diff --git a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform.ts b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform.ts index 6c309b35bdb4c..b6c910ce80c70 100644 --- a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform.ts +++ b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform.ts @@ -11,7 +11,7 @@ import ts from 'typescript'; import {isAngularDecorator} from '../../../ngtsc/annotations'; import {ImportedSymbolsTracker} from '../../../ngtsc/imports'; import {ReflectionHost} from '../../../ngtsc/reflection'; -import {ImportManagerV2} from '../../../ngtsc/translator'; +import {ImportManager} from '../../../ngtsc/translator'; import {signalInputsTransform} from './input_function'; import {signalModelTransform} from './model_function'; @@ -47,7 +47,7 @@ export function getInitializerApiJitTransform( ): ts.TransformerFactory { return ctx => { return sourceFile => { - const importManager = new ImportManagerV2(); + const importManager = new ImportManager(); sourceFile = ts.visitNode( sourceFile, @@ -63,7 +63,7 @@ export function getInitializerApiJitTransform( function createTransformVisitor( ctx: ts.TransformationContext, host: ReflectionHost, - importManager: ImportManagerV2, + importManager: ImportManager, importTracker: ImportedSymbolsTracker, isCore: boolean, ): ts.Visitor { diff --git a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform_api.ts b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform_api.ts index 55db779fdcca6..1cfaa1139a358 100644 --- a/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform_api.ts +++ b/packages/compiler-cli/src/transformers/jit_transforms/initializer_api_transforms/transform_api.ts @@ -10,13 +10,13 @@ import ts from 'typescript'; import {ImportedSymbolsTracker} from '../../../ngtsc/imports'; import {Decorator, ReflectionHost} from '../../../ngtsc/reflection'; -import {ImportManagerV2} from '../../../ngtsc/translator'; +import {ImportManager} from '../../../ngtsc/translator'; /** Function that can be used to transform class properties. */ export type PropertyTransform = (node: ts.PropertyDeclaration&{name: ts.Identifier | ts.StringLiteralLike}, host: ReflectionHost, factory: ts.NodeFactory, importTracker: ImportedSymbolsTracker, - importManager: ImportManagerV2, classDecorator: Decorator, isCore: boolean) => + importManager: ImportManager, classDecorator: Decorator, isCore: boolean) => ts.PropertyDeclaration; /** @@ -26,7 +26,7 @@ export type PropertyTransform = * decorator downlevel transform. */ export function createSyntheticAngularCoreDecoratorAccess( - factory: ts.NodeFactory, importManager: ImportManagerV2, ngClassDecorator: Decorator, + factory: ts.NodeFactory, importManager: ImportManager, ngClassDecorator: Decorator, sourceFile: ts.SourceFile, decoratorName: string): ts.PropertyAccessExpression { const classDecoratorIdentifier = ts.isIdentifier(ngClassDecorator.identifier) ? ngClassDecorator.identifier : From 581c4e07f57ccf69a827b9738c2da5e7b38e27c8 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 11 Mar 2024 16:18:51 +0000 Subject: [PATCH 09/10] test(compiler-cli): add tests to verify import generation in TCB files/blocks This commit adds some unit tests verifying the import generation in TCB files and inline blocks. We don't seem to have any unit tests for these in general. This commit adds some, verifying some characteristics we would like to guarantee. --- .../typecheck/test/type_check_block_spec.ts | 56 ++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index cb9af4fee4706..79ef478a3851e 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -8,10 +8,11 @@ import ts from 'typescript'; +import {absoluteFrom, getSourceFileOrError} from '../../file_system'; import {initMockFileSystem} from '../../file_system/testing'; import {Reference} from '../../imports'; -import {TypeCheckingConfig} from '../api'; -import {ALL_ENABLED_CONFIG, tcb, TestDeclaration, TestDirective} from '../testing'; +import {OptimizeFor, TypeCheckingConfig} from '../api'; +import {ALL_ENABLED_CONFIG, setup, tcb, TestDeclaration, TestDirective} from '../testing'; describe('type check blocks', () => { @@ -1744,4 +1745,55 @@ describe('type check blocks', () => { expect(result).toContain('(this).trackingFn(_t2, _t1, ((this).prop));'); }); }); + + describe('import generation', () => { + const TEMPLATE = `
`; + const DIRECTIVE: TestDeclaration = { + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: { + test: { + isSignal: true, + bindingPropertyName: 'test', + classPropertyName: 'test', + required: true, + transform: null, + } + } + }; + + it('should prefer namespace imports in type check files for new imports', () => { + const result = tcb(TEMPLATE, [DIRECTIVE]); + + expect(result).toContain(`import * as i1 from '@angular/core';`); + expect(result).toContain(`[i1.ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]`); + }); + + it('should re-use existing imports from original source files', () => { + // This is especially important for inline type check blocks. + // See: https://github.com/angular/angular/pull/53521#pullrequestreview-1778130879. + const {templateTypeChecker, program, programStrategy} = setup([{ + fileName: absoluteFrom('/test.ts'), + templates: {'AppComponent': TEMPLATE}, + declarations: [DIRECTIVE], + source: ` + import {Component} from '@angular/core'; // should be re-used + + class AppComponent {} + export class Dir {} + `, + }]); + + // Trigger type check block generation. + templateTypeChecker.getDiagnosticsForFile( + getSourceFileOrError(program, absoluteFrom('/test.ts')), OptimizeFor.SingleFile); + + const testSf = getSourceFileOrError(programStrategy.getProgram(), absoluteFrom('/test.ts')); + expect(testSf.text) + .toContain( + `import { Component, ɵINPUT_SIGNAL_BRAND_WRITE_TYPE } from '@angular/core'; // should be re-used`); + expect(testSf.text).toContain(`[ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]`); + }); + }); }); From 2659d77aa091eb7765a01e8af307bdc48ca4819e Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 13 Mar 2024 11:58:11 +0000 Subject: [PATCH 10/10] refactor(compiler-cli): properly preserve file overview comments This commit updates the logic for preserving file overview comments to be more reliable and less dependent on previous transforms. Previously, with the old import manager, we had a utility called `addImport` that always separated import statements and non-import statements. This meant that the non-emitted statement from Tsickle for the synthetic file-overview comments no longer lived at the beginning of the file. `addImports` tried to overcome this by adding another new non-emitted statement *before* all imports. This then was later used by the transform (or was assumed!) to attach the synthetic file overview comments if the original tsickle AST Node is no longer at the top. This logic can be improved, because the import manager shouldn't need to bother about this fileoverview non-emitted statement, and the logic for re-attaching the fileoverview comment should be local. This commit fixes this and makes it a local transform. --- .../src/ngtsc/transform/src/transform.ts | 16 ++++- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 59 ++++++++++++++++++- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts index 954d3260f610b..47921b4380cd2 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts @@ -341,7 +341,7 @@ function transformIvySourceFile( sf = importManager.transformTsFile(context, sf, constants); if (fileOverviewMeta !== null) { - setFileOverviewComment(sf, fileOverviewMeta); + sf = insertFileOverviewComment(sf, fileOverviewMeta); } return sf; @@ -382,7 +382,8 @@ function getFileOverviewComment(statements: ts.NodeArray): FileOve return null; } -function setFileOverviewComment(sf: ts.SourceFile, fileoverview: FileOverviewMeta): void { +function insertFileOverviewComment( + sf: ts.SourceFile, fileoverview: FileOverviewMeta): ts.SourceFile { const {comments, host, trailing} = fileoverview; // If host statement is no longer the first one, it means that extra statements were added at the // very beginning, so we need to relocate @fileoverview comment and cleanup the original statement @@ -393,8 +394,17 @@ function setFileOverviewComment(sf: ts.SourceFile, fileoverview: FileOverviewMet } else { ts.setSyntheticLeadingComments(host, undefined); } - ts.setSyntheticLeadingComments(sf.statements[0], comments); + + // Note: Do not use the first statement as it may be elided at runtime. + // E.g. an import declaration that is type only. + const commentNode = ts.factory.createNotEmittedStatement(sf); + ts.setSyntheticLeadingComments(commentNode, comments); + + return ts.factory.updateSourceFile( + sf, [commentNode, ...sf.statements], sf.isDeclarationFile, sf.referencedFiles, + sf.typeReferenceDirectives, sf.hasNoDefaultLib, sf.libReferenceDirectives); } + return sf; } function maybeFilterDecorator( diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index f6349f24a897e..7f9d2df4ee84b 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -6,11 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ +import {NgtscProgram} from '@angular/compiler-cli/src/ngtsc/program'; +import {CompilerOptions} from '@angular/compiler-cli/src/transformers/api'; +import {createCompilerHost} from '@angular/compiler-cli/src/transformers/compiler_host'; import {platform} from 'os'; import ts from 'typescript'; import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics'; -import {absoluteFrom} from '../../src/ngtsc/file_system'; +import {absoluteFrom, NgtscCompilerHost} from '../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing'; import {loadStandardTestFiles} from '../../src/ngtsc/testing'; import {restoreTypeScriptVersionForTesting, setTypeScriptVersionForTesting} from '../../src/typescript_support'; @@ -9294,6 +9297,60 @@ function allTests(os: string) { .toContain( '{ i0.ɵɵtwoWayBindingSet(ctx.a, $event) || (ctx.a = $event); return $event; }'); }); + + describe('tsickle compatibility', () => { + it('should preserve fileoverview comments', () => { + env.write('test.ts', ` + // type-only import that will be elided. + import {SimpleChanges} from '@angular/core'; + + export class X { + p: SimpleChanges = null!; + } + `); + + const options: CompilerOptions = { + strict: true, + strictTemplates: true, + target: ts.ScriptTarget.Latest, + module: ts.ModuleKind.ESNext, + annotateForClosureCompiler: true, + }; + + const program = new NgtscProgram(['/test.ts'], options, createCompilerHost({options})); + const transformers = program.compiler.prepareEmit().transformers; + + // Add a "fake tsickle" transform before Angular's transform. + transformers.before!.unshift(ctx => (sf: ts.SourceFile) => { + const tsickleFileOverview = ctx.factory.createNotEmittedStatement(sf); + ts.setSyntheticLeadingComments(tsickleFileOverview, [ + { + kind: ts.SyntaxKind.MultiLineCommentTrivia, + text: `* + * @fileoverview Closure comment + * @supress bla1 + * @supress bla2 + `, + pos: -1, + end: -1, + hasTrailingNewLine: true, + }, + ]); + return ctx.factory.updateSourceFile( + sf, [tsickleFileOverview, ...sf.statements], sf.isDeclarationFile, + sf.referencedFiles, sf.typeReferenceDirectives, sf.hasNoDefaultLib, + sf.libReferenceDirectives); + }); + + const {diagnostics, emitSkipped} = + program.getTsProgram().emit(undefined, undefined, undefined, undefined, transformers); + + expect(diagnostics.length).toBe(0); + expect(emitSkipped).toBe(false); + + expect(env.getContents('/test.js')).toContain(`* @fileoverview Closure comment`); + }); + }); }); });