From 749c85ae9e0d3c31109c274c1ce57899cbb579df Mon Sep 17 00:00:00 2001
From: Jessica Janiuk <jessicajaniuk@google.com>
Date: Thu, 30 Nov 2023 16:46:33 -0500
Subject: [PATCH] fix(migrations): CF migration - ensure NgIfElse attributes
 are properly removed

the attribute in question was assumed to be at the start of the replaced content, but it could be later, too.

fixes: #53288
---
 .../control-flow-migration/util.ts            | 11 +++---
 .../test/control_flow_migration_spec.ts       | 39 +++++++++++++++++++
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts
index 1e9f2b71308d..2e83f8d3f19a 100644
--- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts
+++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts
@@ -471,17 +471,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
index d61624eb34c8..da3f8a58a6a3 100644
--- a/packages/core/schematics/test/control_flow_migration_spec.ts
+++ b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -611,6 +611,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';