Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix(migrations): Update CF migration to skip templates with duplicate ng-template names (#53204)

Browse source 
This adds a message to the console and skips any templates that detect duplicate ng-template names in the same component.
fixes: #53169

PR Close #53204
This commit is contained in:
Jessica Janiuk 2023-11-27 11:48:18 -05:00 committed by Pawel Kozlowski
parent 53912fdf74
commit 8a52674faa
4 changed files with 72 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
packages/core/schematics/ng-generate/control-flow-migration/migration.ts
View file

@ -29,7 +29,11 @@ export function migrateTemplate(
const forResult = migrateFor(ifResult.migrated);
const switchResult = migrateSwitch(forResult.migrated);
const caseResult = migrateCase(switchResult.migrated);
migrated = processNgTemplates(caseResult.migrated);
const templateResult = processNgTemplates(caseResult.migrated);
if (templateResult.err !== undefined) {
return {migrated: template, errors: [{type: 'template', error: templateResult.err}]};
}
migrated = templateResult.migrated;
const changed =
ifResult.changed || forResult.changed || switchResult.changed || caseResult.changed;
if (format && changed) {

8
packages/core/schematics/ng-generate/control-flow-migration/types.ts
View file

@ -338,9 +338,13 @@ export class TemplateCollector extends RecursiveVisitor {
templateAttr = attr;
}
}
if (templateAttr !== null) {
this.elements.push(new ElementToMigrate(el, templateAttr));
if (templateAttr !== null && !this.templates.has(templateAttr.name)) {
this.templates.set(templateAttr.name, new Template(el, templateAttr.name, i18n));
this.elements.push(new ElementToMigrate(el, templateAttr));
} else if (templateAttr !== null) {
throw new Error(
`A duplicate ng-template name "${templateAttr.name}" was found. ` +
`The control flow migration requires unique ng-template names within a component.`);
}
}
super.visitElement(el, null);

58
packages/core/schematics/ng-generate/control-flow-migration/util.ts
View file

@ -309,40 +309,44 @@ function wrapIntoI18nContainer(i18nAttr: Attribute, content: string) {
/**
* Counts, replaces, and removes any necessary ng-templates post control flow migration
*/
export function processNgTemplates(template: string): string {
export function processNgTemplates(template: string): {migrated: string, err: Error|undefined} {
// count usage
const templates = getTemplates(template);
try {
const templates = getTemplates(template);
// swap placeholders and remove
for (const [name, t] of templates) {
const replaceRegex = new RegExp(`${name}\\|`, 'g');
const forRegex = new RegExp(`${name}\\#`, 'g');
const forMatches = [...template.matchAll(forRegex)];
const matches = [...forMatches, ...template.matchAll(replaceRegex)];
let safeToRemove = true;
if (matches.length > 0) {
if (t.i18n !== null) {
const container = wrapIntoI18nContainer(t.i18n, t.children);
template = template.replace(replaceRegex, container);
} else if (t.children.trim() === '' && t.isNgTemplateOutlet) {
template = template.replace(replaceRegex, t.generateTemplateOutlet());
} else if (forMatches.length > 0) {
if (t.count === 2) {
template = template.replace(forRegex, t.children);
// swap placeholders and remove
for (const [name, t] of templates) {
const replaceRegex = new RegExp(`${name}\\|`, 'g');
const forRegex = new RegExp(`${name}\\#`, 'g');
const forMatches = [...template.matchAll(forRegex)];
const matches = [...forMatches, ...template.matchAll(replaceRegex)];
let safeToRemove = true;
if (matches.length > 0) {
if (t.i18n !== null) {
const container = wrapIntoI18nContainer(t.i18n, t.children);
template = template.replace(replaceRegex, container);
} else if (t.children.trim() === '' && t.isNgTemplateOutlet) {
template = template.replace(replaceRegex, t.generateTemplateOutlet());
} else if (forMatches.length > 0) {
if (t.count === 2) {
template = template.replace(forRegex, t.children);
} else {
template = template.replace(forRegex, t.generateTemplateOutlet());
safeToRemove = false;
}
} else {
template = template.replace(forRegex, t.generateTemplateOutlet());
safeToRemove = false;
template = template.replace(replaceRegex, t.children);
}
// the +1 accounts for the t.count's counting of the original template
if (t.count === matches.length + 1 && safeToRemove) {
template = template.replace(t.contents, '');
}
} else {
template = template.replace(replaceRegex, t.children);
}
// the +1 accounts for the t.count's counting of the original template
if (t.count === matches.length + 1 && safeToRemove) {
template = template.replace(t.contents, '');
}
}
return {migrated: template, err: undefined};
} catch (err) {
return {migrated: template, err: err as Error};
}
return template;
}
/**

30
packages/core/schematics/test/control_flow_migration_spec.ts
View file

@ -3190,6 +3190,36 @@ describe('control flow migration', () => {
expect(warnOutput.join(' '))
.toContain('IMPORTANT! This migration is in developer preview. Use with caution.');
});
it('should log a migration error when duplicate ng-template names are detected', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgIf} from '@angular/common';
@Component({
imports: [NgIf],
templateUrl: './comp.html'
})
class Comp {
toggle = false;
}
`);
writeFile('./comp.html', [
`<div *ngIf="show; else elseTmpl">Content</div>`,
`<div *ngIf="hide; else elseTmpl">Content</div>`,
`<ng-template #elseTmpl>Else Content</ng-template>`,
`<ng-template #elseTmpl>Duplicate</ng-template>`,
].join('\n'));
await runMigration();
tree.readContent('/comp.ts');
expect(warnOutput.join(' '))
.toContain(
`A duplicate ng-template name "#elseTmpl" was found. ` +
`The control flow migration requires unique ng-template names within a component.`);
});
});
describe('template', () => {