fix(migrations): move standalone migrations into imports

crisbeto · crisbeto · commit 5e00e3480f05 · 2023-02-08T10:32:45.000+01:00
Normally having a standalone declaration in the `imports` array is an error and something we handle in the conversion to standalone, but tests can end up in this situation, because apps may have separate tsconfigs for the main app and for tests.

These changes make it so that we move any incorrectly-defined standalone declarations, even if they aren't part of the current migration.
diff --git a/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts b/packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts
@@ -41,6 +41,7 @@ export function toStandaloneBootstrap(
   const languageService = createLanguageService(program, host, rootFileNames, basePath);
   const bootstrapCalls: BootstrapCallAnalysis[] = [];
   const testObjects: ts.ObjectLiteralExpression[] = [];
+  const allDeclarations: Reference<ts.ClassDeclaration>[] = [];
 
   for (const sourceFile of sourceFiles) {
     sourceFile.forEachChild(function walk(node) {
@@ -59,24 +60,19 @@ export function toStandaloneBootstrap(
     testObjects.push(...findTestObjectsToMigrate(sourceFile, typeChecker));
   }
 
-  if (bootstrapCalls.length > 0) {
-    const allDeclarations: Reference<ts.ClassDeclaration>[] = [];
-
-    for (const call of bootstrapCalls) {
-      allDeclarations.push(...call.declarations);
-      migrateBootstrapCall(call, tracker, languageService, typeChecker, printer);
-    }
+  for (const call of bootstrapCalls) {
+    allDeclarations.push(...call.declarations);
+    migrateBootstrapCall(call, tracker, languageService, typeChecker, printer);
+  }
 
-    if (allDeclarations.length > 0) {
-      // The previous migrations explicitly skip over bootstrapped
-      // declarations so we have to migrate them now.
-      allDeclarations.forEach(
-          decl => convertNgModuleDeclarationToStandalone(
-              decl, allDeclarations, tracker, templateTypeChecker));
-      migrateTestDeclarations(testObjects, allDeclarations, tracker, typeChecker);
-    }
+  // The previous migrations explicitly skip over bootstrapped
+  // declarations so we have to migrate them now.
+  for (const declaration of allDeclarations) {
+    convertNgModuleDeclarationToStandalone(
+        declaration, allDeclarations, tracker, templateTypeChecker);
   }
 
+  migrateTestDeclarations(testObjects, allDeclarations, tracker, templateTypeChecker, typeChecker);
   return tracker.recordChanges();
 }
 
diff --git a/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts b/packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts
@@ -59,7 +59,8 @@ export function toStandalone(
     migrateNgModuleClass(node, declarations, tracker, typeChecker, templateTypeChecker);
   }
 
-  migrateTestDeclarations(testObjectsToMigrate, declarations, tracker, typeChecker);
+  migrateTestDeclarations(
+      testObjectsToMigrate, declarations, tracker, templateTypeChecker, typeChecker);
   return tracker.recordChanges();
 }
 
@@ -167,7 +168,8 @@ function migrateNgModuleClass(
 
   if (metadata) {
     moveDeclarationsToImports(
-        metadata, allDeclarations.map(decl => decl.node), typeChecker, tracker);
+        metadata, allDeclarations.map(decl => decl.node), typeChecker, templateTypeChecker,
+        tracker);
   }
 }
 
@@ -181,7 +183,8 @@ function migrateNgModuleClass(
  */
 function moveDeclarationsToImports(
     literal: ts.ObjectLiteralExpression, allDeclarations: ts.ClassDeclaration[],
-    typeChecker: ts.TypeChecker, tracker: ChangeTracker): void {
+    typeChecker: ts.TypeChecker, templateTypeChecker: TemplateTypeChecker,
+    tracker: ChangeTracker): void {
   const declarationsProp = findLiteralProperty(literal, 'declarations');
 
   if (!declarationsProp) {
@@ -202,7 +205,12 @@ function moveDeclarationsToImports(
         if (ts.isIdentifier(el)) {
           const correspondingClass = findClassDeclaration(el, typeChecker);
 
-          if (!correspondingClass || allDeclarations.includes(correspondingClass)) {
+          if (!correspondingClass ||
+              // Check whether the declaration is either standalone already or is being converted
+              // in this migration. We need to check if it's standalone already, in order to correct
+              // some cases where the main app and the test files are being migrated in separate
+              // programs.
+              isStandaloneDeclaration(correspondingClass, allDeclarations, templateTypeChecker)) {
             declarationsToCopy.push(el);
           } else {
             declarationsToPreserve.push(el);
@@ -502,13 +510,15 @@ export function extractDeclarationsFromModule(
 /**
  * Migrates the `declarations` from a unit test file to standalone.
  * @param testObjects Object literals used to configure the testing modules.
+ * @param declarationsOutsideOfTestFiles Non-testing declarations that are part of this migration.
  * @param tracker
+ * @param templateTypeChecker
  * @param typeChecker
  */
 export function migrateTestDeclarations(
     testObjects: ts.ObjectLiteralExpression[],
     declarationsOutsideOfTestFiles: Reference<ts.ClassDeclaration>[], tracker: ChangeTracker,
-    typeChecker: ts.TypeChecker) {
+    templateTypeChecker: TemplateTypeChecker, typeChecker: ts.TypeChecker) {
   const {decorators, componentImports} = analyzeTestingModules(testObjects, typeChecker);
   const allDeclarations: ts.ClassDeclaration[] =
       declarationsOutsideOfTestFiles.map(ref => ref.node);
@@ -544,7 +554,7 @@ export function migrateTestDeclarations(
   }
 
   for (const obj of testObjects) {
-    moveDeclarationsToImports(obj, allDeclarations, typeChecker, tracker);
+    moveDeclarationsToImports(obj, allDeclarations, typeChecker, templateTypeChecker, tracker);
   }
 }
 
@@ -636,3 +646,21 @@ function extractMetadataLiteral(decorator: ts.Decorator): ts.ObjectLiteralExpres
       decorator.expression.arguments[0] :
       null;
 }
+
+/**
+ * Checks whether a class is a standalone declaration.
+ * @param node Class being checked.
+ * @param declarationsInMigration Classes that are being converted to standalone in this migration.
+ * @param templateTypeChecker
+ */
+function isStandaloneDeclaration(
+    node: ts.ClassDeclaration, declarationsInMigration: ts.ClassDeclaration[],
+    templateTypeChecker: TemplateTypeChecker): boolean {
+  if (declarationsInMigration.includes(node)) {
+    return true;
+  }
+
+  const metadata =
+      templateTypeChecker.getDirectiveMetadata(node) || templateTypeChecker.getPipeMetadata(node);
+  return metadata != null && metadata.isStandalone;
+}
diff --git a/packages/core/schematics/test/standalone_migration_spec.ts b/packages/core/schematics/test/standalone_migration_spec.ts
@@ -930,6 +930,41 @@ describe('standalone migration', () => {
     `));
   });
 
+  it('should migrate tests where the declaration is already standalone', async () => {
+    writeFile('comp.ts', `
+      import {Component} from '@angular/core';
+
+      @Component({selector: 'comp', template: '', standalone: true})
+      export class MyComp {}
+    `);
+
+    writeFile('app.spec.ts', `
+      import {TestBed} from '@angular/core/testing';
+      import {MyComp} from './comp';
+
+      describe('bootrstrapping an app', () => {
+        it('should work', () => {
+          TestBed.configureTestingModule({declarations: [MyComp]});
+          expect(() => TestBed.createComponent(MyComp)).not.toThrow();
+        });
+      });
+    `);
+
+    await runMigration('convert-to-standalone');
+
+    expect(stripWhitespace(tree.readContent('app.spec.ts'))).toContain(stripWhitespace(`
+      import {TestBed} from '@angular/core/testing';
+      import {MyComp} from './comp';
+
+      describe('bootrstrapping an app', () => {
+        it('should work', () => {
+          TestBed.configureTestingModule({imports: [MyComp]});
+          expect(() => TestBed.createComponent(MyComp)).not.toThrow();
+        });
+      });
+    `));
+  });
+
   it('should import the module that declares a template dependency', async () => {
     writeFile('./should-migrate/module.ts', `
       import {NgModule} from '@angular/core';
