fix(migrations): fix off by one issue with template removal in CF migration (#53255)

thePunderWoman · pkozlowski-opensource · commit 6291c8db092c · 2023-11-29T17:33:18.000+01:00
When ng-templates are removed, an extra space was being added when it was unnecessary. This resulted in malformed html if there was no space afterwards. fixes: #53248 PR Close #53255
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/types.ts b/packages/core/schematics/ng-generate/control-flow-migration/types.ts
@@ -218,7 +218,7 @@ export class Template {
   }
 
   generateContents(tmpl: string) {
-    this.contents = tmpl.slice(this.el.sourceSpan.start.offset, this.el.sourceSpan.end.offset + 1);
+    this.contents = tmpl.slice(this.el.sourceSpan.start.offset, this.el.sourceSpan.end.offset);
     this.children = '';
     if (this.el.children.length > 0) {
       this.children = tmpl.slice(
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -3685,7 +3685,59 @@ describe('control flow migration', () => {
 
       expect(actual).toBe(expected);
     });
+
+    it('should remove a template with no overlap with following elements', 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 *ngIf="stuff">`,
+        `  <div>`,
+        `    <ul>`,
+        `      <li>`,
+        `        <span>`,
+        `          <ng-container *ngIf="things; else elseTmpl">`,
+        `            <p>Hmm</p>`,
+        `          </ng-container>`,
+        `          <ng-template #elseTmpl> 0 </ng-template></span>`,
+        `      </li>`,
+        `    </ul>`,
+        `  </div>`,
+        `</ng-container>`,
+      ].join('\n'));
+
+      await runMigration();
+      const content = tree.readContent('/comp.html');
+
+      expect(content).toBe([
+        `@if (stuff) {`,
+        `  <div>`,
+        `    <ul>`,
+        `      <li>`,
+        `        <span>`,
+        `          @if (things) {`,
+        `            <p>Hmm</p>`,
+        `          } @else {`,
+        `            0`,
+        `          }`,
+        `        </span>`,
+        `      </li>`,
+        `    </ul>`,
+        `  </div>`,
+        `}`,
+      ].join('\n'));
+    });
   });
+
   describe('formatting', () => {
     it('should reformat else if', async () => {
       writeFile('/comp.ts', `
@@ -4052,7 +4104,7 @@ describe('control flow migration', () => {
         `<span>Content here</span>`,
         `} @else {`,
         `Else Content`,
-        `}`,
+        `}\n`,
         `</div>`,
       ].join('\n'));
     });
@@ -4396,7 +4448,7 @@ describe('control flow migration', () => {
          await runMigration();
          const content = tree.readContent('/comp.ts');
          expect(content).toContain(
-             'template: `@if (isLoggedIn$ | async; as logIn) {\n  Log In\n} @else {\n  Log Out\n}`');
+             'template: `@if (isLoggedIn$ | async; as logIn) {\n  Log In\n} @else {\n  Log Out\n}\n`');
        });
   });
 });

Original file line number	Diff line number	Diff line change
`@@ -218,7 +218,7 @@ export class Template {`
`218`	`218`	`}`
`219`	`219`
`220`	`220`	`generateContents(tmpl: string) {`
`221`		`- this.contents = tmpl.slice(this.el.sourceSpan.start.offset, this.el.sourceSpan.end.offset + 1);`
	`221`	`+ this.contents = tmpl.slice(this.el.sourceSpan.start.offset, this.el.sourceSpan.end.offset);`
`222`	`222`	`this.children = '';`
`223`	`223`	`if (this.el.children.length > 0) {`
`224`	`224`	`this.children = tmpl.slice(`