fix(core): do not remove used ng-template nodes in control flow migration (#52186)

cexbrayat · pkozlowski-opensource · commit 99e7629159af · 2023-10-12T19:28:23.000+02:00
This fixes an issue where `ng-template` nodes were removed even when used in other places than control flow directives. Template to migrate: ```html <ng-template #blockUsedElsewhere><div>Block</div></ng-template> <ng-container *ngTemplateOutlet="blockUsedElsewhere"></ng-container> ``` Before: ```html <ng-container *ngTemplateOutlet="blockUsedElsewhere"></ng-container> ``` After: ```html <ng-template #blockUsedElsewhere><div>Block</div></ng-template> <ng-container *ngTemplateOutlet="blockUsedElsewhere"></ng-container> ``` PR Close #52186
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts
@@ -175,7 +175,7 @@ export function migrateTemplate(template: string): {migrated: string|null, error
   }
 
   for (const [_, t] of visitor.templates) {
-    if (t.count === 2) {
+    if (t.count < 2) {
       result = result.replace(t.contents, '');
     }
   }
@@ -282,6 +282,9 @@ function buildIfElseBlock(
   offset = offset + etm.preOffset(startBlock.length) +
       etm.postOffset(mainBlock.length + postBlock.length);
 
+  // decrease usage count of elseTmpl
+  elseTmpl.count--;
+
   return {tmpl: updatedTmpl, offset};
 }
 
@@ -306,6 +309,10 @@ function buildIfThenElseBlock(
 
   offset = offset + etm.preOffset(startBlock.length) + etm.postOffset(postBlock.length);
 
+  // decrease usage count of thenTmpl and elseTmpl
+  thenTmpl.count--;
+  elseTmpl.count--;
+
   return {tmpl: updatedTmpl, offset};
 }
 
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -336,6 +336,33 @@ describe('control flow migration', () => {
         `<ng-container *ngTemplateOutlet="elseBlock"></ng-container>`,
       ].join('\n'));
     });
+
+    it('should not remove ng-templates used by other directives', 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-template #blockUsedElsewhere><div>Block</div></ng-template>`,
+        `<ng-container *ngTemplateOutlet="blockUsedElsewhere"></ng-container>`,
+      ].join('\n'));
+
+      await runMigration();
+      const content = tree.readContent('/comp.html');
+
+      expect(content).toBe([
+        `<ng-template #blockUsedElsewhere><div>Block</div></ng-template>`,
+        `<ng-container *ngTemplateOutlet="blockUsedElsewhere"></ng-container>`,
+      ].join('\n'));
+    });
   });
 
   describe('ngFor', () => {

Original file line number	Diff line number	Diff line change
`@@ -175,7 +175,7 @@ export function migrateTemplate(template: string): {migrated: string\|null, error`
`175`	`175`	`}`
`176`	`176`
`177`	`177`	`for (const [_, t] of visitor.templates) {`
`178`		`- if (t.count === 2) {`
	`178`	`+ if (t.count < 2) {`
`179`	`179`	`result = result.replace(t.contents, '');`
`180`	`180`	`}`
`181`	`181`	`}`
`@@ -282,6 +282,9 @@ function buildIfElseBlock(`
`282`	`282`	`offset = offset + etm.preOffset(startBlock.length) +`
`283`	`283`	`etm.postOffset(mainBlock.length + postBlock.length);`
`284`	`284`
	`285`	`+ // decrease usage count of elseTmpl`
	`286`	`+ elseTmpl.count--;`
	`287`	`+`
`285`	`288`	`return {tmpl: updatedTmpl, offset};`
`286`	`289`	`}`
`287`	`290`
`@@ -306,6 +309,10 @@ function buildIfThenElseBlock(`
`306`	`309`
`307`	`310`	`offset = offset + etm.preOffset(startBlock.length) + etm.postOffset(postBlock.length);`
`308`	`311`
	`312`	`+ // decrease usage count of thenTmpl and elseTmpl`
	`313`	`+ thenTmpl.count--;`
	`314`	`+ elseTmpl.count--;`
	`315`	`+`
`309`	`316`	`return {tmpl: updatedTmpl, offset};`
`310`	`317`	`}`
`311`	`318`