Skip to content

fix(migrations): Add support for ng-templates with i18n attributes #52597

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jessicajaniuk wants to merge 1 commit into from
Closed

fix(migrations): Add support for ng-templates with i18n attributes #52597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions packages/core/schematics/ng-generate/control-flow-migration/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,11 @@ export class Template {
count: number = 0;
contents: string = '';
children: string = '';
i18n: Attribute|null = null;

constructor(el: Element) {
constructor(el: Element, i18n: Attribute|null) {
this.el = el;
this.i18n = i18n;
}

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

override visitElement(el: Element): void {
if (el.name === ngtemplate) {
let i18n = null;
let templateAttr = null;
for (const attr of el.attrs) {
if (attr.name === 'i18n') {
i18n = attr;
}
if (attr.name.startsWith('#')) {
this.elements.push(new ElementToMigrate(el, attr));
this.templates.set(attr.name, new Template(el));
templateAttr = attr;
}
}
if (templateAttr !== null) {
this.elements.push(new ElementToMigrate(el, templateAttr));
this.templates.set(templateAttr.name, new Template(el, i18n));
}
}
super.visitElement(el, null);
}
Expand Down
20 changes: 18 additions & 2 deletions packages/core/schematics/ng-generate/control-flow-migration/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {HtmlParser, ParseTreeResult, visitAll} from '@angular/compiler';
import {Attribute, HtmlParser, ParseTreeResult, visitAll} from '@angular/compiler';
import {dirname, join} from 'path';
import ts from 'typescript';

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

function wrapIntoI18nContainer(i18nAttr: Attribute, content: string) {
const i18n = i18nAttr.value === '' ? 'i18n' : `i18n="${i18nAttr.value}"`;
return `<ng-container ${i18n}>${content}</ng-container>`;
}

export function processNgTemplates(template: string): string {
// count usage
const templates = countTemplateUsage(template);
Expand All @@ -182,7 +187,12 @@ export function processNgTemplates(template: string): string {
const placeholder = `${name}|`;

if (template.indexOf(placeholder) > -1) {
template = template.replace(placeholder, t.children);
if (t.i18n !== null) {
const container = wrapIntoI18nContainer(t.i18n, t.children);
template = template.replace(placeholder, container);
} else {
template = template.replace(placeholder, t.children);
}
if (t.count <= 2) {
template = template.replace(t.contents, '');
}
Expand Down Expand Up @@ -213,13 +223,19 @@ export function getOriginals(

export function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number):
{start: string, middle: string, end: string} {
const i18nAttr = etm.el.attrs.find(x => x.name === 'i18n');
if ((etm.el.name === 'ng-container' || etm.el.name === 'ng-template') &&
etm.el.attrs.length === 1) {
// this is the case where we're migrating and there's no need to keep the ng-container
const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
const middle = tmpl.slice(childStart, childEnd);
return {start: '', middle, end: ''};
} else if (etm.el.name === 'ng-template' && etm.el.attrs.length === 2 && i18nAttr !== undefined) {
const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
const middle = wrapIntoI18nContainer(i18nAttr, tmpl.slice(childStart, childEnd));
return {start: '', middle, end: ''};
}

const attrStart = etm.attr.keySpan!.start.offset - 1 - offset;
Expand Down
168 changes: 168 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,130 @@ describe('control flow migration', () => {
].join('\n'));
});

it('should migrate an if case with an ng-template with i18n', 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>`,
`<ng-template *ngIf="show" i18n="@@something"><span>Content here</span></ng-template>`,
Comment thread
jessicajaniuk marked this conversation as resolved.
Outdated
`</div>`,
].join('\n'));

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

expect(content).toBe([
`<div>`,
`@if (show) {`,
`<ng-container i18n="@@something"><span>Content here</span></ng-container>`,
`}`,
`</div>`,
].join('\n'));
});

it('should migrate an if case with an ng-template with empty i18n', 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>`,
`<ng-template *ngIf="show" i18n><span>Content here</span></ng-template>`,
`</div>`,
].join('\n'));

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

expect(content).toBe([
`<div>`,
`@if (show) {`,
`<ng-container i18n><span>Content here</span></ng-container>`,
`}`,
`</div>`,
].join('\n'));
});

it('should migrate an if case with an ng-container with i18n', 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>`,
`<ng-container *ngIf="show" i18n="@@something"><span>Content here</span></ng-container>`,
`</div>`,
].join('\n'));

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

expect(content).toBe([
`<div>`,
`@if (show) {`,
`<ng-container i18n="@@something"><span>Content here</span></ng-container>`,
`}`,
`</div>`,
].join('\n'));
});

it('should migrate an if case with an ng-container with empty i18n', 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>`,
`<ng-container *ngIf="show" i18n><span>Content here</span></ng-container>`,
`</div>`,
].join('\n'));

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

expect(content).toBe([
`<div>`,
`@if (show) {`,
`<ng-container i18n><span>Content here</span></ng-container>`,
`}`,
`</div>`,
].join('\n'));
});

it('should migrate an if else case', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
Expand Down Expand Up @@ -2560,6 +2684,50 @@ describe('control flow migration', () => {

expect(actual).toBe(expected);
});

it('should preserve i18n attribute on ng-templates in an if/else', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgIf} from '@angular/common';

@Component({
selector: 'declare-comp',
templateUrl: 'comp.html',
})
class DeclareComp {}
`);

writeFile('/comp.html', [
`<div>`,
` <ng-container *ngIf="cond; else testTpl">`,
` bla bla`,
` </ng-container>`,
`</div>`,
`<ng-template #testTpl i18n="@@test_key">`,
` <div class="test" *ngFor="let item of items"></div>`,
`</ng-template>`,
].join('\n'));

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

const expected = [
`<div>`,
` @if (cond) {\n`,
` bla bla`,
` `,
`} @else {`,
`<ng-container i18n="@@test_key">`,
` @for (item of items; track item) {`,
` <div class="test"></div>`,
`}`,
`</ng-container>`,
`}`,
`</div>\n`,
].join('\n');

expect(actual).toBe(expected);
});
});

describe('no migration needed', () => {
Expand Down