fix(migrations): CF migration - ensure NgIfElse attributes are properly removed (#53298)

thePunderWoman · dylhunn · commit 45064f1ae1ce · 2023-12-01T10:32:36.000-08:00
the attribute in question was assumed to be at the start of the replaced content, but it could be later, too. fixes: #53288 PR Close #53298
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts
@@ -472,17 +472,18 @@ export function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number
 
   // the beginning of the updated string in the main block, for example: <div some="attributes">
   let start = tmpl.slice(etm.start(offset), attrStart) + tmpl.slice(valEnd, childStart);
+  // the middle is the actual contents of the element
+  const middle = tmpl.slice(childStart, childEnd);
+  // the end is the closing part of the element, example: </div>
+  let end = tmpl.slice(childEnd, etm.end(offset));
 
   if (etm.shouldRemoveElseAttr()) {
     // this removes a bound ngIfElse attribute that's no longer needed
+    // this could be on the start or end
     start = start.replace(etm.getElseAttrStr(), '');
+    end = end.replace(etm.getElseAttrStr(), '');
   }
 
-  // the middle is the actual contents of the element
-  const middle = tmpl.slice(childStart, childEnd);
-  // the end is the closing part of the element, example: </div>
-  const end = tmpl.slice(childEnd, etm.end(offset));
-
   return {start, middle, end};
 }
 
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -645,6 +645,45 @@ describe('control flow migration', () => {
       ].join('\n'));
     });
 
+    it('should migrate a bound NgIfElse case with ng-templates and remove all unnecessary attributes',
+       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]="fooTemplate"`,
+           `  [ngIfElse]="barTemplate"`,
+           `  [ngTemplateOutlet]="fooTemplate"`,
+           `></ng-template>`,
+           `<ng-template #fooTemplate>Foo</ng-template>`,
+           `<ng-template #barTemplate>Bar</ng-template>`,
+         ].join('\n'));
+
+         await runMigration();
+         const content = tree.readContent('/comp.html');
+
+         expect(content).toBe([
+           `@if (fooTemplate) {`,
+           `  <ng-template`,
+           `    [ngTemplateOutlet]="fooTemplate"`,
+           `  ></ng-template>`,
+           `} @else {`,
+           `  Bar`,
+           `}`,
+           `<ng-template #fooTemplate>Foo</ng-template>\n`,
+         ].join('\n'));
+       });
+
     it('should migrate a bound NgIfThenElse case with ng-templates with i18n', async () => {
       writeFile('/comp.ts', `
         import {Component} from '@angular/core';
