From fe3cf99db750fcada7ba838dcc1e819b2fc508b8 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 24 Feb 2023 09:12:48 +0100 Subject: [PATCH] fix(migrations): delete barrel exports in standalone migration Adds some logic to automatically delete `export * from './foo'` style imports. Previously they weren't being picked up, because finding all the references using the language service doesn't include barrel exports. --- .../standalone-migration/prune-modules.ts | 52 ++++++-- .../ng-generate/standalone-migration/util.ts | 4 + .../test/standalone_migration_spec.ts | 116 ++++++++++++++++++ 3 files changed, 160 insertions(+), 12 deletions(-) diff --git a/packages/core/schematics/ng-generate/standalone-migration/prune-modules.ts b/packages/core/schematics/ng-generate/standalone-migration/prune-modules.ts index aa5196422443..39447886404d 100644 --- a/packages/core/schematics/ng-generate/standalone-migration/prune-modules.ts +++ b/packages/core/schematics/ng-generate/standalone-migration/prune-modules.ts @@ -19,7 +19,6 @@ interface RemovalLocations { arrays: UniqueItemTracker; imports: UniqueItemTracker; exports: UniqueItemTracker; - classes: Set; unknown: Set; } @@ -29,21 +28,33 @@ export function pruneNgModules( referenceLookupExcludedFiles?: RegExp) { const filesToRemove = new Set(); const tracker = new ChangeTracker(printer, importRemapper); - const typeChecker = program.getTsProgram().getTypeChecker(); + const tsProgram = program.getTsProgram(); + const typeChecker = tsProgram.getTypeChecker(); const referenceResolver = new ReferenceResolver(program, host, rootFileNames, basePath, referenceLookupExcludedFiles); const removalLocations: RemovalLocations = { arrays: new UniqueItemTracker(), imports: new UniqueItemTracker(), exports: new UniqueItemTracker(), - classes: new Set(), unknown: new Set() }; + const classesToRemove = new Set(); + const barrelExports = new UniqueItemTracker(); + const nodesToRemove = new Set(); sourceFiles.forEach(function walk(node: ts.Node) { if (ts.isClassDeclaration(node) && canRemoveClass(node, typeChecker)) { collectRemovalLocations(node, removalLocations, referenceResolver, program); - removalLocations.classes.add(node); + classesToRemove.add(node); + } else if ( + ts.isExportDeclaration(node) && !node.exportClause && node.moduleSpecifier && + ts.isStringLiteralLike(node.moduleSpecifier) && node.moduleSpecifier.text.startsWith('.')) { + const exportedSourceFile = + typeChecker.getSymbolAtLocation(node.moduleSpecifier)?.valueDeclaration?.getSourceFile(); + + if (exportedSourceFile) { + barrelExports.track(exportedSourceFile, node); + } } node.forEachChild(walk); }); @@ -55,10 +66,27 @@ export function pruneNgModules( removeExportReferences(removalLocations.exports, tracker); addRemovalTodos(removalLocations.unknown, tracker); - for (const node of removalLocations.classes) { + // Collect all the nodes to be removed before determining which files to delete since we need + // to know it ahead of time when deleting barrel files that export other barrel files. + (function trackNodesToRemove(nodes: Set) { + for (const node of nodes) { + const sourceFile = node.getSourceFile(); + + if (!filesToRemove.has(sourceFile) && canRemoveFile(sourceFile, nodes)) { + const barrelExportsForFile = barrelExports.get(sourceFile); + nodesToRemove.add(node); + filesToRemove.add(sourceFile); + barrelExportsForFile && trackNodesToRemove(barrelExportsForFile); + } else { + nodesToRemove.add(node); + } + } + })(classesToRemove); + + for (const node of nodesToRemove) { const sourceFile = node.getSourceFile(); - if (!filesToRemove.has(sourceFile) && canRemoveFile(sourceFile, removalLocations.classes)) { + if (!filesToRemove.has(sourceFile) && canRemoveFile(sourceFile, nodesToRemove)) { filesToRemove.add(sourceFile); } else { tracker.removeNode(node); @@ -276,17 +304,17 @@ function isNonEmptyNgModuleProperty(node: ts.Node): node is ts.PropertyAssignmen * Determines if a file is safe to delete. A file is safe to delete if all it contains are * import statements, class declarations that are about to be deleted and non-exported code. * @param sourceFile File that is being checked. - * @param classesToBeRemoved Classes that are being removed as a part of the migration. + * @param nodesToBeRemoved Nodes that are being removed as a part of the migration. */ -function canRemoveFile(sourceFile: ts.SourceFile, classesToBeRemoved: Set) { +function canRemoveFile(sourceFile: ts.SourceFile, nodesToBeRemoved: Set) { for (const node of sourceFile.statements) { - if (ts.isImportDeclaration(node) || - (ts.isClassDeclaration(node) && classesToBeRemoved.has(node))) { + if (ts.isImportDeclaration(node) || nodesToBeRemoved.has(node)) { continue; } - if (ts.canHaveModifiers(node) && - ts.getModifiers(node)?.some(m => m.kind === ts.SyntaxKind.ExportKeyword)) { + if (ts.isExportDeclaration(node) || + (ts.canHaveModifiers(node) && + ts.getModifiers(node)?.some(m => m.kind === ts.SyntaxKind.ExportKeyword))) { return false; } } diff --git a/packages/core/schematics/ng-generate/standalone-migration/util.ts b/packages/core/schematics/ng-generate/standalone-migration/util.ts index e28a2c15d03c..94be0d3ab626 100644 --- a/packages/core/schematics/ng-generate/standalone-migration/util.ts +++ b/packages/core/schematics/ng-generate/standalone-migration/util.ts @@ -175,6 +175,10 @@ export class UniqueItemTracker { } } + get(key: K): Set|undefined { + return this._nodes.get(key); + } + getEntries(): IterableIterator<[K, Set]> { return this._nodes.entries(); } diff --git a/packages/core/schematics/test/standalone_migration_spec.ts b/packages/core/schematics/test/standalone_migration_spec.ts index 4e434b62544e..f01447e9bd6f 100644 --- a/packages/core/schematics/test/standalone_migration_spec.ts +++ b/packages/core/schematics/test/standalone_migration_spec.ts @@ -2245,6 +2245,122 @@ describe('standalone migration', () => { `)); }); + it('should remove barrel export if the corresponding file is deleted', async () => { + writeFile('app.module.ts', ` + import {NgModule} from '@angular/core'; + import {MyComp} from './comp'; + + @NgModule({imports: [MyComp]}) + export class AppModule {} + `); + + writeFile('button.module.ts', ` + import {NgModule} from '@angular/core'; + import {MyButton} from './button'; + + @NgModule({imports: [MyButton], exports: [MyButton]}) + export class ButtonModule {} + `); + + writeFile('comp.ts', ` + import {Component} from '@angular/core'; + import {MyButton} from './button'; + + @Component({ + selector: 'my-comp', + template: 'Hello', + standalone: true, + imports: [MyButton] + }) + export class MyComp {} + `); + + writeFile('button.ts', ` + import {Component} from '@angular/core'; + + @Component({selector: 'my-button', template: '', standalone: true}) + export class MyButton {} + `); + + writeFile('index.ts', ` + export * from './app.module'; + export {MyComp} from './comp'; + export {ButtonModule} from './button.module'; + `); + + await runMigration('prune-ng-modules'); + + expect(tree.exists('app.module.ts')).toBe(false); + expect(tree.exists('button.module.ts')).toBe(false); + expect(stripWhitespace(tree.readContent('index.ts'))).toBe(stripWhitespace(` + export {MyComp} from './comp'; + `)); + }); + + it('should remove barrel files referring to other barrel files that were deleted', async () => { + writeFile('app.module.ts', ` + import {NgModule} from '@angular/core'; + import {MyDir} from './dir'; + + @NgModule({imports: [MyDir]}) + export class AppModule {} + `); + + writeFile('dir.ts', ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[dir]', standalone: true}) + export class MyDir {} + `); + + writeFile('index.ts', `export * from './app.module';`); + writeFile('index-2.ts', `export * from './index';`); + writeFile('index-3.ts', `export * from './index-2';`); + + await runMigration('prune-ng-modules'); + + expect(tree.exists('index.ts')).toBe(false); + expect(tree.exists('index-2.ts')).toBe(false); + expect(tree.exists('index-3.ts')).toBe(false); + }); + + it('should not delete dependent barrel files if they have some barrel exports that will not be removed', + async () => { + writeFile('app.module.ts', ` + import {NgModule} from '@angular/core'; + import {MyDir} from './dir'; + + @NgModule({imports: [MyDir]}) + export class AppModule {} + `); + + writeFile('dir.ts', ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[dir]', standalone: true}) + export class MyDir {} + `); + + writeFile('utils.ts', ` + export function sum(a: number, b: number) { return a + b; } + `); + + writeFile('index.ts', `export * from './app.module';`); + writeFile('index-2.ts', ` + export * from './index'; + export * from './utils'; + `); + writeFile('index-3.ts', `export * from './index-2';`); + + await runMigration('prune-ng-modules'); + + expect(tree.exists('index.ts')).toBe(false); + expect(stripWhitespace(tree.readContent('index-2.ts'))) + .toBe(stripWhitespace(`export * from './utils';`)); + expect(stripWhitespace(tree.readContent('index-3.ts'))) + .toBe(stripWhitespace(`export * from './index-2';`)); + }); + it('should add a comment to locations that cannot be removed automatically', async () => { writeFile('app.module.ts', ` import {NgModule} from '@angular/core';