Skip to content

Commit

Permalink
fix(migrations): Fixes control flow migration if then else case (#53006)
Browse files Browse the repository at this point in the history
With if then else use cases, we now properly account for the length
of the original element's contents when tracking new offsets.

fixes: #52927

PR Close #53006
  • Loading branch information
thePunderWoman authored and AndrewKushnir committed Nov 20, 2023
1 parent f84057d commit 5564d02
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 5 deletions.
Expand Up @@ -175,7 +175,9 @@ function buildIfThenElseBlock(

const updatedTmpl = tmplStart + ifThenElseBlock + tmplEnd;

const pre = originals.start.length - startBlock.length;
// We ignore the contents of the element on if then else.
// If there's anything there, we need to account for the length in the offset.
const pre = originals.start.length + originals.childLength - startBlock.length;
const post = originals.end.length - postBlock.length;

return {tmpl: updatedTmpl, offsets: {pre, post}};
Expand Down
Expand Up @@ -336,24 +336,27 @@ export function removeImports(
* retrieves the original block of text in the template for length comparison during migration
* processing
*/
export function getOriginals(
etm: ElementToMigrate, tmpl: string, offset: number): {start: string, end: string} {
export function getOriginals(etm: ElementToMigrate, tmpl: string, offset: number):
{start: string, end: string, childLength: number} {
// original opening block
if (etm.el.children.length > 0) {
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 start = tmpl.slice(
etm.el.sourceSpan.start.offset - offset,
etm.el.children[0].sourceSpan.start.offset - offset);
// original closing block
const end = tmpl.slice(
etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset,
etm.el.sourceSpan.end.offset - offset);
return {start, end};
const childLength = childEnd - childStart;
return {start, end, childLength};
}
// self closing or no children
const start =
tmpl.slice(etm.el.sourceSpan.start.offset - offset, etm.el.sourceSpan.end.offset - offset);
// original closing block
return {start, end: ''};
return {start, end: '', childLength: 0};
}

function isI18nTemplate(etm: ElementToMigrate, i18nAttr: Attribute|undefined): boolean {
Expand Down
53 changes: 53 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
Expand Up @@ -828,6 +828,59 @@ describe('control flow migration', () => {
].join('\n'));
});

it('should migrate a complex if then else case on ng-containers', 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-container>`,
` <ng-container`,
` *ngIf="loading; then loadingTmpl; else showTmpl"`,
` >`,
` </ng-container>`,
`<ng-template #showTmpl>`,
` <ng-container`,
` *ngIf="selected; else emptyTmpl"`,
` >`,
` <div></div>`,
` </ng-container>`,
`</ng-template>`,
`<ng-template #emptyTmpl>`,
` Empty`,
`</ng-template>`,
`</ng-container>`,
`<ng-template #loadingTmpl>`,
` <p>Loading</p>`,
`</ng-template>`,
].join('\n'));

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

expect(content).toBe([
`<ng-container>`,
` @if (loading) {`,
` <p>Loading</p>`,
` } @else {`,
` @if (selected) {`,
` <div></div>`,
` } @else {`,
` Empty`,
` }`,
` }`,
`</ng-container>`,
].join('\n'));
});

it('should migrate but not remove ng-templates when referenced elsewhere', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
Expand Down

0 comments on commit 5564d02

Please sign in to comment.