angular · jessicajaniuk · Nov 29, 2023
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/fors.ts b/packages/core/schematics/ng-generate/control-flow-migration/fors.ts
@@ -173,10 +173,11 @@ function migrateStandardNgFor(etm: ElementToMigrate, tmpl: string, offset: numbe

 function migrateBoundNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Result {
  const forAttrs = etm.forAttrs!;
-  const aliasMap = forAttrs.aliases;
+  const aliasAttrs = etm.aliasAttrs!;
+  const aliasMap = aliasAttrs.aliases;

  const originals = getOriginals(etm, tmpl, offset);
-  const condition = `${forAttrs.item} of ${forAttrs.forOf}`;
+  const condition = `${aliasAttrs.item} of ${forAttrs.forOf}`;

  const aliases = [];
  let aliasedIndex = '$index';
@@ -188,10 +189,10 @@ function migrateBoundNgFor(etm: ElementToMigrate, tmpl: string, offset: number):
  }
  const aliasStr = (aliases.length > 0) ? `;${aliases.join(';')}` : '';

-  let trackBy = forAttrs.item;
+  let trackBy = aliasAttrs.item;
  if (forAttrs.trackBy !== '') {
    // build trackby value
-    trackBy = `${forAttrs.trackBy.trim()}(${aliasedIndex}, ${forAttrs.item})`;
+    trackBy = `${forAttrs.trackBy.trim()}(${aliasedIndex}, ${aliasAttrs.item})`;
  }

  const {start, middle, end} = getMainBlock(etm, tmpl, offset);

diff --git a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
@@ -85,12 +85,22 @@ function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Resul
 }

 function buildIfBlock(etm: ElementToMigrate, tmpl: string, offset: number): Result {
+  const aliasAttrs = etm.aliasAttrs!;
+  const aliases = [...aliasAttrs.aliases.keys()];
+
  // includes the mandatory semicolon before as
  const lbString = etm.hasLineBreaks ? '\n' : '';
-  const condition = etm.attr.value
-                        .replace(' as ', '; as ')
-                        // replace 'let' with 'as' whatever spaces are between ; and 'let'
-                        .replace(/;\s*let/g, '; as');
+  let condition = etm.attr.value
+                      .replace(' as ', '; as ')
+                      // replace 'let' with 'as' whatever spaces are between ; and 'let'
+                      .replace(/;\s*let/g, '; as');
+  if (aliases.length > 1 || (aliases.length === 1 && condition.indexOf('; as') > -1)) {
+    // only 1 alias allowed
+    throw new Error(
+        'Found more than one alias on your ngIf. Remove one of them and re-run the migration.');
+  } else if (aliases.length === 1) {
+    condition += `; as ${aliases[0]}`;
+  }

  const originals = getOriginals(etm, tmpl, offset);

@@ -121,8 +131,18 @@ function buildStandardIfElseBlock(
 }

 function buildBoundIfElseBlock(etm: ElementToMigrate, tmpl: string, offset: number): Result {
+  const aliasAttrs = etm.aliasAttrs!;
+  const aliases = [...aliasAttrs.aliases.keys()];
+
  // includes the mandatory semicolon before as
-  const condition = etm.attr.value.replace(' as ', '; as ');
+  let condition = etm.attr.value.replace(' as ', '; as ');
+  if (aliases.length > 1 || (aliases.length === 1 && condition.indexOf('; as') > -1)) {
+    // only 1 alias allowed
+    throw new Error(
+        'Found more than one alias on your ngIf. Remove one of them and re-run the migration.');
+  } else if (aliases.length === 1) {
+    condition += `; as ${aliases[0]}`;
+  }
  const elsePlaceholder = `#${etm.elseAttr!.value}|`;
  if (etm.thenAttr !== undefined) {
    const thenPlaceholder = `#${etm.thenAttr!.value}|`;

diff --git a/packages/core/schematics/ng-generate/control-flow-migration/types.ts b/packages/core/schematics/ng-generate/control-flow-migration/types.ts
@@ -80,6 +80,9 @@ export type Result = {
 export interface ForAttributes {
  forOf: string;
  trackBy: string;
+}
+
+export interface AliasAttributes {
  item: string;
  aliases: Map<string, string>;
 }
@@ -101,17 +104,20 @@ export class ElementToMigrate {
  elseAttr: Attribute|undefined;
  thenAttr: Attribute|undefined;
  forAttrs: ForAttributes|undefined;
+  aliasAttrs: AliasAttributes|undefined;
  nestCount = 0;
  hasLineBreaks = false;

  constructor(
      el: Element, attr: Attribute, elseAttr: Attribute|undefined = undefined,
-      thenAttr: Attribute|undefined = undefined, forAttrs: ForAttributes|undefined = undefined) {
+      thenAttr: Attribute|undefined = undefined, forAttrs: ForAttributes|undefined = undefined,
+      aliasAttrs: AliasAttributes|undefined = undefined) {
    this.el = el;
    this.attr = attr;
    this.elseAttr = elseAttr;
    this.thenAttr = thenAttr;
    this.forAttrs = forAttrs;
+    this.aliasAttrs = aliasAttrs;
  }

  getCondition(): string {
@@ -317,16 +323,16 @@ export class ElementCollector extends RecursiveVisitor {
          const elseAttr = el.attrs.find(x => x.name === boundngifelse);
          const thenAttr = el.attrs.find(x => x.name === boundngifthenelse);
          const forAttrs = attr.name === nakedngfor ? this.getForAttrs(el) : undefined;
-          this.elements.push(new ElementToMigrate(el, attr, elseAttr, thenAttr, forAttrs));
+          const aliasAttrs = this.getAliasAttrs(el);
+          this.elements.push(
+              new ElementToMigrate(el, attr, elseAttr, thenAttr, forAttrs, aliasAttrs));
        }
      }
    }
    super.visitElement(el, null);
  }

  private getForAttrs(el: Element): ForAttributes {
-    const aliases = new Map<string, string>();
-    let item = '';
    let trackBy = '';
    let forOf = '';
    for (const attr of el.attrs) {
@@ -336,6 +342,14 @@ export class ElementCollector extends RecursiveVisitor {
      if (attr.name === '[ngForOf]') {
        forOf = attr.value;
      }
+    }
+    return {forOf, trackBy};
+  }
+
+  private getAliasAttrs(el: Element): AliasAttributes {
+    const aliases = new Map<string, string>();
+    let item = '';
+    for (const attr of el.attrs) {
      if (attr.name.startsWith('let-')) {
        if (attr.value === '') {
          // item
@@ -346,7 +360,7 @@ export class ElementCollector extends RecursiveVisitor {
        }
      }
    }
-    return {forOf, trackBy, item, aliases};
+    return {item, aliases};
  }
 }


diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts
@@ -416,10 +416,24 @@ function isI18nTemplate(etm: ElementToMigrate, i18nAttr: Attribute|undefined): b
 }

 function isRemovableContainer(etm: ElementToMigrate): boolean {
-  return (etm.el.name === 'ng-container' || etm.el.name === 'ng-template') &&
-      (etm.el.attrs.length === 1 || etm.forAttrs !== undefined ||
-       (etm.el.attrs.length === 2 && etm.elseAttr !== undefined) ||
-       (etm.el.attrs.length === 3 && etm.elseAttr !== undefined && etm.thenAttr !== undefined));
+  let attrCount = countAttributes(etm);
+  const safeToRemove = etm.el.attrs.length === attrCount;
+  return (etm.el.name === 'ng-container' || etm.el.name === 'ng-template') && safeToRemove;
+}
+
+function countAttributes(etm: ElementToMigrate): number {
+  let attrCount = 1;
+  if (etm.elseAttr !== undefined) {
+    attrCount++;
+  }
+  if (etm.thenAttr !== undefined) {
+    attrCount++;
+  }
+  attrCount += etm.aliasAttrs?.aliases.size ?? 0;
+  attrCount += etm.aliasAttrs?.item ? 1 : 0;
+  attrCount += etm.forAttrs?.trackBy ? 1 : 0;
+  attrCount += etm.forAttrs?.forOf ? 1 : 0;
+  return attrCount;
 }

 /**

diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -327,6 +327,35 @@ describe('control flow migration', () => {
      ].join('\n'));
    });

+    it('should migrate an if case as a binding with let variables', 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">`,
+        `  {{ data }}`,
+        `</ng-template>`,
+      ].join('\n'));
+
+      await runMigration();
+      const content = tree.readContent('/comp.html');
+
+      expect(content).toBe([
+        `@if (data$ | async; as data) {`,
+        `  {{ data }}`,
+        `}`,
+      ].join('\n'));
+    });
+
    it('should migrate an if else case as bindings', async () => {
      writeFile('/comp.ts', `
        import {Component} from '@angular/core';