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 0ab55e1b613..a37224ff082 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 @@ -75,6 +75,13 @@ export class ImportManager } > = new Map(); + /** + * Keeps track of imports marked for removal. The root-level key is the file from which the + * import should be removed, the inner map key is the name of the module from which the symbol + * is being imported. The value of the inner map is a set of symbol names that should be removed. + * Note! the inner map tracks the original names of the imported symbols, not their local aliases. + */ + private removedImports: Map>> = new Map(); private nextUniqueIndex = 0; private config: ImportManagerConfig; @@ -142,6 +149,14 @@ export class ImportManager ); } + // Remove the newly-added import from the set of removed imports. + if (request.exportSymbolName !== null && !request.asTypeReference) { + this.removedImports + .get(request.requestedFile) + ?.get(request.exportModuleSpecifier as ModuleName) + ?.delete(request.exportSymbolName); + } + // Attempt to re-use previous identical import requests. const previousGeneratedImportRef = attemptToReuseGeneratedImports( this.reuseGeneratedImportsTracker, @@ -157,6 +172,33 @@ export class ImportManager return createImportReference(!!request.asTypeReference, resultImportRef); } + /** + * Marks all imported symbols with a specific name for removal. + * Call `addImport` to undo this operation. + * @param requestedFile File from which to remove the imports. + * @param exportSymbolName Declared name of the symbol being removed. + * @param moduleSpecifier Module from which the symbol is being imported. + */ + removeImport( + requestedFile: ts.SourceFile, + exportSymbolName: string, + moduleSpecifier: string, + ): void { + let moduleMap = this.removedImports.get(requestedFile); + if (!moduleMap) { + moduleMap = new Map(); + this.removedImports.set(requestedFile, moduleMap); + } + + let removedSymbols = moduleMap.get(moduleSpecifier as ModuleName); + if (!removedSymbols) { + removedSymbols = new Set(); + moduleMap.set(moduleSpecifier as ModuleName, removedSymbols); + } + + removedSymbols.add(exportSymbolName); + } + private _generateNewImport( request: ImportRequest, ): ts.Identifier | [ts.Identifier, ts.Identifier] { @@ -255,10 +297,13 @@ export class ImportManager updatedImports: Map; newImports: Map; reusedOriginalAliasDeclarations: Set; + deletedImports: Set; } { const affectedFiles = new Set(); const updatedImportsResult = new Map(); const newImportsResult = new Map(); + const deletedImports = new Set(); + const importDeclarationsPerFile = new Map(); const addNewImport = (fileName: string, importDecl: ts.ImportDeclaration) => { affectedFiles.add(fileName); @@ -271,10 +316,11 @@ export class ImportManager // Collect original source file imports that need to be updated. this.reuseSourceFileImportsTracker.updatedImports.forEach((expressions, importDecl) => { + const sourceFile = importDecl.getSourceFile(); const namedBindings = importDecl.importClause!.namedBindings as ts.NamedImports; - const newNamedBindings = ts.factory.updateNamedImports( - namedBindings, - namedBindings.elements.concat( + const moduleName = (importDecl.moduleSpecifier as ts.StringLiteral).text as ModuleName; + const newElements = namedBindings.elements + .concat( expressions.map(({propertyName, fileUniqueAlias}) => ts.factory.createImportSpecifier( false, @@ -282,11 +328,60 @@ export class ImportManager fileUniqueAlias ?? propertyName, ), ), - ), - ); + ) + .filter((specifier) => this._canAddSpecifier(sourceFile, moduleName, specifier)); - affectedFiles.add(importDecl.getSourceFile().fileName); - updatedImportsResult.set(namedBindings, newNamedBindings); + affectedFiles.add(sourceFile.fileName); + + if (newElements.length === 0) { + deletedImports.add(importDecl); + } else { + updatedImportsResult.set( + namedBindings, + ts.factory.updateNamedImports(namedBindings, newElements), + ); + } + }); + + this.removedImports.forEach((removeMap, sourceFile) => { + if (removeMap.size === 0) { + return; + } + + let allImports = importDeclarationsPerFile.get(sourceFile); + + if (!allImports) { + allImports = sourceFile.statements.filter(ts.isImportDeclaration); + importDeclarationsPerFile.set(sourceFile, allImports); + } + + for (const node of allImports) { + if ( + !node.importClause?.namedBindings || + !ts.isNamedImports(node.importClause.namedBindings) || + this.reuseSourceFileImportsTracker.updatedImports.has(node) || + deletedImports.has(node) + ) { + continue; + } + + const namedBindings = node.importClause.namedBindings; + const moduleName = (node.moduleSpecifier as ts.StringLiteral).text as ModuleName; + const newImports = namedBindings.elements.filter((specifier) => + this._canAddSpecifier(sourceFile, moduleName, specifier), + ); + + if (newImports.length === 0) { + affectedFiles.add(sourceFile.fileName); + deletedImports.add(node); + } else if (newImports.length !== namedBindings.elements.length) { + affectedFiles.add(sourceFile.fileName); + updatedImportsResult.set( + namedBindings, + ts.factory.updateNamedImports(namedBindings, newImports), + ); + } + } }); // Collect all new imports to be added. Named imports, namespace imports or side-effects. @@ -324,17 +419,23 @@ export class ImportManager }); namedImports.forEach((specifiers, moduleName) => { - const newImport = ts.factory.createImportDeclaration( - undefined, - ts.factory.createImportClause( - false, - undefined, - ts.factory.createNamedImports(specifiers), - ), - ts.factory.createStringLiteral(moduleName, useSingleQuotes), + const filteredSpecifiers = specifiers.filter((specifier) => + this._canAddSpecifier(sourceFile, moduleName, specifier), ); - addNewImport(fileName, newImport); + if (filteredSpecifiers.length > 0) { + const newImport = ts.factory.createImportDeclaration( + undefined, + ts.factory.createImportClause( + false, + undefined, + ts.factory.createNamedImports(filteredSpecifiers), + ), + ts.factory.createStringLiteral(moduleName, useSingleQuotes), + ); + + addNewImport(fileName, newImport); + } }); }); @@ -343,6 +444,7 @@ export class ImportManager newImports: newImportsResult, updatedImports: updatedImportsResult, reusedOriginalAliasDeclarations: this.reuseSourceFileImportsTracker.reusedAliasDeclarations, + deletedImports, }; } @@ -387,6 +489,17 @@ export class ImportManager } return this.newImports.get(file)!; } + + private _canAddSpecifier( + sourceFile: ts.SourceFile, + moduleSpecifier: ModuleName, + specifier: ts.ImportSpecifier, + ): boolean { + return !this.removedImports + .get(sourceFile) + ?.get(moduleSpecifier) + ?.has((specifier.propertyName || specifier.name).text); + } } /** Creates an import reference based on the given identifier, or nested access. */ 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 index 9939ced2ac0..8c1974c53c3 100644 --- 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 @@ -18,14 +18,20 @@ import type {ImportManager} from './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. + * - The transform deletes any nodes that are marked for deletion by the manager. */ export function createTsTransformForImportManager( manager: ImportManager, extraStatementsForFiles?: Map, ): ts.TransformerFactory { return (ctx) => { - const {affectedFiles, newImports, updatedImports, reusedOriginalAliasDeclarations} = - manager.finalize(); + const { + affectedFiles, + newImports, + updatedImports, + reusedOriginalAliasDeclarations, + deletedImports, + } = 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. @@ -45,12 +51,16 @@ export function createTsTransformForImportManager( } } - const visitStatement: ts.Visitor = (node) => { - if ( - !ts.isImportDeclaration(node) || - node.importClause === undefined || - !ts.isImportClause(node.importClause) - ) { + const visitStatement: ts.Visitor = (node) => { + if (!ts.isImportDeclaration(node)) { + return node; + } + + if (deletedImports.has(node)) { + return undefined; + } + + if (node.importClause === undefined || !ts.isImportClause(node.importClause)) { return node; } 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 ea7847fc712..7e9e0dfd331 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 @@ -891,6 +891,196 @@ describe('import manager', () => { `), ); }); + + it('should remove a pre-existing import from a declaration', () => { + const {testFile, emit} = createTestProgram(` + import { input, output, model } from '@angular/core'; + input(); + output(); + model(); + `); + const manager = new ImportManager(); + + manager.removeImport(testFile, 'output', '@angular/core'); + const res = emit(manager, []); + + expect(res).toBe( + omitLeadingWhitespace(` + import { input, model } from '@angular/core'; + input(); + output(); + model(); + `), + ); + }); + + it('should remove the entire declaration if all pre-existing imports are removed', () => { + const {testFile, emit} = createTestProgram(` + import { input, output } from '@angular/core'; + input(); + output(); + `); + const manager = new ImportManager(); + + manager.removeImport(testFile, 'input', '@angular/core'); + manager.removeImport(testFile, 'output', '@angular/core'); + + expect(emit(manager, [])).toBe( + omitLeadingWhitespace(` + input(); + output(); + export {}; + `), + ); + }); + + it('should remove a pre-existing aliased import', () => { + const {testFile, emit} = createTestProgram(` + import { input, output as foo } from '@angular/core'; + input(); + foo(); + `); + const manager = new ImportManager(); + + manager.removeImport(testFile, 'output', '@angular/core'); + + expect(emit(manager, [])).toBe( + omitLeadingWhitespace(` + import { input } from '@angular/core'; + input(); + foo(); + `), + ); + }); + + it('should remove all pre-existing instances of a specific import', () => { + const {testFile, emit} = createTestProgram(` + import { input, input as foo } from '@angular/core'; + import { input as bar } from '@angular/core'; + input(); + foo(); + bar(); + `); + const manager = new ImportManager(); + + manager.removeImport(testFile, 'input', '@angular/core'); + + expect(emit(manager, [])).toBe( + omitLeadingWhitespace(` + input(); + foo(); + bar(); + export {}; + `), + ); + }); + + it('should be able to remove from an import that is being modified', () => { + const {testFile, emit} = createTestProgram(` + import { input } from '@angular/core'; + input(); + `); + const manager = new ImportManager(); + + const ref = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'foo', + requestedFile: testFile, + }); + + manager.removeImport(testFile, 'input', '@angular/core'); + const res = emit(manager, [ts.factory.createExpressionStatement(ref)]); + + expect(res).toBe( + omitLeadingWhitespace(` + import { foo } from '@angular/core'; + foo; + input(); + `), + ); + }); + + it('should be able to remove a symbol from a newly-created import declaration', () => { + const {testFile, emit} = createTestProgram(''); + const manager = new ImportManager(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + const outputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'output', + requestedFile: testFile, + }); + + manager.removeImport(testFile, 'input', '@angular/core'); + + const res = emit(manager, [ + ts.factory.createExpressionStatement(inputRef), + ts.factory.createExpressionStatement(outputRef), + ]); + + expect(res).toBe( + omitLeadingWhitespace(` + import { output } from "@angular/core"; + input; + output; + `), + ); + }); + + it('should add a symbol if addImport is called after removeImport', () => { + const {testFile, emit} = createTestProgram(''); + const manager = new ImportManager(); + + manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + requestedFile: testFile, + }); + + manager.removeImport(testFile, 'input', '@angular/core'); + + 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 remove a newly-added aliased import', () => { + const {testFile, emit} = createTestProgram(''); + const manager = new ImportManager(); + + const inputRef = manager.addImport({ + exportModuleSpecifier: '@angular/core', + exportSymbolName: 'input', + unsafeAliasOverride: 'foo', + requestedFile: testFile, + }); + + manager.removeImport(testFile, 'input', '@angular/core'); + + const res = emit(manager, [ts.factory.createExpressionStatement(inputRef)]); + + expect(res).toBe( + omitLeadingWhitespace(` + foo; + `), + ); + }); }); function createTestProgram(text: string): {