Skip to content

Commit 6070c9d

Browse files
cexbrayatalxhub
authored andcommitted
fix(core): handle trackBy and aliased index in control flow migration (angular#52423)
Currently, the migration always use `$index` in the migrated trackBy function, whereas this variable might be aliased. The compiler then errors with: ``` error TS2339: Property '$index' does not exist on type 'UsersComponent'. 110 @for (user of users; track byId($index, user); let i = $index) { ``` This commit updates the migration to use the aliased index if there is one. PR Close angular#52423
1 parent 9980532 commit 6070c9d

File tree

2 files changed

+40
-0
lines changed

2 files changed

+40
-0
lines changed

packages/core/schematics/ng-generate/control-flow-migration/util.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ function migrateNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Resu
328328
const condition = parts[0].replace('let ', '');
329329
const loopVar = condition.split(' of ')[0];
330330
let trackBy = loopVar;
331+
let aliasedIndex: string|null = null;
331332
for (let i = 1; i < parts.length; i++) {
332333
const part = parts[i].trim();
333334

@@ -343,15 +344,29 @@ function migrateNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Resu
343344
const aliasParts = part.split('=');
344345
// -> 'let myIndex = $index'
345346
aliases.push(` ${aliasParts[0].trim()} = $${aliasParts[1].trim()}`);
347+
// if the aliased variable is the index, then we store it
348+
if (aliasParts[1].trim() === 'index') {
349+
// 'let myIndex' -> 'myIndex'
350+
aliasedIndex = aliasParts[0].trim().split(/\s+as\s+/)[1];
351+
}
346352
}
347353
// declared with `index as myIndex`
348354
if (part.match(aliasWithAsRegexp)) {
349355
// 'index as myIndex' -> ['index', 'myIndex']
350356
const aliasParts = part.split(/\s+as\s+/);
351357
// -> 'let myIndex = $index'
352358
aliases.push(` let ${aliasParts[1].trim()} = $${aliasParts[0].trim()}`);
359+
// if the aliased variable is the index, then we store it
360+
if (aliasParts[0].trim() === 'index') {
361+
aliasedIndex = aliasParts[1].trim();
362+
}
353363
}
354364
}
365+
// if an alias has been defined for the index, then the trackBy function must use it
366+
if (aliasedIndex !== null && trackBy !== loopVar) {
367+
// byId($index, user) -> byId(i, user)
368+
trackBy = trackBy.replace('$index', aliasedIndex);
369+
}
355370

356371
const aliasStr = (aliases.length > 0) ? `;${aliases.join(';')}` : '';
357372

packages/core/schematics/test/control_flow_migration_spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,31 @@ describe('control flow migration', () => {
920920
'template: `<ul>@for (itm of items; track itm; let myIndex = $index) {<li>{{itm.text}}</li>}</ul>`');
921921
});
922922

923+
it('should migrate with a trackBy function and an aliased index', async () => {
924+
writeFile('/comp.ts', `
925+
import {Component} from '@angular/core';
926+
import {NgFor} from '@angular/common';
927+
interface Item {
928+
id: number;
929+
text: string;
930+
}
931+
932+
@Component({
933+
imports: [NgFor],
934+
template: \`<ul><li *ngFor="let itm of items; trackBy: trackMeFn; index as i">{{itm.text}}</li></ul>\`
935+
})
936+
class Comp {
937+
items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
938+
}
939+
`);
940+
941+
await runMigration();
942+
const content = tree.readContent('/comp.ts');
943+
944+
expect(content).toContain(
945+
'template: `<ul>@for (itm of items; track trackMeFn(i, itm); let i = $index) {<li>{{itm.text}}</li>}</ul>`');
946+
});
947+
923948
it('should migrate with multiple aliases', async () => {
924949
writeFile('/comp.ts', `
925950
import {Component} from '@angular/core';

0 commit comments

Comments
 (0)