perf(migrations): avoid re-traversing nodes when resolving bootstrap call dependencies (#49010)

crisbeto · pkozlowski-opensource · commit 4ac25b2affab · 2023-02-09T15:37:18.000+01:00
Fixes that the migration was unnecessarily traversing top-level nodes. This was a large performance bottle-neck, because it involves a lot of language service lookups. PR Close #49010
diff --git a/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts b/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts
@@ -511,6 +511,7 @@ function addNodesToCopy(
 function findAllSameFileReferences(
     rootNode: ts.Node, nodeLookup: NodeLookup, referenceResolver: ReferenceResolver): Set<ts.Node> {
   const results = new Set<ts.Node>();
+  const traversedTopLevelNodes = new Set<ts.Node>();
   const excludeStart = rootNode.getStart();
   const excludeEnd = rootNode.getEnd();
 
@@ -531,14 +532,21 @@ function findAllSameFileReferences(
       if (results.has(ref)) {
         continue;
       }
-      const closestTopLevel = closestNode(ref, isTopLevelStatement);
+
       results.add(ref);
 
+      const closestTopLevel = closestNode(ref, isTopLevelStatement);
+      // Avoid re-traversing the same top-level nodes since we know what the result will be.
+      if (!closestTopLevel || traversedTopLevelNodes.has(closestTopLevel)) {
+        continue;
+      }
+
       // Keep searching, starting from the closest top-level node. We skip import declarations,
       // because we already know about them and they may put the search into an infinite loop.
-      if (closestTopLevel && !ts.isImportDeclaration(closestTopLevel) &&
+      if (!ts.isImportDeclaration(closestTopLevel) &&
           isOutsideRange(
               excludeStart, excludeEnd, closestTopLevel.getStart(), closestTopLevel.getEnd())) {
+        traversedTopLevelNodes.add(closestTopLevel);
         walk(closestTopLevel);
       }
     }
diff --git a/packages/core/schematics/test/standalone_migration_spec.ts b/packages/core/schematics/test/standalone_migration_spec.ts
@@ -3259,4 +3259,48 @@ describe('standalone migration', () => {
         export class AppComponent {}
       `));
      });
+
+  it('should be able to migrate a bootstrapModule call where the root component does not belong to the bootstrapped component',
+     async () => {
+       writeFile('main.ts', `
+          import {AppModule} from './app/app.module';
+          import {platformBrowser} from '@angular/platform-browser';
+
+          platformBrowser().bootstrapModule(AppModule).catch(e => console.error(e));
+        `);
+
+       writeFile('./app/root.module.ts', `
+          import {NgModule, Component} from '@angular/core';
+
+          @Component({selector: 'root-comp', template: '', standalone: true})
+          export class Root {}
+
+          @NgModule({imports: [Root], exports: [Root]})
+          export class RootModule {}
+        `);
+
+       writeFile('./app/app.module.ts', `
+          import {NgModule, Component} from '@angular/core';
+          import {RootModule, Root} from './root.module';
+
+          @NgModule({
+            imports: [RootModule],
+            bootstrap: [Root]
+          })
+          export class AppModule {}
+        `);
+
+       await runMigration('standalone-bootstrap');
+
+       expect(stripWhitespace(tree.readContent('main.ts'))).toBe(stripWhitespace(`
+          import {AppModule} from './app/app.module';
+          import {platformBrowser, bootstrapApplication} from '@angular/platform-browser';
+          import {importProvidersFrom} from '@angular/core';
+          import {RootModule, Root} from './app/root.module';
+
+          bootstrapApplication(Root, {
+            providers: [importProvidersFrom(RootModule)]
+          }).catch(e => console.error(e));
+        `));
+     });
 });
