From 4ac25b2affab4f959ad8c111f1e429a05b435422 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 9 Feb 2023 14:31:18 +0100 Subject: [PATCH] perf(migrations): avoid re-traversing nodes when resolving bootstrap call dependencies (#49010) 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 --- .../standalone-bootstrap.ts | 12 ++++- .../test/standalone_migration_spec.ts | 44 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts b/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts index 4e91220a755..e32ef777e8b 100644 --- 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 { const results = new Set(); + const traversedTopLevelNodes = new Set(); 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 index f6bf92366b4..bc949f4a59b 100644 --- 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)); + `)); + }); });