fix(migrations): fix cf migration import removal when errors occur (#53502)

thePunderWoman · alxhub · commit 79f791543b60 · 2023-12-12T09:02:32.000-08:00
When migrating a component and the associated external template, if errors occur, the component should not remove the common module imports. This fix should allow the application to still build in that instance. PR Close #53502
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts
@@ -40,6 +40,7 @@ export function migrateTemplate(
       migrated = formatTemplate(migrated, templateType);
     }
     file.removeCommonModule = canRemoveCommonModule(template);
+    file.canRemoveImports = true;
 
     // when migrating an external template, we have to pass back
     // whether it's safe to remove the CommonModule to the
@@ -48,6 +49,7 @@ export function migrateTemplate(
         analyzedFiles.has(file.sourceFilePath)) {
       const componentFile = analyzedFiles.get(file.sourceFilePath)!;
       componentFile.removeCommonModule = file.removeCommonModule;
+      componentFile.canRemoveImports = file.canRemoveImports;
     }
 
     errors = [
@@ -56,7 +58,7 @@ export function migrateTemplate(
       ...switchResult.errors,
       ...caseResult.errors,
     ];
-  } else {
+  } else if (file.canRemoveImports) {
     migrated = removeImports(template, node, file.removeCommonModule);
   }
 
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/types.ts b/packages/core/schematics/ng-generate/control-flow-migration/types.ts
@@ -240,6 +240,7 @@ export class Template {
 export class AnalyzedFile {
   private ranges: Range[] = [];
   removeCommonModule = false;
+  canRemoveImports = false;
   sourceFilePath: string = '';
 
   /** Returns the ranges in the order in which they should be migrated. */
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -4638,6 +4638,55 @@ describe('control flow migration', () => {
       expect(actual).toBe(expected);
     });
 
+    it('should not remove common module imports post migration if errors prevented migrating the external template file',
+       async () => {
+         writeFile('/comp.ts', [
+           `import {Component} from '@angular/core';`,
+           `import {NgIf} from '@angular/common';`,
+           `@Component({`,
+           `  imports: [NgIf],`,
+           `  templateUrl: './comp.html',`,
+           `})`,
+           `class Comp {`,
+           `  toggle = false;`,
+           `}`,
+         ].join('\n'));
+
+         writeFile('/comp.html', [
+           `<div>`,
+           `  <span *ngIf="toggle; else elseTmpl">shrug</span>`,
+           `</div>`,
+           `<ng-template #elseTmpl>else content</ng-template>`,
+           `<ng-template #elseTmpl>different</ng-template>`,
+         ].join('\n'));
+
+         await runMigration();
+         const actualCmp = tree.readContent('/comp.ts');
+         const expectedCmp = [
+           `import {Component} from '@angular/core';`,
+           `import {NgIf} from '@angular/common';`,
+           `@Component({`,
+           `  imports: [NgIf],`,
+           `  templateUrl: './comp.html',`,
+           `})`,
+           `class Comp {`,
+           `  toggle = false;`,
+           `}`,
+         ].join('\n');
+         const actualTemplate = tree.readContent('/comp.html');
+
+         const expectedTemplate = [
+           `<div>`,
+           `  <span *ngIf="toggle; else elseTmpl">shrug</span>`,
+           `</div>`,
+           `<ng-template #elseTmpl>else content</ng-template>`,
+           `<ng-template #elseTmpl>different</ng-template>`,
+         ].join('\n');
+
+         expect(actualCmp).toBe(expectedCmp);
+         expect(actualTemplate).toBe(expectedTemplate);
+       });
+
     it('should not remove common module imports post migration if other items used', async () => {
       writeFile('/comp.ts', [
         `import {CommonModule} from '@angular/common';`,
