Skip to content

Commit

Permalink
fix(migrations): fix cf migration import removal when errors occur (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
jessicajaniuk authored and alxhub committed Dec 12, 2023
1 parent 617dc53 commit d232ea1
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 1 deletion.
Expand Up @@ -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
Expand All @@ -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 = [
Expand All @@ -56,7 +58,7 @@ export function migrateTemplate(
...switchResult.errors,
...caseResult.errors,
];
} else {
} else if (file.canRemoveImports) {
migrated = removeImports(template, node, file.removeCommonModule);
}

Expand Down
Expand Up @@ -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. */
Expand Down
49 changes: 49 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
Expand Up @@ -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';`,
Expand Down

0 comments on commit d232ea1

Please sign in to comment.