Skip to content

Commit

Permalink
fix(core): do not remove used ng-template nodes in control flow migra…
Browse files Browse the repository at this point in the history
…tion (#52186)

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
  • Loading branch information
cexbrayat authored and pkozlowski-opensource committed Oct 12, 2023
1 parent 6f19117 commit 99e7629
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 1 deletion.
Expand Up @@ -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, '');
}
}
Expand Down Expand Up @@ -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};
}

Expand All @@ -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};
}

Expand Down
27 changes: 27 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 99e7629

Please sign in to comment.