Skip to content

Commit

Permalink
fix(migrations): Change CF Migration ng-template placeholder generati…
Browse files Browse the repository at this point in the history 
…on 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
  • Loading branch information
@thePunderWoman @dylhunn
thePunderWoman authored and dylhunn committed Dec 6, 2023
1 parent 580af8e commit 2a5a8f6
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 16 deletions.
4 changes: 2 additions & 2 deletions packages/core/schematics/ng-generate/control-flow-migration/fors.ts
View file
Expand Up @@ -113,8 +113,8 @@ function migrateStandardNgFor(etm: ElementToMigrate, tmpl: string, offset: numbe
// template
if (part.startsWith('template:')) {
// this generates a special template placeholder just for this use case
// which has a # at the end instead of the standard | in other placeholders
tmplPlaceholder = `#${part.split(':')[1].trim()}#`;
// which has a φ at the end instead of the standard δ in other placeholders
tmplPlaceholder = `θ${part.split(':')[1].trim()}φ`;
}
// aliases
// declared with `let myIndex = index`
Expand Down
12 changes: 6 additions & 6 deletions packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
View file
Expand Up @@ -133,7 +133,7 @@ function buildStandardIfElseBlock(
.replace(' as ', '; as ')
// replace 'let' with 'as' whatever spaces are between ; and 'let'
.replace(/;\s*let/g, '; as');
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
const elsePlaceholder = `θ${etm.getTemplateName(elseString)}δ`;
return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
}

Expand All @@ -153,9 +153,9 @@ function buildBoundIfElseBlock(etm: ElementToMigrate, tmpl: string, offset: numb
} else if (aliases.length === 1) {
condition += `; as ${aliases[0]}`;
}
const elsePlaceholder = `#${etm.elseAttr!.value}|`;
const elsePlaceholder = `θ${etm.elseAttr!.value}δ`;
if (etm.thenAttr !== undefined) {
const thenPlaceholder = `#${etm.thenAttr!.value}|`;
const thenPlaceholder = `θ${etm.thenAttr!.value}δ`;
return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
}
return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
Expand Down Expand Up @@ -194,8 +194,8 @@ function buildStandardIfThenElseBlock(
.replace(' as ', '; as ')
// replace 'let' with 'as' whatever spaces are between ; and 'let'
.replace(/;\s*let/g, '; as');
const thenPlaceholder = `#${etm.getTemplateName(thenString, elseString)}|`;
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
const thenPlaceholder = `θ${etm.getTemplateName(thenString, elseString)}δ`;
const elsePlaceholder = `θ${etm.getTemplateName(elseString)}δ`;
return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
}

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

Expand Down
11 changes: 6 additions & 5 deletions packages/core/schematics/ng-generate/control-flow-migration/types.ts
View file
Expand Up @@ -141,11 +141,12 @@ export class ElementToMigrate {
getTemplateName(targetStr: string, secondStr?: string): string {
const targetLocation = this.attr.value.indexOf(targetStr);
const secondTargetLocation = secondStr ? this.attr.value.indexOf(secondStr) : undefined;
return this.attr.value.slice(targetLocation + targetStr.length, secondTargetLocation)
.replace(':', '')
.trim()
.split(';')[0]
.trim();
let templateName =
this.attr.value.slice(targetLocation + targetStr.length, secondTargetLocation);
if (templateName.startsWith(':')) {
templateName = templateName.slice(1).trim();
}
return templateName.split(';')[0].trim();
}

getValueEnd(offset: number): number {
Expand Down
6 changes: 3 additions & 3 deletions packages/core/schematics/ng-generate/control-flow-migration/util.ts
View file
Expand Up @@ -325,8 +325,8 @@ export function processNgTemplates(template: string): {migrated: string, err: Er

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

function replaceRemainingPlaceholders(template: string): string {
const replaceRegex = new RegExp(`#\\w*\\|`, 'g');
const replaceRegex = new RegExp(`θ.*δ`, 'g');
const placeholders = [...template.matchAll(replaceRegex)];
let migrated = template;
for (let ph of placeholders) {
Expand Down
103 changes: 103 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
View file
Expand Up @@ -4046,6 +4046,109 @@ describe('control flow migration', () => {
`}`,
].join('\n'));
});

it('should handle OR logic in ngIf else case', 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', [
`<div *ngIf="condition; else titleTemplate || defaultTemplate">Hello!</div>`,
`<ng-template #defaultTemplate> Default </ng-template>`,
].join('\n'));

await runMigration();
const content = tree.readContent('/comp.html');

expect(content).toBe([
`@if (condition) {`,
` <div>Hello!</div>`,
`} @else {`,
` <ng-template [ngTemplateOutlet]="titleTemplate || defaultTemplate"></ng-template>`,
`}`,
`<ng-template #defaultTemplate> Default </ng-template>`,
].join('\n'));
});

it('should handle ternaries in ngIfElse', 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]="customClearTemplate"`,
` [ngIfElse]="isSidebarV3 || variant === 'v3' ? clearTemplateV3 : clearTemplate"`,
` [ngTemplateOutlet]="customClearTemplate"`,
`></ng-template>`,
`<ng-template #clearTemplateV3>v3</ng-template>`,
`<ng-template #clearTemplate>clear</ng-template>`,
].join('\n'));

await runMigration();
const content = tree.readContent('/comp.html');

expect(content).toBe([
`@if (customClearTemplate) {`,
` <ng-template`,
` [ngTemplateOutlet]="customClearTemplate"`,
` ></ng-template>`,
`} @else {`,
` <ng-template [ngTemplateOutlet]="isSidebarV3 || variant === 'v3' ? clearTemplateV3 : clearTemplate"></ng-template>`,
`}`,
`<ng-template #clearTemplateV3>v3</ng-template>`,
`<ng-template #clearTemplate>clear</ng-template>`,
].join('\n'));
});

it('should handle ternaries in ngIf', 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', [
`<div *ngIf="!vm.isEmpty; else vm.loading ? loader : empty"></div>`,
`<ng-template #loader>Loading</ng-template>`,
`<ng-template #empty>Empty</ng-template>`,
].join('\n'));

await runMigration();
const content = tree.readContent('/comp.html');

expect(content).toBe([
`@if (!vm.isEmpty) {`,
` <div></div>`,
`} @else {`,
` <ng-template [ngTemplateOutlet]="vm.loading ? loader : empty"></ng-template>`,
`}`,
`<ng-template #loader>Loading</ng-template>`,
`<ng-template #empty>Empty</ng-template>`,
].join('\n'));
});
});

describe('formatting', () => {
Expand Down

0 comments on commit 2a5a8f6

Please sign in to comment.