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 00848dfc8df..4ad27fc9a80 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/fors.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/fors.ts @@ -12,6 +12,12 @@ import {ElementCollector, ElementToMigrate, MigrateError, Result} from './types' import {calculateNesting, getMainBlock, getOriginals, hasLineBreaks, parseTemplate, reduceNestingOffset} from './util'; export const ngfor = '*ngFor'; +export const nakedngfor = 'ngFor'; +const fors = [ + ngfor, + nakedngfor, +]; + export const commaSeparatedSyntax = new Map([ ['(', ')'], ['{', '}'], @@ -34,7 +40,7 @@ export function migrateFor(template: string): {migrated: string, errors: Migrate } let result = template; - const visitor = new ElementCollector([ngfor]); + const visitor = new ElementCollector(fors); visitAll(visitor, parsed.rootNodes); calculateNesting(visitor, hasLineBreaks(template)); @@ -65,8 +71,14 @@ export function migrateFor(template: string): {migrated: string, errors: Migrate return {migrated: result, errors}; } - function migrateNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Result { + if (etm.forAttrs !== undefined) { + return migrateBoundNgFor(etm, tmpl, offset); + } + return migrateStandardNgFor(etm, tmpl, offset); +} + +function migrateStandardNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Result { const aliasWithEqualRegexp = /=\s*(count|index|first|last|even|odd)/gm; const aliasWithAsRegexp = /(count|index|first|last|even|odd)\s+as/gm; const aliases = []; @@ -136,6 +148,43 @@ function migrateNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Resu return {tmpl: updatedTmpl, offsets: {pre, post}}; } +function migrateBoundNgFor(etm: ElementToMigrate, tmpl: string, offset: number): Result { + const forAttrs = etm.forAttrs!; + const aliasMap = forAttrs.aliases; + + const originals = getOriginals(etm, tmpl, offset); + const condition = `${forAttrs.item} of ${forAttrs.forOf}`; + + const aliases = []; + let aliasedIndex = '$index'; + for (const [key, val] of aliasMap) { + aliases.push(` let ${key.trim()} = $${val}`); + if (val.trim() === 'index') { + aliasedIndex = key; + } + } + const aliasStr = (aliases.length > 0) ? `;${aliases.join(';')}` : ''; + + let trackBy = forAttrs.item; + if (forAttrs.trackBy !== '') { + // build trackby value + trackBy = `${forAttrs.trackBy.trim()}(${aliasedIndex}, ${forAttrs.item})`; + } + + const {start, middle, end} = getMainBlock(etm, tmpl, offset); + const startBlock = `@for (${condition}; track ${trackBy}${aliasStr}) {\n ${start}`; + + const endBlock = `${end}\n}`; + const forBlock = startBlock + middle + endBlock; + + const updatedTmpl = tmpl.slice(0, etm.start(offset)) + forBlock + tmpl.slice(etm.end(offset)); + + const pre = originals.start.length - startBlock.length; + const post = originals.end.length - endBlock.length; + + return {tmpl: updatedTmpl, offsets: {pre, post}}; +} + function getNgForParts(expression: string): string[] { const parts: string[] = []; const commaSeparatedStack: string[] = []; 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 5bcdc0407ee..faa917dc8d1 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/ifs.ts @@ -15,7 +15,7 @@ export const ngif = '*ngIf'; export const boundngif = '[ngIf]'; export const nakedngif = 'ngIf'; -export const ifs = [ +const ifs = [ ngif, nakedngif, boundngif, diff --git a/packages/core/schematics/ng-generate/control-flow-migration/switches.ts b/packages/core/schematics/ng-generate/control-flow-migration/switches.ts index 50ba91948a9..9647189cdd0 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/switches.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/switches.ts @@ -18,7 +18,7 @@ export const nakedcase = 'ngSwitchCase'; export const switchdefault = '*ngSwitchDefault'; export const nakeddefault = 'ngSwitchDefault'; -export const switches = [ +const switches = [ ngswitch, boundcase, switchcase, 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 818416030cd..f4bfaaa41cb 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/types.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/types.ts @@ -12,6 +12,7 @@ import ts from 'typescript'; export const ngtemplate = 'ng-template'; export const boundngifelse = '[ngIfElse]'; export const boundngifthenelse = '[ngIfThenElse]'; +export const nakedngfor = 'ngFor'; function allFormsOf(selector: string): string[] { return [ @@ -30,6 +31,10 @@ const commonModuleDirectives = new Set([ ...allFormsOf('ngStyle'), ...allFormsOf('ngTemplateOutlet'), ...allFormsOf('ngComponentOutlet'), + '[NgForOf]', + '[NgForTrackBy]', + '[ngIfElse]', + '[ngIfThenElse]', ]); function pipeMatchRegExpFor(name: string): RegExp { @@ -72,6 +77,13 @@ export type Result = { offsets: Offsets, }; +export interface ForAttributes { + forOf: string; + trackBy: string; + item: string; + aliases: Map; +} + /** * Represents an error that happened during migration */ @@ -88,16 +100,18 @@ export class ElementToMigrate { attr: Attribute; elseAttr: Attribute|undefined; thenAttr: Attribute|undefined; + forAttrs: ForAttributes|undefined; nestCount = 0; hasLineBreaks = false; constructor( el: Element, attr: Attribute, elseAttr: Attribute|undefined = undefined, - thenAttr: Attribute|undefined = undefined) { + thenAttr: Attribute|undefined = undefined, forAttrs: ForAttributes|undefined = undefined) { this.el = el; this.attr = attr; this.elseAttr = elseAttr; this.thenAttr = thenAttr; + this.forAttrs = forAttrs; } getCondition(targetStr: string): string { @@ -236,12 +250,38 @@ export class ElementCollector extends RecursiveVisitor { if (this._attributes.includes(attr.name)) { const elseAttr = el.attrs.find(x => x.name === boundngifelse); const thenAttr = el.attrs.find(x => x.name === boundngifthenelse); - this.elements.push(new ElementToMigrate(el, attr, elseAttr, thenAttr)); + const forAttrs = attr.name === nakedngfor ? this.getForAttrs(el) : undefined; + this.elements.push(new ElementToMigrate(el, attr, elseAttr, thenAttr, forAttrs)); } } } super.visitElement(el, null); } + + private getForAttrs(el: Element): ForAttributes { + const aliases = new Map(); + let item = ''; + let trackBy = ''; + let forOf = ''; + for (const attr of el.attrs) { + if (attr.name === '[ngForTrackBy]') { + trackBy = attr.value; + } + if (attr.name === '[ngForOf]') { + forOf = attr.value; + } + if (attr.name.startsWith('let-')) { + if (attr.value === '') { + // item + item = attr.name.replace('let-', ''); + } else { + // alias + aliases.set(attr.name.replace('let-', ''), attr.value); + } + } + } + return {forOf, trackBy, item, aliases}; + } } /** Finds all elements with ngif structural directives. */ 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 e9fbe29be9f..a3379cc85bd 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts @@ -12,7 +12,10 @@ import ts from 'typescript'; import {AnalyzedFile, CommonCollector, ElementCollector, ElementToMigrate, Template, TemplateCollector} from './types'; -const importRemovals = ['NgIf', 'NgFor', 'NgSwitch', 'NgSwitchCase', 'NgSwitchDefault']; +const importRemovals = [ + 'NgIf', 'NgIfElse', 'NgIfThenElse', 'NgFor', 'NgForOf', 'NgForTrackBy', 'NgSwitch', + 'NgSwitchCase', 'NgSwitchDefault' +]; const importWithCommonRemovals = [...importRemovals, 'CommonModule']; /** @@ -353,26 +356,34 @@ export function getOriginals( return {start, end: ''}; } +function isI18nTemplate(etm: ElementToMigrate, i18nAttr: Attribute|undefined): boolean { + return etm.el.name === 'ng-template' && i18nAttr !== undefined && + (etm.el.attrs.length === 2 || (etm.el.attrs.length === 3 && etm.elseAttr !== undefined)); +} + +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)); +} + /** * builds the proper contents of what goes inside a given control flow block after migration */ export function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number): {start: string, middle: string, end: string} { const i18nAttr = etm.el.attrs.find(x => x.name === 'i18n'); - if ((etm.el.name === 'ng-container' || etm.el.name === 'ng-template') && - (etm.el.attrs.length === 1 || (etm.el.attrs.length === 2 && etm.elseAttr !== undefined) || - (etm.el.attrs.length === 3 && etm.elseAttr !== undefined && etm.thenAttr !== undefined))) { + if (isRemovableContainer(etm)) { // this is the case where we're migrating and there's no need to keep the ng-container const childStart = etm.el.children[0].sourceSpan.start.offset - offset; const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset; const middle = tmpl.slice(childStart, childEnd); return {start: '', middle, end: ''}; - } else if ( - etm.el.name === 'ng-template' && i18nAttr !== undefined && - (etm.el.attrs.length === 2 || (etm.el.attrs.length === 3 && etm.elseAttr !== undefined))) { + } else if (isI18nTemplate(etm, i18nAttr)) { const childStart = etm.el.children[0].sourceSpan.start.offset - offset; const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset; - const middle = wrapIntoI18nContainer(i18nAttr, tmpl.slice(childStart, childEnd)); + const middle = wrapIntoI18nContainer(i18nAttr!, tmpl.slice(childStart, childEnd)); return {start: '', middle, end: ''}; } diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index f7ad20a42e6..d5f172b45c0 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -1585,6 +1585,208 @@ describe('control flow migration', () => { }); }); + describe('ngForOf', () => { + it('should migrate a basic ngForOf', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgFor} from '@angular/common'; + interface Item { + id: number; + text: string; + } + + @Component({ + imports: [NgFor,NgForOf], + templateUrl: 'comp.html', + }) + class Comp { + items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}]; + } + `); + + writeFile('/comp.html', [ + ``, + ` `, + ` {{rowData}}`, + ` `, + ``, + ].join('\n')); + + await runMigration(); + const actual = tree.readContent('/comp.html'); + + const expected = [ + ``, + ` @for (rowData of things; track rowData) {\n `, + ` {{rowData}}\n `, + `}`, + ``, + ].join('\n'); + + expect(actual).toBe(expected); + }); + + it('should migrate ngForOf with an alias', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgFor} from '@angular/common'; + interface Item { + id: number; + text: string; + } + + @Component({ + imports: [NgFor,NgForOf], + templateUrl: 'comp.html', + }) + class Comp { + items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}]; + } + `); + + writeFile('/comp.html', [ + ``, + ` `, + ` {{rowIndex}}{{rowData}}`, + ` `, + ``, + ].join('\n')); + + await runMigration(); + const actual = tree.readContent('/comp.html'); + + const expected = [ + ``, + ` @for (rowData of things; track rowData; let rowIndex = $index) {\n `, + ` {{rowIndex}}{{rowData}}\n `, + `}`, + ``, + ].join('\n'); + + expect(actual).toBe(expected); + }); + + it('should migrate ngForOf with track by', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgFor} from '@angular/common'; + interface Item { + id: number; + text: string; + } + + @Component({ + imports: [NgFor,NgForOf], + templateUrl: 'comp.html', + }) + class Comp { + items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}]; + } + `); + + writeFile('/comp.html', [ + ``, + ` `, + ` {{rowData}}`, + ` `, + ``, + ].join('\n')); + + await runMigration(); + const actual = tree.readContent('/comp.html'); + + const expected = [ + ``, + ` @for (rowData of things; track trackMe($index, rowData)) {\n `, + ` {{rowData}}\n `, + `}`, + ``, + ].join('\n'); + + expect(actual).toBe(expected); + }); + + it('should migrate ngForOf with track by and alias', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgFor} from '@angular/common'; + interface Item { + id: number; + text: string; + } + + @Component({ + imports: [NgFor,NgForOf], + templateUrl: 'comp.html', + }) + class Comp { + items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}]; + } + `); + + writeFile('/comp.html', [ + ``, + ` `, + ` {{rowIndex}}{{rowData}}`, + ` `, + ``, + ].join('\n')); + + await runMigration(); + const actual = tree.readContent('/comp.html'); + + const expected = [ + ``, + ` @for (rowData of things; track trackMe(rowIndex, rowData); let rowIndex = $index) {\n `, + ` {{rowIndex}}{{rowData}}\n `, + `}`, + ``, + ].join('\n'); + + expect(actual).toBe(expected); + }); + + it('should migrate ngForOf with track by and multiple aliases', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgFor} from '@angular/common'; + interface Item { + id: number; + text: string; + } + + @Component({ + imports: [NgFor,NgForOf], + templateUrl: 'comp.html', + }) + class Comp { + items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}]; + } + `); + + writeFile('/comp.html', [ + ``, + ` `, + ` {{rowIndex}}{{rowData}}`, + ` `, + ``, + ].join('\n')); + + await runMigration(); + const actual = tree.readContent('/comp.html'); + + const expected = [ + ``, + ` @for (rowData of things; track trackMe(rowIndex, rowData); let rowIndex = $index; let rCount = $count) {\n `, + ` {{rowIndex}}{{rowData}}\n `, + `}`, + ``, + ].join('\n'); + + expect(actual).toBe(expected); + }); + }); + describe('ngSwitch', () => { it('should migrate an inline template', async () => { writeFile(