Skip to content

Commit 836aeba

Browse files
thePunderWomandylhunn
thePunderWoman
authored and
dylhunn
committed
fix(migrations): Change CF Migration ng-template placeholder generation and handling (#53394)
Using more unique characters makes it easier to parse placeholders that may contain JS logic, making it more flexible. fixes: #53386 fixes: #53385 fixes: #53384 PR Close #53394
1 parent d4fdba5 commit 836aeba

File tree

5 files changed

+120
-16
lines changed

5 files changed

+120
-16
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ function migrateStandardNgFor(etm: ElementToMigrate, tmpl: string, offset: numbe
113113
// template
114114
if (part.startsWith('template:')) {
115115
// this generates a special template placeholder just for this use case
116-
// which has a # at the end instead of the standard | in other placeholders
117-
tmplPlaceholder = `#${part.split(':')[1].trim()}#`;
116+
// which has a φ at the end instead of the standard δ in other placeholders
117+
tmplPlaceholder = `θ${part.split(':')[1].trim()}φ`;
118118
}
119119
// aliases
120120
// declared with `let myIndex = index`

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ function buildStandardIfElseBlock(
133133
.replace(' as ', '; as ')
134134
// replace 'let' with 'as' whatever spaces are between ; and 'let'
135135
.replace(/;\s*let/g, '; as');
136-
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
136+
const elsePlaceholder = `θ${etm.getTemplateName(elseString)}δ`;
137137
return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
138138
}
139139

@@ -153,9 +153,9 @@ function buildBoundIfElseBlock(etm: ElementToMigrate, tmpl: string, offset: numb
153153
} else if (aliases.length === 1) {
154154
condition += `; as ${aliases[0]}`;
155155
}
156-
const elsePlaceholder = `#${etm.elseAttr!.value}|`;
156+
const elsePlaceholder = `θ${etm.elseAttr!.value}δ`;
157157
if (etm.thenAttr !== undefined) {
158-
const thenPlaceholder = `#${etm.thenAttr!.value}|`;
158+
const thenPlaceholder = `θ${etm.thenAttr!.value}δ`;
159159
return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
160160
}
161161
return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
@@ -194,8 +194,8 @@ function buildStandardIfThenElseBlock(
194194
.replace(' as ', '; as ')
195195
// replace 'let' with 'as' whatever spaces are between ; and 'let'
196196
.replace(/;\s*let/g, '; as');
197-
const thenPlaceholder = `#${etm.getTemplateName(thenString, elseString)}|`;
198-
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
197+
const thenPlaceholder = `θ${etm.getTemplateName(thenString, elseString)}δ`;
198+
const elsePlaceholder = `θ${etm.getTemplateName(elseString)}δ`;
199199
return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
200200
}
201201

@@ -206,7 +206,7 @@ function buildStandardIfThenBlock(
206206
.replace(' as ', '; as ')
207207
// replace 'let' with 'as' whatever spaces are between ; and 'let'
208208
.replace(/;\s*let/g, '; as');
209-
const thenPlaceholder = `#${etm.getTemplateName(thenString)}|`;
209+
const thenPlaceholder = `θ${etm.getTemplateName(thenString)}δ`;
210210
return buildIfThenBlock(etm, tmpl, condition, thenPlaceholder, offset);
211211
}
212212

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,11 +141,12 @@ export class ElementToMigrate {
141141
getTemplateName(targetStr: string, secondStr?: string): string {
142142
const targetLocation = this.attr.value.indexOf(targetStr);
143143
const secondTargetLocation = secondStr ? this.attr.value.indexOf(secondStr) : undefined;
144-
return this.attr.value.slice(targetLocation + targetStr.length, secondTargetLocation)
145-
.replace(':', '')
146-
.trim()
147-
.split(';')[0]
148-
.trim();
144+
let templateName =
145+
this.attr.value.slice(targetLocation + targetStr.length, secondTargetLocation);
146+
if (templateName.startsWith(':')) {
147+
templateName = templateName.slice(1).trim();
148+
}
149+
return templateName.split(';')[0].trim();
149150
}
150151

151152
getValueEnd(offset: number): number {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,8 +325,8 @@ export function processNgTemplates(template: string): {migrated: string, err: Er
325325

326326
// swap placeholders and remove
327327
for (const [name, t] of templates) {
328-
const replaceRegex = new RegExp(`${name}\\|`, 'g');
329-
const forRegex = new RegExp(`${name}\\#`, 'g');
328+
const replaceRegex = new RegExp(`θ${name.slice(1)}\\δ`, 'g');
329+
const forRegex = new RegExp(`θ${name.slice(1)}\\φ`, 'g');
330330
const forMatches = [...template.matchAll(forRegex)];
331331
const matches = [...forMatches, ...template.matchAll(replaceRegex)];
332332
let safeToRemove = true;
@@ -367,7 +367,7 @@ export function processNgTemplates(template: string): {migrated: string, err: Er
367367
}
368368

369369
function replaceRemainingPlaceholders(template: string): string {
370-
const replaceRegex = new RegExp(`#\\w*\\|`, 'g');
370+
const replaceRegex = new RegExp(`θ.*δ`, 'g');
371371
const placeholders = [...template.matchAll(replaceRegex)];
372372
let migrated = template;
373373
for (let ph of placeholders) {

‎packages/core/schematics/test/control_flow_migration_spec.ts‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4046,6 +4046,109 @@ describe('control flow migration', () => {
40464046
`}`,
40474047
].join('\n'));
40484048
});
4049+
4050+
it('should handle OR logic in ngIf else case', async () => {
4051+
writeFile('/comp.ts', `
4052+
import {Component} from '@angular/core';
4053+
import {NgIf} from '@angular/common';
4054+
4055+
@Component({
4056+
templateUrl: './comp.html'
4057+
})
4058+
class Comp {
4059+
show = false;
4060+
}
4061+
`);
4062+
4063+
writeFile('/comp.html', [
4064+
`<div *ngIf="condition; else titleTemplate || defaultTemplate">Hello!</div>`,
4065+
`<ng-template #defaultTemplate> Default </ng-template>`,
4066+
].join('\n'));
4067+
4068+
await runMigration();
4069+
const content = tree.readContent('/comp.html');
4070+
4071+
expect(content).toBe([
4072+
`@if (condition) {`,
4073+
` <div>Hello!</div>`,
4074+
`} @else {`,
4075+
` <ng-template [ngTemplateOutlet]="titleTemplate || defaultTemplate"></ng-template>`,
4076+
`}`,
4077+
`<ng-template #defaultTemplate> Default </ng-template>`,
4078+
].join('\n'));
4079+
});
4080+
4081+
it('should handle ternaries in ngIfElse', async () => {
4082+
writeFile('/comp.ts', `
4083+
import {Component} from '@angular/core';
4084+
import {NgIf} from '@angular/common';
4085+
4086+
@Component({
4087+
templateUrl: './comp.html'
4088+
})
4089+
class Comp {
4090+
show = false;
4091+
}
4092+
`);
4093+
4094+
writeFile('/comp.html', [
4095+
`<ng-template`,
4096+
` [ngIf]="customClearTemplate"`,
4097+
` [ngIfElse]="isSidebarV3 || variant === 'v3' ? clearTemplateV3 : clearTemplate"`,
4098+
` [ngTemplateOutlet]="customClearTemplate"`,
4099+
`></ng-template>`,
4100+
`<ng-template #clearTemplateV3>v3</ng-template>`,
4101+
`<ng-template #clearTemplate>clear</ng-template>`,
4102+
].join('\n'));
4103+
4104+
await runMigration();
4105+
const content = tree.readContent('/comp.html');
4106+
4107+
expect(content).toBe([
4108+
`@if (customClearTemplate) {`,
4109+
` <ng-template`,
4110+
` [ngTemplateOutlet]="customClearTemplate"`,
4111+
` ></ng-template>`,
4112+
`} @else {`,
4113+
` <ng-template [ngTemplateOutlet]="isSidebarV3 || variant === 'v3' ? clearTemplateV3 : clearTemplate"></ng-template>`,
4114+
`}`,
4115+
`<ng-template #clearTemplateV3>v3</ng-template>`,
4116+
`<ng-template #clearTemplate>clear</ng-template>`,
4117+
].join('\n'));
4118+
});
4119+
4120+
it('should handle ternaries in ngIf', async () => {
4121+
writeFile('/comp.ts', `
4122+
import {Component} from '@angular/core';
4123+
import {NgIf} from '@angular/common';
4124+
4125+
@Component({
4126+
templateUrl: './comp.html'
4127+
})
4128+
class Comp {
4129+
show = false;
4130+
}
4131+
`);
4132+
4133+
writeFile('/comp.html', [
4134+
`<div *ngIf="!vm.isEmpty; else vm.loading ? loader : empty"></div>`,
4135+
`<ng-template #loader>Loading</ng-template>`,
4136+
`<ng-template #empty>Empty</ng-template>`,
4137+
].join('\n'));
4138+
4139+
await runMigration();
4140+
const content = tree.readContent('/comp.html');
4141+
4142+
expect(content).toBe([
4143+
`@if (!vm.isEmpty) {`,
4144+
` <div></div>`,
4145+
`} @else {`,
4146+
` <ng-template [ngTemplateOutlet]="vm.loading ? loader : empty"></ng-template>`,
4147+
`}`,
4148+
`<ng-template #loader>Loading</ng-template>`,
4149+
`<ng-template #empty>Empty</ng-template>`,
4150+
].join('\n'));
4151+
});
40494152
});
40504153

40514154
describe('formatting', () => {

0 commit comments

Comments
 (0)