Skip to content

Commit

Permalink
fix(migrations): fixes CF migration i18n ng-template offsets
Browse files Browse the repository at this point in the history
This addresses an issue where multiple ng-templates are present with i18n attributes. The offsets would be incorrectly accounted for when being replaced with an ng-container.

fixes: angular#53149
  • Loading branch information
thePunderWoman committed Nov 27, 2023
1 parent 7db4ecd commit b3a21c3
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 28 deletions.
Expand Up @@ -302,8 +302,14 @@ export function getTemplates(template: string): Map<string, Template> {
}

function wrapIntoI18nContainer(i18nAttr: Attribute, content: string) {
const {start, middle, end} = generatei18nContainer(i18nAttr, content);
return `${start}${middle}${end}`;
}

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

/**
Expand Down Expand Up @@ -427,8 +433,7 @@ export function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number
} else if (isI18nTemplate(etm, i18nAttr)) {
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: ''};
return generatei18nContainer(i18nAttr!, tmpl.slice(childStart, childEnd));
}

const attrStart = etm.attr.keySpan!.start.offset - 1 - offset;
Expand Down
52 changes: 27 additions & 25 deletions packages/core/schematics/test/control_flow_migration_spec.ts
Expand Up @@ -620,9 +620,9 @@ describe('control flow migration', () => {
].join('\n'));
});

fit('should migrate an NgIfElse case with ng-templates with multiple i18n attributes',
async () => {
writeFile('/comp.ts', `
it('should migrate an NgIfElse case with ng-templates with multiple i18n attributes',
async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgIf} from '@angular/common';
Expand All @@ -634,28 +634,30 @@ describe('control flow migration', () => {
}
`);

writeFile('/comp.html', [
`<ng-template *ngIf="false; else barTempl" i18n="@@foo">`,
` Foo`,
`</ng-template>`,
`<ng-template i18n="@@bar" #barTempl> Bar </ng-template>`,
`<a *ngIf="true" i18n="@@bam">Bam</a>`,
].join('\n'));

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

expect(content).toBe([
`@if (false) {`,
` <ng-container i18n="@@foo">Foo</ng-container>`,
`} @else {`,
` <ng-container i18n="@@bar"> Bar </ng-container>`,
`}`,
`@if (true) {`,
` <a i18n="@@bam">Bam</a>`,
`}`,
].join('\n'));
});
writeFile('/comp.html', [
`<ng-template *ngIf="false; else barTempl" i18n="@@foo">`,
` Foo`,
`</ng-template>`,
`<ng-template i18n="@@bar" #barTempl> Bar </ng-template>`,
`<a *ngIf="true" i18n="@@bam">Bam</a>`,
].join('\n'));

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

expect(content).toBe([
`@if (false) {`,
` <ng-container i18n="@@foo">`,
` Foo`,
` </ng-container>`,
`} @else {`,
` <ng-container i18n="@@bar"> Bar </ng-container>`,
`}`,
`@if (true) {`,
` <a i18n="@@bam">Bam</a>`,
`}`,
].join('\n'));
});

it('should migrate an if else case', async () => {
writeFile('/comp.ts', `
Expand Down

0 comments on commit b3a21c3

Please sign in to comment.