fix(migrations): CF Migration - Fix case of aliases on i18n ng-templates preventing removal (#53299)

thePunderWoman · dylhunn · commit a6275cfa54d6 · 2023-12-01T09:18:32.000-08:00
i18n template removal expected no other attributes to be present, but if a bound ngIf is present with aliases and i18n, that is more than what was expected. Now it should safely remove them appropriately. fixes: #53289 PR Close #53299
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts
@@ -411,8 +411,9 @@ export function getOriginals(etm: ElementToMigrate, tmpl: string, offset: number
 }
 
 function isI18nTemplate(etm: ElementToMigrate, i18nAttr: Attribute|undefined): boolean {
-  return etm.el.name === 'ng-template' && i18nAttr !== undefined &&
-      (etm.el.attrs.length === 2 || (etm.el.attrs.length === 3 && etm.elseAttr !== undefined));
+  let attrCount = countAttributes(etm);
+  const safeToRemove = etm.el.attrs.length === attrCount + (i18nAttr !== undefined ? 1 : 0);
+  return etm.el.name === 'ng-template' && i18nAttr !== undefined && safeToRemove;
 }
 
 function isRemovableContainer(etm: ElementToMigrate): boolean {
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -580,6 +580,40 @@ describe('control flow migration', () => {
       ].join('\n'));
     });
 
+    it('should migrate a bound if case on an ng-template with i18n', async () => {
+      writeFile('/comp.ts', `
+        import {Component} from '@angular/core';
+        import {NgIf} from '@angular/common';
+
+        @Component({
+          templateUrl: './comp.html'
+        })
+        class Comp {
+          show = false;
+        }
+      `);
+
+      writeFile('/comp.html', [
+        `<ng-template`,
+        `  [ngIf]="data$ | async"`,
+        `  let-data="ngIf"`,
+        `  i18n="@@i18n-label">`,
+        `  {{ data }}`,
+        `</ng-template>`,
+      ].join('\n'));
+
+      await runMigration();
+      const content = tree.readContent('/comp.html');
+
+      expect(content).toBe([
+        `@if (data$ | async; as data) {`,
+        `  <ng-container i18n="@@i18n-label">`,
+        `    {{ data }}`,
+        `  </ng-container>`,
+        `}`,
+      ].join('\n'));
+    });
+
     it('should migrate an if case with an ng-container with empty i18n', async () => {
       writeFile('/comp.ts', `
         import {Component} from '@angular/core';

Original file line number	Diff line number	Diff line change
`@@ -411,8 +411,9 @@ export function getOriginals(etm: ElementToMigrate, tmpl: string, offset: number`
`411`	`411`	`}`
`412`	`412`
`413`	`413`	`function isI18nTemplate(etm: ElementToMigrate, i18nAttr: Attribute\|undefined): boolean {`
`414`		`- return etm.el.name === 'ng-template' && i18nAttr !== undefined &&`
`415`		`- (etm.el.attrs.length === 2 \|\| (etm.el.attrs.length === 3 && etm.elseAttr !== undefined));`
	`414`	`+ let attrCount = countAttributes(etm);`
	`415`	`+ const safeToRemove = etm.el.attrs.length === attrCount + (i18nAttr !== undefined ? 1 : 0);`
	`416`	`+ return etm.el.name === 'ng-template' && i18nAttr !== undefined && safeToRemove;`
`416`	`417`	`}`
`417`	`418`
`418`	`419`	`function isRemovableContainer(etm: ElementToMigrate): boolean {`