From d232ea143f99fffc190c126d01920e965ffde3a3 Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Mon, 11 Dec 2023 13:05:30 -0500 Subject: [PATCH] fix(migrations): fix cf migration import removal when errors occur (#53502) 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 --- .../control-flow-migration/migration.ts | 4 +- .../control-flow-migration/types.ts | 1 + .../test/control_flow_migration_spec.ts | 49 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts index 31ea72c07f9..4131a8c43ab 100644 --- 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 index 32a27bc15c1..9f74225ca80 100644 --- 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 index fe723573b82..af93b372e1d 100644 --- 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', [ + `
`, + ` shrug`, + `
`, + `else content`, + `different`, + ].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 = [ + `
`, + ` shrug`, + `
`, + `else content`, + `different`, + ].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';`,