fix(migrations): Fix cf migration bug with parsing for loop conditions properly (#53558)

thePunderWoman · atscott · commit 886aa7b2a99a · 2023-12-14T09:33:27.000-08:00
The order of operations for getting for loop parts was inverted resulting in the StringStack to never be reduced. fixes: #53555 PR Close #53558
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/fors.ts b/packages/core/schematics/ng-generate/control-flow-migration/fors.ts
@@ -219,7 +219,6 @@ function getNgForParts(expression: string): string[] {
     const char = expression[i];
     const isInString = stringStack.length === 0;
     const isInCommaSeparated = commaSeparatedStack.length === 0;
-
     // Any semicolon is a delimiter, as well as any comma outside
     // of comma-separated syntax, as long as they're outside of a string.
     if (isInString && current.length > 0 &&
@@ -229,10 +228,10 @@ function getNgForParts(expression: string): string[] {
       continue;
     }
 
-    if (stringPairs.has(char)) {
-      stringStack.push(stringPairs.get(char)!);
-    } else if (stringStack.length > 0 && stringStack[stringStack.length - 1] === char) {
+    if (stringStack.length > 0 && stringStack[stringStack.length - 1] === char) {
       stringStack.pop();
+    } else if (stringPairs.has(char)) {
+      stringStack.push(stringPairs.get(char)!);
     }
 
     if (commaSeparatedSyntax.has(char)) {
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -2284,6 +2284,64 @@ describe('control flow migration', () => {
       expect(actual).toBe(expected);
     });
 
+    it('should migrate ngFor with a long ternary and trackby', async () => {
+      writeFile('/comp.ts', `
+        import {Component} from '@angular/core';
+        import {NgFor} from '@angular/common';
+        interface Item {
+          id: number;
+          text: string;
+        }
+
+        @Component({
+          imports: [NgFor,NgForOf],
+          templateUrl: 'comp.html',
+        })
+        class Comp {
+          items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
+        }
+      `);
+
+      writeFile('/comp.html', [
+        `<div`,
+        `  *ngFor="`,
+        `    let item of section === 'manage'`,
+        `      ? filteredPermissions?.manage`,
+        `      : section === 'customFields'`,
+        `      ? filteredPermissions?.customFields`,
+        `      : section === 'createAndDelete'`,
+        `      ? filteredPermissions?.createAndDelete`,
+        `      : filteredPermissions?.team;`,
+        `    trackBy: trackById`,
+        `  "`,
+        `>`,
+        `  {{ item }}`,
+        `</div>`,
+      ].join('\n'));
+
+      await runMigration();
+      const actual = tree.readContent('/comp.html');
+
+      const expected = [
+        `@for (`,
+        `  item of section === 'manage'`,
+        `  ? filteredPermissions?.manage`,
+        `  : section === 'customFields'`,
+        `  ? filteredPermissions?.customFields`,
+        `  : section === 'createAndDelete'`,
+        `  ? filteredPermissions?.createAndDelete`,
+        `  : filteredPermissions?.team; track trackById($index,`,
+        `  item)) {`,
+        `  <div`,
+        `    >`,
+        `    {{ item }}`,
+        `  </div>`,
+        `}`,
+      ].join('\n');
+
+      expect(actual).toBe(expected);
+    });
+
     it('should migrate ngForOf with track by and multiple aliases', async () => {
       writeFile('/comp.ts', `
         import {Component} from '@angular/core';
