fix(migrations): automatically prune root module after bootstrap step (#49030)

crisbeto · pkozlowski-opensource · commit 816e76a5789b · 2023-02-13T09:52:07.000+01:00
Currently as a part of the bootstrapping API migration we comment out the metadata of the root module and instruct users to re-run the module pruning step which can be cumbersome. These changes run the module pruning automatically. Note that initially I tried to reuse the module pruning logic and to run it against the existing program, but it was problematic, because it was common to have conflicting changes for the same AST nodes. PR Close #49030
diff --git a/packages/core/schematics/ng-generate/standalone-migration/README.md b/packages/core/schematics/ng-generate/standalone-migration/README.md
@@ -194,8 +194,7 @@ modules were skipped explicitly in the first step of the migration.
 5. Adjust any dynamic import paths so that they're correct when they're copied over.
 6. If an API with a standalone equivalent is detected, it may be converted automatically as well.
 E.g. `RouterModule.forRoot` will become `provideRouter`.
-7. Comment out the module metadata of the root class and leave a TODO to remove it. This can also
-be done automatically by running the "Remove unnecessary NgModules" step again.
+7. Remove the root module.
 
 If the migration detects that the `providers` or `imports` of the root module are referencing code
 outside of the class declaration, it will attempt to carry over as much of it as it can to the new
@@ -276,26 +275,6 @@ interface NonImportedInterface {
 const token = new InjectionToken<NonImportedInterface>('token');
 
 export class ExportedConfigClass {}
-
-@NgModule(/*
-TODO(standalone-migration): clean up removed NgModule class manually or run the "Remove unnecessary NgModule classes" step of the migration again.
-{
-  imports: [
-    SharedModule,
-    BrowserAnimationsModule,
-    RouterModule.forRoot([{
-      path: 'shop',
-      loadComponent: () => import('./shop/shop.component').then(m => m.ShopComponent)
-    }])
-  ],
-  declarations: [AppComponent],
-  bootstrap: [AppComponent],
-  providers: [
-    {provide: token, useValue: {foo: true, bar: {baz: false}}},
-    {provide: CONFIG, useClass: ExportedConfigClass}
-  ]
-}*/)
-export class AppModule {}
 ```
 
 ```typescript
diff --git a/packages/core/schematics/ng-generate/standalone-migration/index.ts b/packages/core/schematics/ng-generate/standalone-migration/index.ts
@@ -32,7 +32,7 @@ interface Options {
 }
 
 export default function(options: Options): Rule {
-  return async (tree) => {
+  return async (tree, context) => {
     const {buildPaths, testPaths} = await getProjectTsConfigPaths(tree);
     const basePath = process.cwd();
     const allPaths = [...buildPaths, ...testPaths];
@@ -54,12 +54,17 @@ export default function(options: Options): Rule {
       throw new SchematicsException(`Could not find any files to migrate under the path ${
           pathToMigrate}. Cannot run the standalone migration.`);
     }
+
+    context.logger.info('🎉 Automated migration step has finished! 🎉');
+    context.logger.info(
+        'IMPORTANT! Please verify manually that your application builds and behaves as expected.');
+    // TODO(crisbeto): log a link to the guide once it's published
   };
 }
 
 function standaloneMigration(
     tree: Tree, tsconfigPath: string, basePath: string, pathToMigrate: string,
-    schematicOptions: Options): number {
+    schematicOptions: Options, oldProgram?: NgtscProgram): number {
   if (schematicOptions.path.startsWith('..')) {
     throw new SchematicsException(
         'Cannot run standalone migration outside of the current project.');
@@ -75,7 +80,7 @@ function standaloneMigration(
         skipDefaultLibCheck: true,
       });
   const referenceLookupExcludedFiles = /node_modules|\.ngtypecheck\.ts/;
-  const program = createProgram({rootNames, host, options}) as NgtscProgram;
+  const program = createProgram({rootNames, host, options, oldProgram}) as NgtscProgram;
   const printer = ts.createPrinter();
 
   if (existsSync(pathToMigrate) && !statSync(pathToMigrate).isDirectory()) {
@@ -134,5 +139,15 @@ function standaloneMigration(
     }
   }
 
+  // Run the module pruning after the standalone bootstrap to automatically remove the root module.
+  // Note that we can't run the module pruning internally without propagating the changes to disk,
+  // because there may be conflicting AST node changes.
+  if (schematicOptions.mode === MigrationMode.standaloneBootstrap) {
+    return standaloneMigration(
+               tree, tsconfigPath, basePath, pathToMigrate,
+               {...schematicOptions, mode: MigrationMode.pruneModules}, program) +
+        sourceFiles.length;
+  }
+
   return sourceFiles.length;
 }
diff --git a/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts b/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts
@@ -151,14 +151,11 @@ function migrateBootstrapCall(
   const moduleImportsInNewCall: ts.Expression[] = [];
   let nodeLookup: NodeLookup|null = null;
 
-  // We can't reuse the module pruning logic, because we would have to recreate the entire program.
-  // Instead we comment out the module's metadata so that the user doesn't get compilation errors
-  // for the classes that are left in the `declarations` array. This should allow the app to
-  // run and the user can run module pruning themselves to get rid of the module afterwards.
+  // Comment out the metadata so that it'll be removed when we run the module pruning afterwards.
+  // If the pruning is left for some reason, the user will still have an actionable TODO.
   tracker.insertText(
       moduleSourceFile, analysis.metadata.getStart(),
-      '/* TODO(standalone-migration): clean up removed NgModule class manually or run the ' +
-          '"Remove unnecessary NgModule classes" step of the migration again. \n');
+      '/* TODO(standalone-migration): clean up removed NgModule class manually. \n');
   tracker.insertText(moduleSourceFile, analysis.metadata.getEnd(), ' */');
 
   if (providers && ts.isPropertyAssignment(providers)) {
diff --git a/packages/core/schematics/test/standalone_migration_spec.ts b/packages/core/schematics/test/standalone_migration_spec.ts
