From d033540d0f874a7a05b79c00e3151ed076fa71c3 Mon Sep 17 00:00:00 2001
From: Jessica Janiuk <jessicajaniuk@google.com>
Date: Mon, 13 Nov 2023 15:26:29 -0500
Subject: [PATCH] fix(migrations): Add support for bound versions of NgIfElse
 and NgIfThenElse (#52869)

This ensures the bound version of NgIfElse and NgIfThenElse work properly with the migration.

fixes: #52842

PR Close #52869
---
 .../ng-generate/control-flow-migration/ifs.ts |  48 ++++--
 .../control-flow-migration/types.ts           |  14 +-
 .../control-flow-migration/util.ts            |   7 +-
 .../test/control_flow_migration_spec.ts       | 138 ++++++++++++++++++
 4 files changed, 192 insertions(+), 15 deletions(-)

diff --git a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
index 49ed4af7b51a6..5bcdc0407eea8 100644
--- a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
+++ b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
@@ -68,11 +68,14 @@ function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Resul
   const matchThen = etm.attr.value.match(/;\s*then/gm);
   const matchElse = etm.attr.value.match(/;\s*else/gm);
 
-  if (matchThen && matchThen.length > 0) {
-    return buildIfThenElseBlock(etm, tmpl, matchThen[0], matchElse![0], offset);
-  } else if (matchElse && matchElse.length > 0) {
+  if (etm.thenAttr !== undefined || etm.elseAttr !== undefined) {
+    // bound if then / if then else
+    return buildBoundIfElseBlock(etm, tmpl, offset);
+  } else if (matchThen && matchThen.length > 0) {
+    return buildStandardIfThenElseBlock(etm, tmpl, matchThen[0], matchElse![0], offset);
+  } else if ((matchElse && matchElse.length > 0)) {
     // just else
-    return buildIfElseBlock(etm, tmpl, matchElse[0], offset);
+    return buildStandardIfElseBlock(etm, tmpl, matchElse![0], offset);
   }
 
   return buildIfBlock(etm, tmpl, offset);
@@ -100,15 +103,32 @@ function buildIfBlock(etm: ElementToMigrate, tmpl: string, offset: number): Resu
   return {tmpl: updatedTmpl, offsets: {pre, post}};
 }
 
-function buildIfElseBlock(
+function buildStandardIfElseBlock(
     etm: ElementToMigrate, tmpl: string, elseString: string, offset: number): Result {
   // includes the mandatory semicolon before as
-  const lbString = etm.hasLineBreaks ? '\n' : '';
   const condition = etm.getCondition(elseString).replace(' as ', '; as ');
+  const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
+  return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
+}
+
+function buildBoundIfElseBlock(etm: ElementToMigrate, tmpl: string, offset: number): Result {
+  // includes the mandatory semicolon before as
+  const condition = etm.attr.value.replace(' as ', '; as ');
+  const elsePlaceholder = `#${etm.elseAttr!.value}|`;
+  if (etm.thenAttr !== undefined) {
+    const thenPlaceholder = `#${etm.thenAttr!.value}|`;
+    return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
+  }
+  return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
+}
+
+function buildIfElseBlock(
+    etm: ElementToMigrate, tmpl: string, condition: string, elsePlaceholder: string,
+    offset: number): Result {
+  const lbString = etm.hasLineBreaks ? '\n' : '';
 
   const originals = getOriginals(etm, tmpl, offset);
 
-  const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
   const {start, middle, end} = getMainBlock(etm, tmpl, offset);
   const startBlock = `@if (${condition}) {${lbString}${start}`;
 
@@ -127,10 +147,19 @@ function buildIfElseBlock(
   return {tmpl: updatedTmpl, offsets: {pre, post}};
 }
 
-function buildIfThenElseBlock(
+function buildStandardIfThenElseBlock(
     etm: ElementToMigrate, tmpl: string, thenString: string, elseString: string,
     offset: number): Result {
+  // includes the mandatory semicolon before as
   const condition = etm.getCondition(thenString).replace(' as ', '; as ');
+  const thenPlaceholder = `#${etm.getTemplateName(thenString, elseString)}|`;
+  const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
+  return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
+}
+
+function buildIfThenElseBlock(
+    etm: ElementToMigrate, tmpl: string, condition: string, thenPlaceholder: string,
+    elsePlaceholder: string, offset: number): Result {
   const lbString = etm.hasLineBreaks ? '\n' : '';
 
   const originals = getOriginals(etm, tmpl, offset);
@@ -138,9 +167,6 @@ function buildIfThenElseBlock(
   const startBlock = `@if (${condition}) {${lbString}`;
   const elseBlock = `${lbString}} @else {${lbString}`;
 
-  const thenPlaceholder = `#${etm.getTemplateName(thenString, elseString)}|`;
-  const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
-
   const postBlock = thenPlaceholder + elseBlock + elsePlaceholder + `${lbString}}`;
   const ifThenElseBlock = startBlock + postBlock;
 
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 37fa9694b48cf..818416030cd2d 100644
--- a/packages/core/schematics/ng-generate/control-flow-migration/types.ts
+++ b/packages/core/schematics/ng-generate/control-flow-migration/types.ts
@@ -10,6 +10,8 @@ import {Attribute, Element, RecursiveVisitor, Text} from '@angular/compiler';
 import ts from 'typescript';
 
 export const ngtemplate = 'ng-template';
+export const boundngifelse = '[ngIfElse]';
+export const boundngifthenelse = '[ngIfThenElse]';
 
 function allFormsOf(selector: string): string[] {
   return [
@@ -84,12 +86,18 @@ export type MigrateError = {
 export class ElementToMigrate {
   el: Element;
   attr: Attribute;
+  elseAttr: Attribute|undefined;
+  thenAttr: Attribute|undefined;
   nestCount = 0;
   hasLineBreaks = false;
 
-  constructor(el: Element, attr: Attribute) {
+  constructor(
+      el: Element, attr: Attribute, elseAttr: Attribute|undefined = undefined,
+      thenAttr: Attribute|undefined = undefined) {
     this.el = el;
     this.attr = attr;
+    this.elseAttr = elseAttr;
+    this.thenAttr = thenAttr;
   }
 
   getCondition(targetStr: string): string {
@@ -226,7 +234,9 @@ export class ElementCollector extends RecursiveVisitor {
     if (el.attrs.length > 0) {
       for (const attr of el.attrs) {
         if (this._attributes.includes(attr.name)) {
-          this.elements.push(new ElementToMigrate(el, attr));
+          const elseAttr = el.attrs.find(x => x.name === boundngifelse);
+          const thenAttr = el.attrs.find(x => x.name === boundngifthenelse);
+          this.elements.push(new ElementToMigrate(el, attr, elseAttr, thenAttr));
         }
       }
     }
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 fc87547c7aeb1..e9fbe29be9f0f 100644
--- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts
+++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts
@@ -360,13 +360,16 @@ export function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number
     {start: string, middle: string, end: string} {
   const i18nAttr = etm.el.attrs.find(x => x.name === 'i18n');
   if ((etm.el.name === 'ng-container' || etm.el.name === 'ng-template') &&
-      etm.el.attrs.length === 1) {
+      (etm.el.attrs.length === 1 || (etm.el.attrs.length === 2 && etm.elseAttr !== undefined) ||
+       (etm.el.attrs.length === 3 && etm.elseAttr !== undefined && etm.thenAttr !== undefined))) {
     // this is the case where we're migrating and there's no need to keep the ng-container
     const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
     const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
     const middle = tmpl.slice(childStart, childEnd);
     return {start: '', middle, end: ''};
-  } else if (etm.el.name === 'ng-template' && etm.el.attrs.length === 2 && i18nAttr !== undefined) {
+  } else if (
+      etm.el.name === 'ng-template' && i18nAttr !== undefined &&
+      (etm.el.attrs.length === 2 || (etm.el.attrs.length === 3 && etm.elseAttr !== undefined))) {
     const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
     const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
     const middle = wrapIntoI18nContainer(i18nAttr, tmpl.slice(childStart, childEnd));
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
index df9d853f50148..f7ad20a42e6b3 100644
--- a/packages/core/schematics/test/control_flow_migration_spec.ts
+++ b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -328,6 +328,75 @@ describe('control flow migration', () => {
       ].join('\n'));
     });
 
+    it('should migrate an if else case as bindings', async () => {
+      writeFile('/comp.ts', `
+        import {Component} from '@angular/core';
+        import {NgIf,NgIfElse} from '@angular/common';
+
+        @Component({
+          templateUrl: './comp.html'
+        })
+        class Comp {
+          show = false;
+        }
+      `);
+
+      writeFile('/comp.html', [
+        `<div>`,
+        `<ng-template [ngIf]="show" [ngIfElse]="tmplName"><span>Content here</span></ng-template>`,
+        `</div>`,
+        `<ng-template #tmplName><p>Stuff</p></ng-template>`,
+      ].join('\n'));
+
+      await runMigration();
+      const content = tree.readContent('/comp.html');
+
+      expect(content).toBe([
+        `<div>`,
+        `@if (show) {`,
+        `<span>Content here</span>`,
+        `} @else {`,
+        `<p>Stuff</p>`,
+        `}`,
+        `</div>\n`,
+      ].join('\n'));
+    });
+
+    it('should migrate an if then else case as bindings', async () => {
+      writeFile('/comp.ts', `
+        import {Component} from '@angular/core';
+        import {NgIf,NgIfElse} from '@angular/common';
+
+        @Component({
+          templateUrl: './comp.html'
+        })
+        class Comp {
+          show = false;
+        }
+      `);
+
+      writeFile('/comp.html', [
+        `<div>`,
+        `<ng-template [ngIf]="show" [ngIfElse]="elseTmpl" [ngIfThenElse]="thenTmpl">Ignore Me</ng-template>`,
+        `</div>`,
+        `<ng-template #elseTmpl><p>Stuff</p></ng-template>`,
+        `<ng-template #thenTmpl><span>Content here</span></ng-template>`,
+      ].join('\n'));
+
+      await runMigration();
+      const content = tree.readContent('/comp.html');
+
+      expect(content).toBe([
+        `<div>`,
+        `@if (show) {`,
+        `<span>Content here</span>`,
+        `} @else {`,
+        `<p>Stuff</p>`,
+        `}`,
+        `</div>\n`,
+      ].join('\n'));
+    });
+
     it('should migrate an if case on a container', async () => {
       writeFile('/comp.ts', `
         import {Component} from '@angular/core';
@@ -483,6 +552,75 @@ describe('control flow migration', () => {
       ].join('\n'));
     });
 
+    it('should migrate a bound NgIfThenElse case with ng-templates 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', [
+        `<div>`,
+        `<ng-template [ngIf]="show" [ngIfThenElse]="thenTmpl" [ngIfElse]="elseTmpl">ignored</ng-template>`,
+        `</div>`,
+        `<ng-template #thenTmpl i18n="@@something"><span>Content here</span></ng-template>`,
+        `<ng-template #elseTmpl i18n="@@somethingElse"><p>other</p></ng-template>`,
+      ].join('\n'));
+
+      await runMigration();
+      const content = tree.readContent('/comp.html');
+
+      expect(content).toBe([
+        `<div>`,
+        `@if (show) {`,
+        `<ng-container i18n="@@something"><span>Content here</span></ng-container>`,
+        `} @else {`,
+        `<ng-container i18n="@@somethingElse"><p>other</p></ng-container>`,
+        `}`,
+        `</div>\n`,
+      ].join('\n'));
+    });
+
+    it('should migrate a bound NgIfElse case with ng-templates 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', [
+        `<div>`,
+        `<ng-template [ngIf]="show" [ngIfElse]="elseTmpl" i18n="@@something"><span>Content here</span></ng-template>`,
+        `</div>`,
+        `<ng-template #elseTmpl i18n="@@somethingElse"><p>other</p></ng-template>`,
+      ].join('\n'));
+
+      await runMigration();
+      const content = tree.readContent('/comp.html');
+
+      expect(content).toBe([
+        `<div>`,
+        `@if (show) {`,
+        `<ng-container i18n="@@something"><span>Content here</span></ng-container>`,
+        `} @else {`,
+        `<ng-container i18n="@@somethingElse"><p>other</p></ng-container>`,
+        `}`,
+        `</div>\n`,
+      ].join('\n'));
+    });
+
     it('should migrate an if else case', async () => {
       writeFile('/comp.ts', `
         import {Component} from '@angular/core';