Skip to content

Commit 4f125c5

Browse files
thePunderWomanatscott
thePunderWoman
authored and
atscott
committed
fix(migrations): Switches to multiple passes to fix several reported bugs (#52592)
Rather than migrate all in one pass, this now migrates in a separate pass per control flow item plus one for templates at the end. This resolves issues with multiple control flow items on a single element as well as making sure ng-templates are fully migrated before being moved to new locations. fixes: #52518 fixes: #52516 fixes: #52513 PR Close #52592
1 parent c30dde2 commit 4f125c5

File tree

7 files changed

+666
-402
lines changed

7 files changed

+666
-402
lines changed

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

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {visitAll} from '@angular/compiler';
10+
11+
import {ElementCollector, ElementToMigrate, MigrateError, Result} from './types';
12+
import {calculateNesting, getMainBlock, getOriginals, hasLineBreaks, parseTemplate, reduceNestingOffset} from './util';
13+
14+
export const ngfor = '*ngFor';
15+
export const commaSeparatedSyntax = new Map([
16+
['(', ')'],
17+
['{', '}'],
18+
['[', ']'],
19+
]);
20+
export const stringPairs = new Map([
21+
[`"`, `"`],
22+
[`'`, `'`],
23+
]);
24+
25+
/**
26+
* Replaces structural directive ngFor instances with new for.
27+
* Returns null if the migration failed (e.g. there was a syntax error).
28+
*/
29+
export function migrateFor(template: string): {migrated: string, errors: MigrateError[]} {
30+
let errors: MigrateError[] = [];
31+
let parsed = parseTemplate(template);
32+
if (parsed === null) {
33+
return {migrated: template, errors};
34+
}
35+
36+
let result = template;
37+
const visitor = new ElementCollector([ngfor]);
38+
visitAll(visitor, parsed.rootNodes);
39+
calculateNesting(visitor, hasLineBreaks(template));
40+
41+
// this tracks the character shift from different lengths of blocks from
42+
// the prior directives so as to adjust for nested block replacement during
43+
// migration. Each block calculates length differences and passes that offset
44+
// to the next migrating block to adjust character offsets properly.
45+
let offset = 0;
46+
let nestLevel = -1;
47+
let postOffsets: number[] = [];
48+
for (const el of visitor.elements) {
49+
let migrateResult: Result = {tmpl: result, offsets: {pre: 0, post: 0}};
50+
// applies the post offsets after closing
51+
offset = reduceNestingOffset(el, nestLevel, offset, postOffsets);
52+
53+
try {
54+
migrateResult = migrateNgFor(el, result, offset);
55+
} catch (error: unknown) {
56+
errors.push({type: ngfor, error});
57+
}
58+
59+
result = migrateResult.tmpl;
60+
offset += migrateResult.offsets.pre;
61+
postOffsets.push(migrateResult.offsets.post);
62+
nestLevel = el.nestCount;
63+
}
64+
65+
return {migrated: result, errors};
66+
}
67+
68+
69+
function migrateNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Result {
70+
const aliasWithEqualRegexp = /=\s*(count|index|first|last|even|odd)/gm;
71+
const aliasWithAsRegexp = /(count|index|first|last|even|odd)\s+as/gm;
72+
const aliases = [];
73+
const lbString = etm.hasLineBreaks ? '\n' : '';
74+
const lbSpaces = etm.hasLineBreaks ? `\n ` : '';
75+
const parts = getNgForParts(etm.attr.value);
76+
77+
const originals = getOriginals(etm, tmpl, offset);
78+
79+
// first portion should always be the loop definition prefixed with `let`
80+
const condition = parts[0].replace('let ', '');
81+
const loopVar = condition.split(' of ')[0];
82+
let trackBy = loopVar;
83+
let aliasedIndex: string|null = null;
84+
for (let i = 1; i < parts.length; i++) {
85+
const part = parts[i].trim();
86+
87+
if (part.startsWith('trackBy:')) {
88+
// build trackby value
89+
const trackByFn = part.replace('trackBy:', '').trim();
90+
trackBy = `${trackByFn}($index, ${loopVar})`;
91+
}
92+
// aliases
93+
// declared with `let myIndex = index`
94+
if (part.match(aliasWithEqualRegexp)) {
95+
// 'let myIndex = index' -> ['let myIndex', 'index']
96+
const aliasParts = part.split('=');
97+
// -> 'let myIndex = $index'
98+
aliases.push(` ${aliasParts[0].trim()} = $${aliasParts[1].trim()}`);
99+
// if the aliased variable is the index, then we store it
100+
if (aliasParts[1].trim() === 'index') {
101+
// 'let myIndex' -> 'myIndex'
102+
aliasedIndex = aliasParts[0].replace('let', '').trim();
103+
}
104+
}
105+
// declared with `index as myIndex`
106+
if (part.match(aliasWithAsRegexp)) {
107+
// 'index as myIndex' -> ['index', 'myIndex']
108+
const aliasParts = part.split(/\s+as\s+/);
109+
// -> 'let myIndex = $index'
110+
aliases.push(` let ${aliasParts[1].trim()} = $${aliasParts[0].trim()}`);
111+
// if the aliased variable is the index, then we store it
112+
if (aliasParts[0].trim() === 'index') {
113+
aliasedIndex = aliasParts[1].trim();
114+
}
115+
}
116+
}
117+
// if an alias has been defined for the index, then the trackBy function must use it
118+
if (aliasedIndex !== null && trackBy !== loopVar) {
119+
// byId($index, user) -> byId(i, user)
120+
trackBy = trackBy.replace('$index', aliasedIndex);
121+
}
122+
123+
const aliasStr = (aliases.length > 0) ? `;${aliases.join(';')}` : '';
124+
125+
const {start, middle, end} = getMainBlock(etm, tmpl, offset);
126+
const startBlock = `@for (${condition}; track ${trackBy}${aliasStr}) {${lbSpaces}${start}`;
127+
128+
const endBlock = `${end}${lbString}}`;
129+
const forBlock = startBlock + middle + endBlock;
130+
131+
const updatedTmpl = tmpl.slice(0, etm.start(offset)) + forBlock + tmpl.slice(etm.end(offset));
132+
133+
const pre = originals.start.length - startBlock.length;
134+
const post = originals.end.length - endBlock.length;
135+
136+
return {tmpl: updatedTmpl, offsets: {pre, post}};
137+
}
138+
139+
function getNgForParts(expression: string): string[] {
140+
const parts: string[] = [];
141+
const commaSeparatedStack: string[] = [];
142+
const stringStack: string[] = [];
143+
let current = '';
144+
145+
for (let i = 0; i < expression.length; i++) {
146+
const char = expression[i];
147+
const isInString = stringStack.length === 0;
148+
const isInCommaSeparated = commaSeparatedStack.length === 0;
149+
150+
// Any semicolon is a delimiter, as well as any comma outside
151+
// of comma-separated syntax, as long as they're outside of a string.
152+
if (isInString && current.length > 0 &&
153+
(char === ';' || (char === ',' && isInCommaSeparated))) {
154+
parts.push(current);
155+
current = '';
156+
continue;
157+
}
158+
159+
if (stringPairs.has(char)) {
160+
stringStack.push(stringPairs.get(char)!);
161+
} else if (stringStack.length > 0 && stringStack[stringStack.length - 1] === char) {
162+
stringStack.pop();
163+
}
164+
165+
if (commaSeparatedSyntax.has(char)) {
166+
commaSeparatedStack.push(commaSeparatedSyntax.get(char)!);
167+
} else if (
168+
commaSeparatedStack.length > 0 &&
169+
commaSeparatedStack[commaSeparatedStack.length - 1] === char) {
170+
commaSeparatedStack.pop();
171+
}
172+
173+
current += char;
174+
}
175+
176+
if (current.length > 0) {
177+
parts.push(current);
178+
}
179+
180+
return parts;
181+
}

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

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {visitAll} from '@angular/compiler';
10+
11+
import {ElementCollector, ElementToMigrate, MigrateError, Result} from './types';
12+
import {calculateNesting, getMainBlock, getOriginals, hasLineBreaks, parseTemplate, reduceNestingOffset} from './util';
13+
14+
export const ngif = '*ngIf';
15+
export const boundngif = '[ngIf]';
16+
export const nakedngif = 'ngIf';
17+
18+
export const ifs = [
19+
ngif,
20+
nakedngif,
21+
boundngif,
22+
];
23+
24+
/**
25+
* Replaces structural directive ngif instances with new if.
26+
* Returns null if the migration failed (e.g. there was a syntax error).
27+
*/
28+
export function migrateIf(template: string): {migrated: string, errors: MigrateError[]} {
29+
let errors: MigrateError[] = [];
30+
let parsed = parseTemplate(template);
31+
if (parsed === null) {
32+
return {migrated: template, errors};
33+
}
34+
35+
let result = template;
36+
const visitor = new ElementCollector(ifs);
37+
visitAll(visitor, parsed.rootNodes);
38+
calculateNesting(visitor, hasLineBreaks(template));
39+
40+
// this tracks the character shift from different lengths of blocks from
41+
// the prior directives so as to adjust for nested block replacement during
42+
// migration. Each block calculates length differences and passes that offset
43+
// to the next migrating block to adjust character offsets properly.
44+
let offset = 0;
45+
let nestLevel = -1;
46+
let postOffsets: number[] = [];
47+
for (const el of visitor.elements) {
48+
let migrateResult: Result = {tmpl: result, offsets: {pre: 0, post: 0}};
49+
// applies the post offsets after closing
50+
offset = reduceNestingOffset(el, nestLevel, offset, postOffsets);
51+
52+
try {
53+
migrateResult = migrateNgIf(el, result, offset);
54+
} catch (error: unknown) {
55+
errors.push({type: ngif, error});
56+
}
57+
58+
result = migrateResult.tmpl;
59+
offset += migrateResult.offsets.pre;
60+
postOffsets.push(migrateResult.offsets.post);
61+
nestLevel = el.nestCount;
62+
}
63+
64+
return {migrated: result, errors};
65+
}
66+
67+
export function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Result {
68+
const matchThen = etm.attr.value.match(/;\s*then/gm);
69+
const matchElse = etm.attr.value.match(/;\s*else/gm);
70+
71+
if (matchThen && matchThen.length > 0) {
72+
return buildIfThenElseBlock(etm, tmpl, matchThen[0], matchElse![0], offset);
73+
} else if (matchElse && matchElse.length > 0) {
74+
// just else
75+
return buildIfElseBlock(etm, tmpl, matchElse[0], offset);
76+
}
77+
78+
return buildIfBlock(etm, tmpl, offset);
79+
}
80+
81+
function buildIfBlock(etm: ElementToMigrate, tmpl: string, offset: number): Result {
82+
// includes the mandatory semicolon before as
83+
const lbString = etm.hasLineBreaks ? '\n' : '';
84+
const condition = etm.attr.value.replace(' as ', '; as ');
85+
86+
const originals = getOriginals(etm, tmpl, offset);
87+
88+
const {start, middle, end} = getMainBlock(etm, tmpl, offset);
89+
const startBlock = `@if (${condition}) {${lbString}${start}`;
90+
const endBlock = `${end}${lbString}}`;
91+
92+
const ifBlock = startBlock + middle + endBlock;
93+
const updatedTmpl = tmpl.slice(0, etm.start(offset)) + ifBlock + tmpl.slice(etm.end(offset));
94+
95+
// this should be the difference between the starting element up to the start of the closing
96+
// element and the mainblock sans }
97+
const pre = originals.start.length - startBlock.length;
98+
const post = originals.end.length - endBlock.length;
99+
100+
return {tmpl: updatedTmpl, offsets: {pre, post}};
101+
}
102+
103+
function buildIfElseBlock(
104+
etm: ElementToMigrate, tmpl: string, elseString: string, offset: number): Result {
105+
// includes the mandatory semicolon before as
106+
const lbString = etm.hasLineBreaks ? '\n' : '';
107+
const condition = etm.getCondition(elseString).replace(' as ', '; as ');
108+
109+
const originals = getOriginals(etm, tmpl, offset);
110+
111+
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
112+
const {start, middle, end} = getMainBlock(etm, tmpl, offset);
113+
const startBlock = `@if (${condition}) {${lbString}${start}`;
114+
115+
const elseBlock = `${end}${lbString}} @else {${lbString}`;
116+
117+
const postBlock = elseBlock + elsePlaceholder + `${lbString}}`;
118+
const ifElseBlock = startBlock + middle + postBlock;
119+
120+
const tmplStart = tmpl.slice(0, etm.start(offset));
121+
const tmplEnd = tmpl.slice(etm.end(offset));
122+
const updatedTmpl = tmplStart + ifElseBlock + tmplEnd;
123+
124+
const pre = originals.start.length - startBlock.length;
125+
const post = originals.end.length - postBlock.length;
126+
127+
return {tmpl: updatedTmpl, offsets: {pre, post}};
128+
}
129+
130+
function buildIfThenElseBlock(
131+
etm: ElementToMigrate, tmpl: string, thenString: string, elseString: string,
132+
offset: number): Result {
133+
const condition = etm.getCondition(thenString).replace(' as ', '; as ');
134+
const lbString = etm.hasLineBreaks ? '\n' : '';
135+
136+
const originals = getOriginals(etm, tmpl, offset);
137+
138+
const startBlock = `@if (${condition}) {${lbString}`;
139+
const elseBlock = `${lbString}} @else {${lbString}`;
140+
141+
const thenPlaceholder = `#${etm.getTemplateName(thenString, elseString)}|`;
142+
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
143+
144+
const postBlock = thenPlaceholder + elseBlock + elsePlaceholder + `${lbString}}`;
145+
const ifThenElseBlock = startBlock + postBlock;
146+
147+
const tmplStart = tmpl.slice(0, etm.start(offset));
148+
const tmplEnd = tmpl.slice(etm.end(offset));
149+
150+
const updatedTmpl = tmplStart + ifThenElseBlock + tmplEnd;
151+
152+
const pre = originals.start.length - startBlock.length;
153+
const post = originals.end.length - postBlock.length;
154+
155+
return {tmpl: updatedTmpl, offsets: {pre, post}};
156+
}

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@ import {normalizePath} from '../../utils/change_tracker';
1313
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
1414
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
1515

16+
import {migrateFor} from './fors';
17+
import {migrateIf} from './ifs';
18+
import {migrateSwitch} from './switches';
1619
import {AnalyzedFile, MigrateError} from './types';
17-
import {analyze, migrateTemplate} from './util';
20+
import {analyze, processNgTemplates} from './util';
1821

1922
interface Options {
2023
path: string;
@@ -84,7 +87,18 @@ function runControlFlowMigration(
8487
for (const [start, end] of ranges) {
8588
const template = content.slice(start, end);
8689
const length = (end ?? content.length) - start;
87-
const {migrated, errors} = migrateTemplate(template);
90+
91+
const ifResult = migrateIf(template);
92+
const forResult = migrateFor(ifResult.migrated);
93+
const switchResult = migrateSwitch(forResult.migrated);
94+
95+
const errors = [
96+
...ifResult.errors,
97+
...forResult.errors,
98+
...switchResult.errors,
99+
];
100+
101+
const migrated = processNgTemplates(switchResult.migrated);
88102

89103
if (migrated !== null) {
90104
update.remove(start, length);

0 commit comments

Comments
 (0)