Skip to content

Commit

Permalink
fix(migrations): Add support for ng-templates with i18n attributes (#…
Browse files Browse the repository at this point in the history 
…52597)

This makes sure that i18n attributes are preserved on ng-templates being removed during the migration.

fixes: #52517

PR Close #52597
  • Loading branch information
@jessicajaniuk @atscott
jessicajaniuk authored and atscott committed Nov 8, 2023
1 parent 6240613 commit 70d30c2
Show file tree
Hide file tree
Showing 3 changed files with 199 additions and 5 deletions.
16 changes: 13 additions & 3 deletions packages/core/schematics/ng-generate/control-flow-migration/types.ts
View file
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
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
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>`,
`</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

0 comments on commit 70d30c2

Please sign in to comment.