diff --git a/packages/core/schematics/ng-generate/control-flow-migration/fors.ts b/packages/core/schematics/ng-generate/control-flow-migration/fors.ts index 30d579957390..cc0ad8b40039 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/fors.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/fors.ts @@ -173,10 +173,11 @@ function migrateStandardNgFor(etm: ElementToMigrate, tmpl: string, offset: numbe function migrateBoundNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Result { const forAttrs = etm.forAttrs!; - const aliasMap = forAttrs.aliases; + const aliasAttrs = etm.aliasAttrs!; + const aliasMap = aliasAttrs.aliases; const originals = getOriginals(etm, tmpl, offset); - const condition = `${forAttrs.item} of ${forAttrs.forOf}`; + const condition = `${aliasAttrs.item} of ${forAttrs.forOf}`; const aliases = []; let aliasedIndex = '$index'; @@ -188,10 +189,10 @@ function migrateBoundNgFor(etm: ElementToMigrate, tmpl: string, offset: number): } const aliasStr = (aliases.length > 0) ? `;${aliases.join(';')}` : ''; - let trackBy = forAttrs.item; + let trackBy = aliasAttrs.item; if (forAttrs.trackBy !== '') { // build trackby value - trackBy = `${forAttrs.trackBy.trim()}(${aliasedIndex}, ${forAttrs.item})`; + trackBy = `${forAttrs.trackBy.trim()}(${aliasedIndex}, ${aliasAttrs.item})`; } const {start, middle, end} = getMainBlock(etm, tmpl, offset); 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 e85559d40c08..49b2c528ad1b 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts @@ -85,12 +85,22 @@ function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Resul } function buildIfBlock(etm: ElementToMigrate, tmpl: string, offset: number): Result { + const aliasAttrs = etm.aliasAttrs!; + const aliases = [...aliasAttrs.aliases.keys()]; + // includes the mandatory semicolon before as const lbString = etm.hasLineBreaks ? '\n' : ''; - const condition = etm.attr.value - .replace(' as ', '; as ') - // replace 'let' with 'as' whatever spaces are between ; and 'let' - .replace(/;\s*let/g, '; as'); + let condition = etm.attr.value + .replace(' as ', '; as ') + // replace 'let' with 'as' whatever spaces are between ; and 'let' + .replace(/;\s*let/g, '; as'); + if (aliases.length > 1 || (aliases.length === 1 && condition.indexOf('; as') > -1)) { + // only 1 alias allowed + throw new Error( + 'Found more than one alias on your ngIf. Remove one of them and re-run the migration.'); + } else if (aliases.length === 1) { + condition += `; as ${aliases[0]}`; + } const originals = getOriginals(etm, tmpl, offset); @@ -121,8 +131,18 @@ function buildStandardIfElseBlock( } function buildBoundIfElseBlock(etm: ElementToMigrate, tmpl: string, offset: number): Result { + const aliasAttrs = etm.aliasAttrs!; + const aliases = [...aliasAttrs.aliases.keys()]; + // includes the mandatory semicolon before as - const condition = etm.attr.value.replace(' as ', '; as '); + let condition = etm.attr.value.replace(' as ', '; as '); + if (aliases.length > 1 || (aliases.length === 1 && condition.indexOf('; as') > -1)) { + // only 1 alias allowed + throw new Error( + 'Found more than one alias on your ngIf. Remove one of them and re-run the migration.'); + } else if (aliases.length === 1) { + condition += `; as ${aliases[0]}`; + } const elsePlaceholder = `#${etm.elseAttr!.value}|`; if (etm.thenAttr !== undefined) { const thenPlaceholder = `#${etm.thenAttr!.value}|`; diff --git a/packages/core/schematics/ng-generate/control-flow-migration/types.ts b/packages/core/schematics/ng-generate/control-flow-migration/types.ts index d22900d40a1b..ed236e2c0703 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/types.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/types.ts @@ -80,6 +80,9 @@ export type Result = { export interface ForAttributes { forOf: string; trackBy: string; +} + +export interface AliasAttributes { item: string; aliases: Map; } @@ -101,17 +104,20 @@ export class ElementToMigrate { elseAttr: Attribute|undefined; thenAttr: Attribute|undefined; forAttrs: ForAttributes|undefined; + aliasAttrs: AliasAttributes|undefined; nestCount = 0; hasLineBreaks = false; constructor( el: Element, attr: Attribute, elseAttr: Attribute|undefined = undefined, - thenAttr: Attribute|undefined = undefined, forAttrs: ForAttributes|undefined = undefined) { + thenAttr: Attribute|undefined = undefined, forAttrs: ForAttributes|undefined = undefined, + aliasAttrs: AliasAttributes|undefined = undefined) { this.el = el; this.attr = attr; this.elseAttr = elseAttr; this.thenAttr = thenAttr; this.forAttrs = forAttrs; + this.aliasAttrs = aliasAttrs; } getCondition(): string { @@ -317,7 +323,9 @@ export class ElementCollector extends RecursiveVisitor { const elseAttr = el.attrs.find(x => x.name === boundngifelse); const thenAttr = el.attrs.find(x => x.name === boundngifthenelse); const forAttrs = attr.name === nakedngfor ? this.getForAttrs(el) : undefined; - this.elements.push(new ElementToMigrate(el, attr, elseAttr, thenAttr, forAttrs)); + const aliasAttrs = this.getAliasAttrs(el); + this.elements.push( + new ElementToMigrate(el, attr, elseAttr, thenAttr, forAttrs, aliasAttrs)); } } } @@ -325,8 +333,6 @@ export class ElementCollector extends RecursiveVisitor { } private getForAttrs(el: Element): ForAttributes { - const aliases = new Map(); - let item = ''; let trackBy = ''; let forOf = ''; for (const attr of el.attrs) { @@ -336,6 +342,14 @@ export class ElementCollector extends RecursiveVisitor { if (attr.name === '[ngForOf]') { forOf = attr.value; } + } + return {forOf, trackBy}; + } + + private getAliasAttrs(el: Element): AliasAttributes { + const aliases = new Map(); + let item = ''; + for (const attr of el.attrs) { if (attr.name.startsWith('let-')) { if (attr.value === '') { // item @@ -346,7 +360,7 @@ export class ElementCollector extends RecursiveVisitor { } } } - return {forOf, trackBy, item, aliases}; + return {item, aliases}; } } 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 9027de82fb90..495eeb60aaa4 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts @@ -416,10 +416,24 @@ function isI18nTemplate(etm: ElementToMigrate, i18nAttr: Attribute|undefined): b } function isRemovableContainer(etm: ElementToMigrate): boolean { - return (etm.el.name === 'ng-container' || etm.el.name === 'ng-template') && - (etm.el.attrs.length === 1 || etm.forAttrs !== undefined || - (etm.el.attrs.length === 2 && etm.elseAttr !== undefined) || - (etm.el.attrs.length === 3 && etm.elseAttr !== undefined && etm.thenAttr !== undefined)); + let attrCount = countAttributes(etm); + const safeToRemove = etm.el.attrs.length === attrCount; + return (etm.el.name === 'ng-container' || etm.el.name === 'ng-template') && safeToRemove; +} + +function countAttributes(etm: ElementToMigrate): number { + let attrCount = 1; + if (etm.elseAttr !== undefined) { + attrCount++; + } + if (etm.thenAttr !== undefined) { + attrCount++; + } + attrCount += etm.aliasAttrs?.aliases.size ?? 0; + attrCount += etm.aliasAttrs?.item ? 1 : 0; + attrCount += etm.forAttrs?.trackBy ? 1 : 0; + attrCount += etm.forAttrs?.forOf ? 1 : 0; + return attrCount; } /** diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index 9051853233ed..6504db2fddbc 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -327,6 +327,35 @@ describe('control flow migration', () => { ].join('\n')); }); + it('should migrate an if case as a binding with let variables', 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', [ + ``, + ` {{ data }}`, + ``, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + `@if (data$ | async; as data) {`, + ` {{ data }}`, + `}`, + ].join('\n')); + }); + it('should migrate an if else case as bindings', async () => { writeFile('/comp.ts', ` import {Component} from '@angular/core';