From 5564d020cdcea8273b65cf69c45c3f935195af66 Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Fri, 17 Nov 2023 11:54:20 -0500 Subject: [PATCH] fix(migrations): Fixes control flow migration if then else case (#53006) 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 --- .../ng-generate/control-flow-migration/ifs.ts | 4 +- .../control-flow-migration/util.ts | 11 ++-- .../test/control_flow_migration_spec.ts | 53 +++++++++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts index faa917dc8d1ad..34527b4a9b546 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts @@ -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}}; diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts index 35297ce780796..e925b8e0752b2 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts @@ -336,10 +336,12 @@ 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); @@ -347,13 +349,14 @@ export function getOriginals( 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 { diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index 6a67ad659141c..00107f3b9aca8 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -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', [ + ``, + ` `, + ` `, + ``, + ` `, + `
`, + ` `, + `
`, + ``, + ` Empty`, + ``, + ``, + ``, + `

Loading

`, + `
`, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + ``, + ` @if (loading) {`, + `

Loading

`, + ` } @else {`, + ` @if (selected) {`, + `
`, + ` } @else {`, + ` Empty`, + ` }`, + ` }`, + `
`, + ].join('\n')); + }); + it('should migrate but not remove ng-templates when referenced elsewhere', async () => { writeFile('/comp.ts', ` import {Component} from '@angular/core';