Skip to content

Commit 12f979d

Browse files
thePunderWomanatscott
thePunderWoman
authored and
atscott
committed
fix(migrations): Add support for ng-templates with i18n attributes (#52597)
This makes sure that i18n attributes are preserved on ng-templates being removed during the migration. fixes: #52517 PR Close #52597
1 parent abdbdf7 commit 12f979d

File tree

3 files changed

+199
-5
lines changed

3 files changed

+199
-5
lines changed

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,11 @@ export class Template {
8080
count: number = 0;
8181
contents: string = '';
8282
children: string = '';
83+
i18n: Attribute|null = null;
8384

84-
constructor(el: Element) {
85+
constructor(el: Element, i18n: Attribute|null) {
8586
this.el = el;
87+
this.i18n = i18n;
8688
}
8789

8890
generateContents(tmpl: string) {
@@ -155,12 +157,20 @@ export class TemplateCollector extends RecursiveVisitor {
155157

156158
override visitElement(el: Element): void {
157159
if (el.name === ngtemplate) {
160+
let i18n = null;
161+
let templateAttr = null;
158162
for (const attr of el.attrs) {
163+
if (attr.name === 'i18n') {
164+
i18n = attr;
165+
}
159166
if (attr.name.startsWith('#')) {
160-
this.elements.push(new ElementToMigrate(el, attr));
161-
this.templates.set(attr.name, new Template(el));
167+
templateAttr = attr;
162168
}
163169
}
170+
if (templateAttr !== null) {
171+
this.elements.push(new ElementToMigrate(el, templateAttr));
172+
this.templates.set(templateAttr.name, new Template(el, i18n));
173+
}
164174
}
165175
super.visitElement(el, null);
166176
}

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {HtmlParser, ParseTreeResult, visitAll} from '@angular/compiler';
9+
import {Attribute, HtmlParser, ParseTreeResult, visitAll} from '@angular/compiler';
1010
import {dirname, join} from 'path';
1111
import ts from 'typescript';
1212

@@ -173,6 +173,11 @@ export function countTemplateUsage(template: string): Map<string, Template> {
173173
return new Map<string, Template>();
174174
}
175175

176+
function wrapIntoI18nContainer(i18nAttr: Attribute, content: string) {
177+
const i18n = i18nAttr.value === '' ? 'i18n' : `i18n="${i18nAttr.value}"`;
178+
return `<ng-container ${i18n}>${content}</ng-container>`;
179+
}
180+
176181
export function processNgTemplates(template: string): string {
177182
// count usage
178183
const templates = countTemplateUsage(template);
@@ -182,7 +187,12 @@ export function processNgTemplates(template: string): string {
182187
const placeholder = `${name}|`;
183188

184189
if (template.indexOf(placeholder) > -1) {
185-
template = template.replace(placeholder, t.children);
190+
if (t.i18n !== null) {
191+
const container = wrapIntoI18nContainer(t.i18n, t.children);
192+
template = template.replace(placeholder, container);
193+
} else {
194+
template = template.replace(placeholder, t.children);
195+
}
186196
if (t.count <= 2) {
187197
template = template.replace(t.contents, '');
188198
}
@@ -213,13 +223,19 @@ export function getOriginals(
213223

214224
export function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number):
215225
{start: string, middle: string, end: string} {
226+
const i18nAttr = etm.el.attrs.find(x => x.name === 'i18n');
216227
if ((etm.el.name === 'ng-container' || etm.el.name === 'ng-template') &&
217228
etm.el.attrs.length === 1) {
218229
// this is the case where we're migrating and there's no need to keep the ng-container
219230
const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
220231
const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
221232
const middle = tmpl.slice(childStart, childEnd);
222233
return {start: '', middle, end: ''};
234+
} else if (etm.el.name === 'ng-template' && etm.el.attrs.length === 2 && i18nAttr !== undefined) {
235+
const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
236+
const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
237+
const middle = wrapIntoI18nContainer(i18nAttr, tmpl.slice(childStart, childEnd));
238+
return {start: '', middle, end: ''};
223239
}
224240

225241
const attrStart = etm.attr.keySpan!.start.offset - 1 - offset;

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

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,130 @@ describe('control flow migration', () => {
357357
].join('\n'));
358358
});
359359

360+
it('should migrate an if case with an ng-template with i18n', async () => {
361+
writeFile('/comp.ts', `
362+
import {Component} from '@angular/core';
363+
import {NgIf} from '@angular/common';
364+
365+
@Component({
366+
templateUrl: './comp.html'
367+
})
368+
class Comp {
369+
show = false;
370+
}
371+
`);
372+
373+
writeFile('/comp.html', [
374+
`<div>`,
375+
`<ng-template *ngIf="show" i18n="@@something"><span>Content here</span></ng-template>`,
376+
`</div>`,
377+
].join('\n'));
378+
379+
await runMigration();
380+
const content = tree.readContent('/comp.html');
381+
382+
expect(content).toBe([
383+
`<div>`,
384+
`@if (show) {`,
385+
`<ng-container i18n="@@something"><span>Content here</span></ng-container>`,
386+
`}`,
387+
`</div>`,
388+
].join('\n'));
389+
});
390+
391+
it('should migrate an if case with an ng-template with empty i18n', async () => {
392+
writeFile('/comp.ts', `
393+
import {Component} from '@angular/core';
394+
import {NgIf} from '@angular/common';
395+
396+
@Component({
397+
templateUrl: './comp.html'
398+
})
399+
class Comp {
400+
show = false;
401+
}
402+
`);
403+
404+
writeFile('/comp.html', [
405+
`<div>`,
406+
`<ng-template *ngIf="show" i18n><span>Content here</span></ng-template>`,
407+
`</div>`,
408+
].join('\n'));
409+
410+
await runMigration();
411+
const content = tree.readContent('/comp.html');
412+
413+
expect(content).toBe([
414+
`<div>`,
415+
`@if (show) {`,
416+
`<ng-container i18n><span>Content here</span></ng-container>`,
417+
`}`,
418+
`</div>`,
419+
].join('\n'));
420+
});
421+
422+
it('should migrate an if case with an ng-container with i18n', async () => {
423+
writeFile('/comp.ts', `
424+
import {Component} from '@angular/core';
425+
import {NgIf} from '@angular/common';
426+
427+
@Component({
428+
templateUrl: './comp.html'
429+
})
430+
class Comp {
431+
show = false;
432+
}
433+
`);
434+
435+
writeFile('/comp.html', [
436+
`<div>`,
437+
`<ng-container *ngIf="show" i18n="@@something"><span>Content here</span></ng-container>`,
438+
`</div>`,
439+
].join('\n'));
440+
441+
await runMigration();
442+
const content = tree.readContent('/comp.html');
443+
444+
expect(content).toBe([
445+
`<div>`,
446+
`@if (show) {`,
447+
`<ng-container i18n="@@something"><span>Content here</span></ng-container>`,
448+
`}`,
449+
`</div>`,
450+
].join('\n'));
451+
});
452+
453+
it('should migrate an if case with an ng-container with empty i18n', async () => {
454+
writeFile('/comp.ts', `
455+
import {Component} from '@angular/core';
456+
import {NgIf} from '@angular/common';
457+
458+
@Component({
459+
templateUrl: './comp.html'
460+
})
461+
class Comp {
462+
show = false;
463+
}
464+
`);
465+
466+
writeFile('/comp.html', [
467+
`<div>`,
468+
`<ng-container *ngIf="show" i18n><span>Content here</span></ng-container>`,
469+
`</div>`,
470+
].join('\n'));
471+
472+
await runMigration();
473+
const content = tree.readContent('/comp.html');
474+
475+
expect(content).toBe([
476+
`<div>`,
477+
`@if (show) {`,
478+
`<ng-container i18n><span>Content here</span></ng-container>`,
479+
`}`,
480+
`</div>`,
481+
].join('\n'));
482+
});
483+
360484
it('should migrate an if else case', async () => {
361485
writeFile('/comp.ts', `
362486
import {Component} from '@angular/core';
@@ -2560,6 +2684,50 @@ describe('control flow migration', () => {
25602684

25612685
expect(actual).toBe(expected);
25622686
});
2687+
2688+
it('should preserve i18n attribute on ng-templates in an if/else', async () => {
2689+
writeFile('/comp.ts', `
2690+
import {Component} from '@angular/core';
2691+
import {NgIf} from '@angular/common';
2692+
2693+
@Component({
2694+
selector: 'declare-comp',
2695+
templateUrl: 'comp.html',
2696+
})
2697+
class DeclareComp {}
2698+
`);
2699+
2700+
writeFile('/comp.html', [
2701+
`<div>`,
2702+
` <ng-container *ngIf="cond; else testTpl">`,
2703+
` bla bla`,
2704+
` </ng-container>`,
2705+
`</div>`,
2706+
`<ng-template #testTpl i18n="@@test_key">`,
2707+
` <div class="test" *ngFor="let item of items"></div>`,
2708+
`</ng-template>`,
2709+
].join('\n'));
2710+
2711+
await runMigration();
2712+
const actual = tree.readContent('/comp.html');
2713+
2714+
const expected = [
2715+
`<div>`,
2716+
` @if (cond) {\n`,
2717+
` bla bla`,
2718+
` `,
2719+
`} @else {`,
2720+
`<ng-container i18n="@@test_key">`,
2721+
` @for (item of items; track item) {`,
2722+
` <div class="test"></div>`,
2723+
`}`,
2724+
`</ng-container>`,
2725+
`}`,
2726+
`</div>\n`,
2727+
].join('\n');
2728+
2729+
expect(actual).toBe(expected);
2730+
});
25632731
});
25642732

25652733
describe('no migration needed', () => {

0 commit comments

Comments
 (0)