fix(migrations): delete barrel exports in standalone migration

crisbeto · crisbeto · commit fe3cf99db750 · 2023-02-24T09:12:48.000+01:00
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.
diff --git 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<ts.ArrayLiteralExpression, ts.Node>;
   imports: UniqueItemTracker<ts.NamedImports, ts.Node>;
   exports: UniqueItemTracker<ts.NamedExports, ts.Node>;
-  classes: Set<ts.ClassDeclaration>;
   unknown: Set<ts.Node>;
 }
 
@@ -29,21 +28,33 @@ export function pruneNgModules(
     referenceLookupExcludedFiles?: RegExp) {
   const filesToRemove = new Set<ts.SourceFile>();
   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<ts.ArrayLiteralExpression, ts.Node>(),
     imports: new UniqueItemTracker<ts.NamedImports, ts.Node>(),
     exports: new UniqueItemTracker<ts.NamedExports, ts.Node>(),
-    classes: new Set<ts.ClassDeclaration>(),
     unknown: new Set<ts.Node>()
   };
+  const classesToRemove = new Set<ts.ClassDeclaration>();
+  const barrelExports = new UniqueItemTracker<ts.SourceFile, ts.ExportDeclaration>();
+  const nodesToRemove = new Set<ts.Node>();
 
   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<ts.Node>) {
+    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<ts.ClassDeclaration>) {
+function canRemoveFile(sourceFile: ts.SourceFile, nodesToBeRemoved: Set<ts.Node>) {
   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
@@ -175,6 +175,10 @@ export class UniqueItemTracker<K, V> {
     }
   }
 
+  get(key: K): Set<V>|undefined {
+    return this._nodes.get(key);
+  }
+
   getEntries(): IterableIterator<[K, Set<V>]> {
     return this._nodes.entries();
   }
diff --git 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: '<my-button>Hello</my-button>',
+        standalone: true,
+        imports: [MyButton]
+      })
+      export class MyComp {}
+    `);
+
+    writeFile('button.ts', `
+      import {Component} from '@angular/core';
+
+      @Component({selector: 'my-button', template: '<ng-content></ng-content>', 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';

Original file line number	Diff line number	Diff line change
`@@ -175,6 +175,10 @@ export class UniqueItemTracker<K, V> {`
`175`	`175`	`}`
`176`	`176`	`}`
`177`	`177`
	`178`	`+ get(key: K): Set<V>\|undefined {`
	`179`	`+ return this._nodes.get(key);`
	`180`	`+ }`
	`181`	`+`
`178`	`182`	`getEntries(): IterableIterator<[K, Set<V>]> {`
`179`	`183`	`return this._nodes.entries();`
`180`	`184`	`}`