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 21f9a27bbfc..d6904e71db8 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/types.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/types.ts @@ -9,18 +9,25 @@ import {Attribute, Element, RecursiveVisitor} from '@angular/compiler'; export const ngif = '*ngIf'; +export const boundngif = '[ngIf]'; +export const nakedngif = 'ngIf'; export const ngfor = '*ngFor'; export const ngswitch = '[ngSwitch]'; const attributesToMigrate = [ ngif, + nakedngif, + boundngif, ngfor, ngswitch, ]; const casesToMigrate = [ + '[ngSwitchCase]', '*ngSwitchCase', + 'ngSwitchCase', '*ngSwitchDefault', + 'ngSwitchDefault', ]; /** @@ -29,6 +36,16 @@ const casesToMigrate = [ */ type Range = [start: number, end?: number]; +export type Offsets = { + pre: number, + post: number, +}; + +export type Result = { + tmpl: string, + offsets: Offsets, +}; + /** * Represents an error that happened during migration */ @@ -44,6 +61,7 @@ export class ElementToMigrate { el: Element; attr: Attribute; nestCount = 0; + lineBreaks = false; constructor(el: Element, attr: Attribute) { this.el = el; @@ -65,32 +83,16 @@ export class ElementToMigrate { } start(offset: number): number { - return this.el.sourceSpan?.start.offset - this.nestCount - offset; + return this.el.sourceSpan?.start.offset - offset; } end(offset: number): number { - return this.el.sourceSpan?.end.offset - this.nestCount - offset; + return this.el.sourceSpan?.end.offset - offset; } length(): number { return this.el.sourceSpan?.end.offset - this.el.sourceSpan?.start.offset; } - - openLength(): number { - return this.el.children[0]?.sourceSpan.start.offset - this.el.sourceSpan?.start.offset; - } - - closeLength(): number { - return this.el.sourceSpan?.end.offset - this.el.children[0]?.sourceSpan.end.offset; - } - - preOffset(newOffset: number): number { - return newOffset - this.openLength() + 1; - } - - postOffset(newOffset: number): number { - return newOffset - this.closeLength(); - } } export class Template { @@ -98,6 +100,7 @@ export class Template { count: number = 0; contents: string = ''; children: string = ''; + used: boolean = false; constructor(el: Element) { this.el = el; 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 9353ae74e15..5a007d41257 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts @@ -10,7 +10,7 @@ import {HtmlParser, Node, ParseTreeResult, visitAll} from '@angular/compiler'; import {dirname, join} from 'path'; import ts from 'typescript'; -import {AnalyzedFile, CaseCollector, ElementCollector, ElementToMigrate, MigrateError, ngfor, ngif, ngswitch, Template} from './types'; +import {AnalyzedFile, boundngif, CaseCollector, ElementCollector, ElementToMigrate, MigrateError, nakedngif, ngfor, ngif, ngswitch, Result, Template} from './types'; /** * Analyzes a source file to find file that need to be migrated and the text ranges within them. @@ -86,6 +86,8 @@ function getNestedCount(etm: ElementToMigrate, aggregator: number[]) { } } +const lb = '\n'; + /** * Replaces structural directive control flow instances with block control flow equivalents. * Returns null if the migration failed (e.g. there was a syntax error). @@ -104,6 +106,7 @@ export function migrateTemplate(template: string): {migrated: string|null, error tokenizeExpansionForms: true, // Explicitly disable blocks so that their characters are treated as plain text. tokenizeBlocks: false, + preserveLineEndings: true, }); // Don't migrate invalid templates. @@ -119,6 +122,9 @@ export function migrateTemplate(template: string): {migrated: string|null, error } let result = template; + const lineBreaks = template.match(/\r|\n/g); + const hasLineBreaks = lineBreaks !== null; + const visitor = new ElementCollector(); visitAll(visitor, parsed.rootNodes); @@ -137,6 +143,9 @@ export function migrateTemplate(template: string): {migrated: string|null, error for (let i = 1; i < visitor.elements.length; i++) { let currEl = visitor.elements[i]; currEl.nestCount = getNestedCount(currEl, nestedQueue); + if (currEl.el.sourceSpan.end.offset !== nestedQueue[nestedQueue.length - 1]) { + nestedQueue.push(currEl.el.sourceSpan.end.offset); + } } // this tracks the character shift from different lengths of blocks from @@ -144,38 +153,48 @@ export function migrateTemplate(template: string): {migrated: string|null, error // migration. Each block calculates length differences and passes that offset // to the next migrating block to adjust character offsets properly. let offset = 0; + let nestLevel = -1; + let postOffsets: number[] = []; + let migrateResult: Result = {tmpl: result, offsets: {pre: 0, post: 0}}; for (const el of visitor.elements) { - // these are all migratable nodes + // applies the post offsets after closing + if (el.nestCount <= nestLevel) { + const count = nestLevel - el.nestCount; + // reduced nesting, add postoffset + for (let i = 0; i <= count; i++) { + offset += postOffsets.pop() ?? 0; + } + } - if (el.attr.name === ngif) { + // these are all migratable nodes + if (el.attr.name === ngif || el.attr.name === nakedngif || el.attr.name === boundngif) { try { - let ifResult = migrateNgIf(el, visitor.templates, result, offset); - result = ifResult.tmpl; - offset = ifResult.offset; + migrateResult = migrateNgIf(el, visitor.templates, result, offset, hasLineBreaks); } catch (error: unknown) { - errors.push({type: ngfor, error}); + errors.push({type: ngif, error}); } } else if (el.attr.name === ngfor) { try { - let forResult = migrateNgFor(el, result, offset); - result = forResult.tmpl; - offset = forResult.offset; + migrateResult = migrateNgFor(el, result, offset, hasLineBreaks); } catch (error: unknown) { errors.push({type: ngfor, error}); } } else if (el.attr.name === ngswitch) { try { - let switchResult = migrateNgSwitch(el, result, offset); - result = switchResult.tmpl; - offset = switchResult.offset; + migrateResult = migrateNgSwitch(el, result, offset, hasLineBreaks); } catch (error: unknown) { - errors.push({type: ngfor, error}); + errors.push({type: ngswitch, error}); } } + result = migrateResult.tmpl; + offset += migrateResult.offsets.pre; + postOffsets.push(migrateResult.offsets.post); + const nm = el.el.name; + nestLevel = el.nestCount; } for (const [_, t] of visitor.templates) { - if (t.count < 2) { + if (t.count < 2 && t.used) { result = result.replace(t.contents, ''); } } @@ -183,14 +202,122 @@ export function migrateTemplate(template: string): {migrated: string|null, error return {migrated: result, errors}; } +function migrateNgIf( + etm: ElementToMigrate, ngTemplates: Map, tmpl: string, offset: number, + hasLineBreaks: boolean): Result { + const matchThen = etm.attr.value.match(/;\s+then/gm); + const matchElse = etm.attr.value.match(/;\s+else/gm); + + if (matchThen && matchThen.length > 0) { + return buildIfThenElseBlock( + etm, ngTemplates, tmpl, matchThen[0], matchElse![0], offset, hasLineBreaks); + } else if (matchElse && matchElse.length > 0) { + // just else + return buildIfElseBlock(etm, ngTemplates, tmpl, matchElse[0], offset, hasLineBreaks); + } + + return buildIfBlock(etm, tmpl, offset, hasLineBreaks); +} + +function buildIfBlock( + etm: ElementToMigrate, tmpl: string, offset: number, hasLineBreaks: boolean): Result { + // includes the mandatory semicolon before as + const lbString = hasLineBreaks ? lb : ''; + const condition = etm.attr.value.replace(' as ', '; as '); + + const originals = getOriginals(etm, tmpl, offset); + + const {start, middle, end} = getMainBlock(etm, tmpl, offset); + const startBlock = `@if (${condition}) {${lbString}${start}`; + const endBlock = `${end}${lbString}}`; + + const ifBlock = startBlock + middle + endBlock; + const updatedTmpl = tmpl.slice(0, etm.start(offset)) + ifBlock + tmpl.slice(etm.end(offset)); + + // this should be the difference between the starting element up to the start of the closing + // element and the mainblock sans } + const pre = originals.start.length - startBlock.length; + const post = originals.end.length - endBlock.length; + + return {tmpl: updatedTmpl, offsets: {pre, post}}; +} + +function buildIfElseBlock( + etm: ElementToMigrate, ngTemplates: Map, tmpl: string, elseString: string, + offset: number, hasLineBreaks: boolean): Result { + // includes the mandatory semicolon before as + const lbString = hasLineBreaks ? lb : ''; + const condition = etm.getCondition(elseString).replace(' as ', '; as '); + + const originals = getOriginals(etm, tmpl, offset); + + const elseTmpl = ngTemplates.get(`#${etm.getTemplateName(elseString)}`)!; + const {start, middle, end} = getMainBlock(etm, tmpl, offset); + const startBlock = `@if (${condition}) {${lbString}${start}`; + + const elseBlock = `${end}${lbString}} @else {${lbString}`; + const postBlock = elseBlock + elseTmpl.children + `${lbString}}`; + const ifElseBlock = startBlock + middle + postBlock; + + const tmplStart = tmpl.slice(0, etm.start(offset)); + const tmplEnd = tmpl.slice(etm.end(offset)); + const updatedTmpl = tmplStart + ifElseBlock + tmplEnd; + + // decrease usage count of elseTmpl + elseTmpl.count--; + elseTmpl.used = true; + + const pre = originals.start.length - startBlock.length; + const post = originals.end.length - postBlock.length; + + return {tmpl: updatedTmpl, offsets: {pre, post}}; +} + +function buildIfThenElseBlock( + etm: ElementToMigrate, ngTemplates: Map, tmpl: string, thenString: string, + elseString: string, offset: number, hasLineBreaks: boolean): Result { + const condition = etm.getCondition(thenString).replace(' as ', '; as '); + const lbString = hasLineBreaks ? lb : ''; + + const originals = getOriginals(etm, tmpl, offset); + + const startBlock = `@if (${condition}) {${lbString}`; + const elseBlock = `${lbString}} @else {${lbString}`; + + const thenTmpl = ngTemplates.get(`#${etm.getTemplateName(thenString, elseString)}`)!; + const elseTmpl = ngTemplates.get(`#${etm.getTemplateName(elseString)}`)!; + + const postBlock = thenTmpl.children + elseBlock + elseTmpl.children + `${lbString}}`; + const ifThenElseBlock = startBlock + postBlock; + + const tmplStart = tmpl.slice(0, etm.start(offset)); + const tmplEnd = tmpl.slice(etm.end(offset)); + + const updatedTmpl = tmplStart + ifThenElseBlock + tmplEnd; + + // decrease usage count of thenTmpl and elseTmpl + thenTmpl.count--; + thenTmpl.used = true; + elseTmpl.count--; + elseTmpl.used = true; + + const pre = originals.start.length - startBlock.length; + const post = originals.end.length - postBlock.length; + + return {tmpl: updatedTmpl, offsets: {pre, post}}; +} + function migrateNgFor( - etm: ElementToMigrate, tmpl: string, offset: number): {tmpl: string, offset: number} { + etm: ElementToMigrate, tmpl: string, offset: number, hasLineBreaks: boolean): Result { const aliasWithEqualRegexp = /=\s+(count|index|first|last|even|odd)/gm; const aliasWithAsRegexp = /(count|index|first|last|even|odd)\s+as/gm; const aliases = []; - + const lbString = hasLineBreaks ? lb : ''; + const lbSpaces = hasLineBreaks ? `${lb} ` : ''; const parts = etm.attr.value.split(';'); + const originals = getOriginals(etm, tmpl, offset); + // first portion should always be the loop definition prefixed with `let` const condition = parts[0].replace('let ', ''); const loopVar = condition.split(' of ')[0]; @@ -222,148 +349,97 @@ function migrateNgFor( const aliasStr = (aliases.length > 0) ? `;${aliases.join(';')}` : ''; - const startBlock = `@for (${condition}; track ${trackBy}${aliasStr}) {`; + const {start, middle, end} = getMainBlock(etm, tmpl, offset); + const startBlock = `@for (${condition}; track ${trackBy}${aliasStr}) {${lbSpaces}${start}`; - const mainBlock = getMainBlock(etm, tmpl, offset); - const forBlock = startBlock + mainBlock + '}'; + const endBlock = `${end}${lbString}}`; + const forBlock = startBlock + middle + endBlock; const updatedTmpl = tmpl.slice(0, etm.start(offset)) + forBlock + tmpl.slice(etm.end(offset)); - offset = offset + etm.length() - forBlock.length; + const pre = originals.start.length - startBlock.length; + const post = originals.end.length - endBlock.length; - return {tmpl: updatedTmpl, offset}; + return {tmpl: updatedTmpl, offsets: {pre, post}}; } -function migrateNgIf( - etm: ElementToMigrate, ngTemplates: Map, tmpl: string, - offset: number): {tmpl: string, offset: number} { - const matchThen = etm.attr.value.match(/;\s+then/gm); - const matchElse = etm.attr.value.match(/;\s+else/gm); +function getOriginals( + etm: ElementToMigrate, tmpl: string, offset: number): {start: string, end: string} { + // original opening block + if (etm.el.children.length > 0) { + const start = tmpl.slice( + etm.el.sourceSpan.start.offset - offset, + etm.el.children[0].sourceSpan.start.offset - offset); + // original closing block + const end = tmpl.slice( + etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset, + etm.el.sourceSpan.end.offset - offset); + return {start, end}; + } + // self closing or no children + const start = + tmpl.slice(etm.el.sourceSpan.start.offset - offset, etm.el.sourceSpan.end.offset - offset); + // original closing block + return {start, end: ''}; +} - if (matchThen && matchThen.length > 0) { - return buildIfThenElseBlock(etm, ngTemplates, tmpl, matchThen[0], matchElse![0], offset); - } else if (matchElse && matchElse.length > 0) { - // just else - return buildIfElseBlock(etm, ngTemplates, tmpl, matchElse[0], offset); +function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number): + {start: string, middle: string, end: string} { + if (etm.el.name === 'ng-container' && etm.el.attrs.length === 1) { + // 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: ''}; } - return buildIfBlock(etm, tmpl, offset); -} + const attrStart = etm.attr.keySpan!.start.offset - 1 - offset; + const valEnd = etm.attr.valueSpan!.end.offset + 1 - offset; + let childStart = valEnd; + let childEnd = valEnd; -function buildIfBlock( - etm: ElementToMigrate, tmpl: string, offset: number): {tmpl: string, offset: number} { - // includes the mandatory semicolon before as - const condition = etm.attr.value.replace(' as ', '; as '); - - const startBlock = `@if (${condition}) {`; - - const ifBlock = startBlock + getMainBlock(etm, tmpl, offset) + `}`; - const updatedTmpl = tmpl.slice(0, etm.start(offset)) + ifBlock + tmpl.slice(etm.end(offset)); - - offset = offset + etm.length() - ifBlock.length; - - return {tmpl: updatedTmpl, offset}; -} - -function buildIfElseBlock( - etm: ElementToMigrate, ngTemplates: Map, tmpl: string, elseString: string, - offset: number): {tmpl: string, offset: number} { - // includes the mandatory semicolon before as - const condition = etm.getCondition(elseString).replace(' as ', '; as '); - - const elseTmpl = ngTemplates.get(`#${etm.getTemplateName(elseString)}`)!; - const startBlock = `@if (${condition}) {`; - const mainBlock = getMainBlock(etm, tmpl, offset); - const elseBlock = `} @else {`; - const postBlock = elseBlock + elseTmpl.children + '}'; - const ifElseBlock = startBlock + mainBlock + postBlock; - - let tmplStart = tmpl.slice(0, etm.start(offset)); - let tmplEnd = tmpl.slice(etm.end(offset)); - const updatedTmpl = tmplStart + ifElseBlock + tmplEnd; - - offset = offset + etm.preOffset(startBlock.length) + - etm.postOffset(mainBlock.length + postBlock.length); - - // decrease usage count of elseTmpl - elseTmpl.count--; - - return {tmpl: updatedTmpl, offset}; -} - -function buildIfThenElseBlock( - etm: ElementToMigrate, ngTemplates: Map, tmpl: string, thenString: string, - elseString: string, offset: number): {tmpl: string, offset: number} { - const condition = etm.getCondition(thenString).replace(' as ', '; as '); - - const startBlock = `@if (${condition}) {`; - const elseBlock = `} @else {`; - - const thenTmpl = ngTemplates.get(`#${etm.getTemplateName(thenString, elseString)}`)!; - const elseTmpl = ngTemplates.get(`#${etm.getTemplateName(elseString)}`)!; - - const postBlock = thenTmpl.children + elseBlock + elseTmpl.children + '}'; - const ifThenElseBlock = startBlock + postBlock; - - let tmplStart = tmpl.slice(0, etm.start(offset)); - let tmplEnd = tmpl.slice(etm.end(offset)); - - const updatedTmpl = tmplStart + ifThenElseBlock + tmplEnd; - - 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}; -} - -function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number) { - if (etm.el.name === 'ng-container' && etm.el.attrs.length === 1 && etm.attr.name === ngfor) { - // this is the case where we're migrating an ngFor and there's no need to keep the ng-container - const childStart = etm.el.children[0].sourceSpan.start.offset - etm.nestCount - offset; - const childEnd = - etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - etm.nestCount - offset; - return tmpl.slice(childStart, childEnd); + if (etm.el.children.length > 0) { + childStart = etm.el.children[0].sourceSpan.start.offset - offset; + childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset; } - const attrStart = etm.attr.keySpan!.start.offset - 1 - etm.nestCount - offset; - const valEnd = etm.attr.valueSpan!.end.offset + 1 - etm.nestCount - offset; - const start = tmpl.slice(etm.start(offset), attrStart); - const end = tmpl.slice(valEnd, etm.end(offset)); - return start + end; + + let start = tmpl.slice(etm.start(offset), attrStart); + start += tmpl.slice(valEnd, childStart); + const middle = tmpl.slice(childStart, childEnd); + const end = tmpl.slice(childEnd, etm.end(offset)); + + return {start, middle, end}; } function migrateNgSwitch( - etm: ElementToMigrate, tmpl: string, offset: number): {tmpl: string, offset: number} { + etm: ElementToMigrate, tmpl: string, offset: number, hasLineBreaks: boolean): Result { const condition = etm.attr.value; - const startBlock = `@switch (${condition}) { `; + const startBlock = `@switch (${condition}) {`; + const lbString = hasLineBreaks ? lb : ''; const {openTag, closeTag, children} = getSwitchBlockElements(etm, tmpl, offset); - const cases = getSwitchCases(children, tmpl, etm.nestCount, offset); - const switchBlock = openTag + startBlock + cases.join(' ') + `}` + closeTag; + const cases = getSwitchCases(children, tmpl, offset, hasLineBreaks); + const switchBlock = openTag + startBlock + cases.join('') + `${lbString}}` + closeTag; const updatedTmpl = tmpl.slice(0, etm.start(offset)) + switchBlock + tmpl.slice(etm.end(offset)); + const pre = etm.length() - switchBlock.length; - const difference = etm.length() - switchBlock.length; - - offset = offset + difference; - - return {tmpl: updatedTmpl, offset}; + return {tmpl: updatedTmpl, offsets: {pre, post: 0}}; } function getSwitchBlockElements(etm: ElementToMigrate, tmpl: string, offset: number) { - const attrStart = etm.attr.keySpan!.start.offset - 1 - etm.nestCount - offset; - const valEnd = etm.attr.valueSpan!.end.offset + 1 - etm.nestCount - offset; - const childStart = etm.el.children[0].sourceSpan.start.offset - etm.nestCount - offset; - const childEnd = - etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - etm.nestCount - offset; - let openTag = tmpl.slice(etm.start(offset), attrStart) + tmpl.slice(valEnd, childStart); - if (tmpl.slice(childStart, childStart + 1) === '\n') { - openTag += '\n'; + const attrStart = etm.attr.keySpan!.start.offset - 1 - offset; + const valEnd = etm.attr.valueSpan!.end.offset + 1 - offset; + const childStart = etm.el.children[0].sourceSpan.start.offset - offset; + const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset; + let openTag = (etm.el.name === 'ng-container') ? + '' : + tmpl.slice(etm.start(offset), attrStart) + tmpl.slice(valEnd, childStart); + if (tmpl.slice(childStart, childStart + 1) === lb) { + openTag += lb; } - let closeTag = tmpl.slice(childEnd, etm.end(offset)); - if (tmpl.slice(childEnd - 1, childEnd) === '\n') { - closeTag = '\n' + closeTag; + let closeTag = (etm.el.name === 'ng-container') ? '' : tmpl.slice(childEnd, etm.end(offset)); + if (tmpl.slice(childEnd - 1, childEnd) === lb) { + closeTag = lb + closeTag; } return { openTag, @@ -372,25 +448,38 @@ function getSwitchBlockElements(etm: ElementToMigrate, tmpl: string, offset: num }; } -function getSwitchCases(children: Node[], tmpl: string, nestCount: number, offset: number) { +function getSwitchCases(children: Node[], tmpl: string, offset: number, hasLineBreaks: boolean) { const collector = new CaseCollector(); visitAll(collector, children); - return collector.elements.map(etm => getSwitchCaseBlock(etm, tmpl, nestCount, offset)); + return collector.elements.map(etm => getSwitchCaseBlock(etm, tmpl, offset, hasLineBreaks)); } function getSwitchCaseBlock( - etm: ElementToMigrate, tmpl: string, nestCount: number, offset: number): string { - const elStart = etm.el.sourceSpan?.start.offset - nestCount - offset; - const elEnd = etm.el.sourceSpan?.end.offset - nestCount - offset; - // beginning of the ngIf minus a leading space - const attrStart = etm.attr.keySpan!.start.offset - 1 - nestCount - offset; + etm: ElementToMigrate, tmpl: string, offset: number, hasLineBreaks: boolean): string { + let elStart = etm.el.sourceSpan?.start.offset - offset; + let elEnd = etm.el.sourceSpan?.end.offset - offset; + const lbString = hasLineBreaks ? '\n ' : ' '; + const lbSpaces = hasLineBreaks ? ' ' : ''; + let shift = 0; + + if ((etm.el.name === 'ng-container' || etm.el.name === 'ng-template') && + etm.el.attrs.length === 1) { + // no need to keep the containers + elStart = etm.el.children[0].sourceSpan.start.offset - offset; + elEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset; + // account for the `>` that isn't needed + shift += 1; + } + + const attrStart = etm.attr.keySpan!.start.offset - 1 - offset + shift; // ngSwitchDefault case has no valueSpan and relies on the end of the key - const attrEnd = etm.attr.keySpan!.end.offset - nestCount - offset; - if (etm.attr.name === '*ngSwitchDefault') { - return `@default { ${tmpl.slice(elStart, attrStart) + tmpl.slice(attrEnd, elEnd)} }`; + if (etm.attr.name === '*ngSwitchDefault' || etm.attr.name === 'ngSwitchDefault') { + const attrEnd = etm.attr.keySpan!.end.offset - offset + shift; + return `${lbString}@default {${lbString}${lbSpaces}${ + tmpl.slice(elStart, attrStart) + tmpl.slice(attrEnd, elEnd)}${lbString}}`; } // ngSwitchCase has a valueSpan - const valEnd = etm.attr.valueSpan!.end.offset + 1 - nestCount - offset; - return `@case (${etm.attr.value}) { ${ - tmpl.slice(elStart, attrStart) + tmpl.slice(valEnd, elEnd)} }`; + let valEnd = etm.attr.valueSpan!.end.offset + 1 - offset + shift; + return `${lbString}@case (${etm.attr.value}) {${lbString}${lbSpaces}${ + tmpl.slice(elStart, attrStart) + tmpl.slice(valEnd, elEnd)}${lbString}}`; } diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index 4e42c396f0c..7133351b747 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -139,7 +139,9 @@ describe('control flow migration', () => { expect(content).toBe([ `
`, - `@if (show) {Content here}`, + `@if (show) {`, + `Content here`, + `}`, `
`, ].join('\n')); }); @@ -176,7 +178,102 @@ describe('control flow migration', () => { expect(content).toBe([ `
`, - `@if (show) {Content here}`, + `@if (show) {`, + `Content here`, + `}`, + `
`, + ].join('\n')); + }); + + it('should migrate an if case with no star', 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', [ + `
`, + `Content here`, + `
`, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + `
`, + `@if (show) {`, + `Content here`, + `}`, + `
`, + ].join('\n')); + }); + + it('should migrate an if case as a binding', 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', [ + `
`, + `Content here`, + `
`, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + `
`, + `@if (show) {`, + `Content here`, + `}`, + `
`, + ].join('\n')); + }); + + it('should migrate an if case on a container', 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', [ + `
`, + `Content here`, + `
`, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + `
`, + `@if (show) {`, + `Content here`, + `}`, `
`, ].join('\n')); }); @@ -206,7 +303,11 @@ describe('control flow migration', () => { expect(content).toBe([ `
`, - `@if (show) {Content here} @else {Else Content}`, + `@if (show) {`, + `Content here`, + `} @else {`, + `Else Content`, + `}`, `
`, ].join('\n')); }); @@ -236,7 +337,11 @@ describe('control flow migration', () => { expect(content).toBe([ `
`, - `@if (show) {Content here} @else {Else Content}`, + `@if (show) {`, + `Content here`, + `} @else {`, + `Else Content`, + `}`, `
`, ].join('\n')); }); @@ -267,7 +372,11 @@ describe('control flow migration', () => { expect(content).toBe([ `
`, - `@if (show) {
THEN Stuff
} @else {Else Content}`, + `@if (show) {`, + `
THEN Stuff
`, + `} @else {`, + `Else Content`, + `}`, `
`, ].join('\n')); }); @@ -298,7 +407,11 @@ describe('control flow migration', () => { expect(content).toBe([ `
`, - `@if (show) {
THEN Stuff
} @else {Else Content}`, + `@if (show) {`, + `
THEN Stuff
`, + `} @else {`, + `Else Content`, + `}`, `
`, ].join('\n')); }); @@ -330,7 +443,11 @@ describe('control flow migration', () => { expect(content).toBe([ `
`, - `@if (show) {
THEN Stuff
} @else {Else Content}`, + `@if (show) {`, + `
THEN Stuff
`, + `} @else {`, + `Else Content`, + `}`, `Else Content`, `
`, ``, @@ -377,14 +494,12 @@ describe('control flow migration', () => { } `); - writeFile( - '/comp.html', [`
{{ user.name }}
`].join('\n')); + writeFile('/comp.html', `
{{ user.name }}
`); await runMigration(); const content = tree.readContent('/comp.html'); - expect(content).toBe( - [`@if (user$ | async; as user) {
{{ user.name }}
}`].join('\n')); + expect(content).toBe(`@if (user$ | async; as user) {
{{ user.name }}
}`); }); it('should migrate if/else with alias', async () => { @@ -412,7 +527,11 @@ describe('control flow migration', () => { expect(content).toBe([ `
`, - `@if (user$ | async; as user) {
{{ user.name }}
} @else {No user}`, + `@if (user$ | async; as user) {`, + `
{{ user.name }}
`, + `} @else {`, + `No user`, + `}`, `
`, ].join('\n')); }); @@ -443,7 +562,11 @@ describe('control flow migration', () => { expect(content).toBe([ `
`, - `@if (user$ | async; as user) {User} @else {No user}`, + `@if (user$ | async; as user) {`, + `User`, + `} @else {`, + `No user`, + `}`, `
`, ].join('\n')); }); @@ -539,7 +662,9 @@ describe('control flow migration', () => { expect(content).toBe([ `
    `, - `@for (item of items; track item) {
  • {{item.text}}
    • }`, + `@for (item of items; track item) {`, + `
  • {{item.text}}
    • `, + `}`, `
`, ].join('\n')); }); @@ -588,7 +713,9 @@ describe('control flow migration', () => { expect(content).toBe([ `
    `, - `@for (item of items; track item) {
  • {{item.text}}
    • }`, + `@for (item of items; track item) {`, + `
  • {{item.text}}
    • `, + `}`, `
`, ].join('\n')); }); @@ -768,7 +895,7 @@ describe('control flow migration', () => { const content = tree.readContent('/comp.ts'); expect(content).toContain( - 'template: `
@switch (testOpts) { @case (1) {

Option 1

} @case (2) {

Option 2

}}
`'); + 'template: `
@switch (testOpts) { @case (1) {

Option 1

} @case (2) {

Option 2

}}
'); }); it('should migrate multiple inline templates in the same file', async () => { @@ -833,7 +960,17 @@ describe('control flow migration', () => { const content = tree.readContent('/comp.html'); expect(content).toBe([ `
`, - `@switch (testOpts) { @case (1) {

Option 1

} @case (2) {

Option 2

} @default {

Option 3

}}`, + `@switch (testOpts) {`, + ` @case (1) {`, + `

Option 1

`, + ` }`, + ` @case (2) {`, + `

Option 2

`, + ` }`, + ` @default {`, + `

Option 3

`, + ` }`, + `}`, `
`, ].join('\n')); }); @@ -876,13 +1013,158 @@ describe('control flow migration', () => { expect(content).toBe([ `
`, - `@switch (testOpts) { @case (1) {

Option 1

} @case (2) {

Option 2

} @default {

Option 3

}}`, + `@switch (testOpts) {`, + ` @case (1) {`, + `

Option 1

`, + ` }`, + ` @case (2) {`, + `

Option 2

`, + ` }`, + ` @default {`, + `

Option 3

`, + ` }`, + `}`, `
`, ].join('\n')); }); + + it('should remove unnecessary ng-containers', async () => { + writeFile( + '/comp.ts', + ` + import {Component} from '@angular/core'; + import {ngSwitch, ngSwitchCase} from '@angular/common'; + + @Component({ + template: \`
` + + `

Option 1

` + + `

Option 2

` + + `
\` + }) + class Comp { + testOpts = "1"; + } + `); + + await runMigration(); + const content = tree.readContent('/comp.ts'); + + expect(content).toContain( + 'template: `
@switch (testOpts) { @case (1) {

Option 1

} @case (2) {

Option 2

}}
'); + }); + + it('should remove unnecessary ng-container on ngswitch', async () => { + writeFile( + '/comp.ts', + ` + import {Component} from '@angular/core'; + import {ngSwitch, ngSwitchCase} from '@angular/common'; + + @Component({ + template: \`
` + + `` + + `

Option 1

` + + `

Option 2

` + + `` + + `
\` + }) + class Comp { + testOpts = "1"; + } + `); + + await runMigration(); + const content = tree.readContent('/comp.ts'); + + expect(content).toContain( + 'template: `
@switch (testOpts) { @case (1) {

Option 1

} @case (2) {

Option 2

}}
'); + }); + + it('should handle cases with missing star', async () => { + writeFile( + '/comp.ts', + ` + import {Component} from '@angular/core'; + import {ngSwitch, ngSwitchCase} from '@angular/common'; + + @Component({ + template: \`
` + + `

Option 1

` + + `

Option 2

` + + `

Option 3

` + + `
\` + }) + class Comp { + testOpts = "1"; + } + `); + + await runMigration(); + const content = tree.readContent('/comp.ts'); + + expect(content).toContain( + 'template: `
@switch (testOpts) { @case (1) {

Option 1

} @case (2) {

Option 2

} @default {

Option 3

}}
'); + }); + + it('should handle cases with binding', async () => { + writeFile( + '/comp.ts', + ` + import {Component} from '@angular/core'; + import {ngSwitch, ngSwitchCase} from '@angular/common'; + + @Component({ + template: \`
` + + `

Option 1

` + + `

Option 2

` + + `

Option 3

` + + `
\` + }) + class Comp { + testOpts = "1"; + } + `); + + await runMigration(); + const content = tree.readContent('/comp.ts'); + + expect(content).toContain( + 'template: `
@switch (testOpts) { @case (1) {

Option 1

} @case (2) {

Option 2

} @default {

Option 3

}}
'); + }); }); describe('nested structures', () => { + it('should migrate an inline template with nested control flow structures and no line breaks', + async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf} from '@angular/common'; + + @Component({ + imports: [NgFor, NgIf], + templateUrl: './comp.html' + }) + class Comp { + show = false; + nest = true; + again = true; + more = true; + } + `); + + writeFile( + '/comp.html', + `
thingsstuff
stuff
`, + ); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe( + `@if (show) {
@if (nest) {
@if (again) {things}@if (more) {stuff}
}@if (more) {stuff}
}`, + ); + }); + it('should migrate an inline template with multiple nested control flow structures', async () => { writeFile('/comp.ts', ` @@ -903,11 +1185,10 @@ describe('control flow migration', () => { writeFile('/comp.html', [ `
`, + `things`, `
`, - `things`, - `stuff`, + `stuff`, `
`, - `stuff`, `
`, ].join('\n')); @@ -915,16 +1196,222 @@ describe('control flow migration', () => { const content = tree.readContent('/comp.html'); expect(content).toBe([ - `@if (show) {
`, - `@if (nest) {
`, - `@if (again) {things}`, - `@if (more) {stuff}`, - `
}`, - `@if (more) {stuff}`, - `
}`, + `@if (show) {`, + `
`, + `things`, + `@if (nest) {`, + `
`, + `stuff`, + `
`, + `}`, + `
`, + `}`, ].join('\n')); }); + it('should migrate an inline template with multiple nested control flow structures', + async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf} from '@angular/common'; + + @Component({ + imports: [NgFor, NgIf], + templateUrl: './comp.html' + }) + class Comp { + show = false; + nest = true; + again = true; + more = true; + } + `); + + writeFile('/comp.html', [ + `
`, + `things`, + `
`, + `stuff`, + `
`, + `
`, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + `@if (show) {`, + `
`, + `things`, + `@if (nest) {`, + `
`, + `stuff`, + `
`, + `}`, + `
`, + `}`, + ].join('\n')); + }); + + it('should migrate a simple nested case', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf} from '@angular/common'; + + @Component({ + imports: [NgFor, NgIf], + templateUrl: './comp.html' + }) + class Comp { + show = false; + nest = true; + again = true; + more = true; + } + `); + + writeFile('/comp.html', [ + `
`, + `
`, + `
`, + `
`, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + `@if (show) {`, + `
`, + `@if (nest) {`, + `
`, + `
`, + `}`, + `
`, + `}`, + ].join('\n')); + }); + + it('should migrate a simple for inside an if', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf} from '@angular/common'; + + @Component({ + imports: [NgFor, NgIf], + templateUrl: './comp.html' + }) + class Comp { + show = false; + nest = true; + again = true; + more = true; + } + `); + + writeFile('/comp.html', [ + `
    `, + `
  • {{h.name}} ({{h.emotion}})
    • `, + `
`, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + `@if (show) {`, + `
    `, + `@for (h of heroes; track h) {`, + `
  • {{h.name}} ({{h.emotion}})
    • `, + `}`, + `
`, + `}`, + ].join('\n')); + }); + + it('should migrate an if inside a for loop', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf} from '@angular/common'; + + @Component({ + imports: [NgFor, NgIf], + templateUrl: './comp.html' + }) + class Comp { + heroes = [{name: 'cheese', emotion: 'happy'},{name: 'stuff', emotion: 'sad'}]; + show = true; + } + `); + + writeFile('/comp.html', [ + ``, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + ``, + ].join('\n')); + }); + + it('should migrate an if inside a for loop with ng-containers', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf} from '@angular/common'; + + @Component({ + imports: [NgFor, NgIf], + templateUrl: './comp.html' + }) + class Comp { + heroes = [{name: 'cheese', emotion: 'happy'},{name: 'stuff', emotion: 'sad'}]; + show = true; + } + `); + + writeFile('/comp.html', [ + ``, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + + expect(content).toBe([ + ``, + ].join('\n')); + }); + it('should migrate an inline template with if and for loops', async () => { writeFile('/comp.ts', ` import {Component} from '@angular/core'; @@ -957,15 +1444,21 @@ describe('control flow migration', () => { const content = tree.readContent('/comp.html'); expect(content).toBe([ - `@if (show) {
`, - `@if (nest) {
    `, - `@for (item of items; track item) {
  • {{item.text}}
    • }`, - `
}`, - `
}`, + `@if (show) {`, + `
`, + `@if (nest) {`, + `
    `, + `@for (item of items; track item) {`, + `
  • {{item.text}}
    • `, + `}`, + `
`, + `}`, + `
`, + `}`, ].join('\n')); }); - it('should migrate an inline template with if, else and for loops', async () => { + it('should migrate template with if, else, and for loops', async () => { writeFile('/comp.ts', ` import {Component} from '@angular/core'; import {NgIf, NgFor} from '@angular/common'; @@ -998,52 +1491,19 @@ describe('control flow migration', () => { const content = tree.readContent('/comp.html'); expect(content).toBe([ - `@if (show) {
`, - `@if (nest) {
    `, - `@for (item of items; track item) {
  • {{item.text}}
    • }`, - `
} @else {

Else content

}`, - `
}`, - ].join('\n')); - }); - - it('should migrate an inline template with if, else, and for loops', async () => { - writeFile('/comp.ts', ` - import {Component} from '@angular/core'; - import {NgIf, NgFor} from '@angular/common'; - interface Item { - id: number; - text: string; - } - - @Component({ - imports: [NgFor, NgIf], - templateUrl: './comp.html' - }) - class Comp { - show = false; - nest = true; - items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}]; - } - `); - - writeFile('/comp.html', [ - `
`, - `
    `, - `
  • {{item.text}}
    • `, + `@if (show) {`, + `
    `, + `@if (nest) {`, + `
      `, + `@for (item of items; track item) {`, + `
    • {{item.text}}
      • `, + `}`, `
    `, - `

    Else content

    `, + `} @else {`, + `

    Else content

    `, + `}`, `
    `, - ].join('\n')); - - await runMigration(); - const content = tree.readContent('/comp.html'); - - expect(content).toBe([ - `@if (show) {
    `, - `@if (nest) {
      `, - `@for (item of items; track item) {
    • {{item.text}}
      • }`, - `
    } @else {

    Else content

    }`, - `
    }`, + `}`, ].join('\n')); }); @@ -1091,21 +1551,231 @@ describe('control flow migration', () => { const content = tree.readContent('/comp.html'); expect(content).toBe([ - `@if (show) {
    `, - `@if (again) {
    `, - `@if (more) {
    `, + `@if (show) {`, `
    `, - `@switch (testOpts) { @case (1) {

    Option 1

    } @case (2) {

    Option 2

    } @default {

    Option 3

    }}`, + `@if (again) {`, + `
    `, + `@if (more) {`, + `
    `, + `
    `, + `@switch (testOpts) {`, + ` @case (1) {`, + `

    Option 1

    `, + ` }`, + ` @case (2) {`, + `

    Option 2

    `, + ` }`, + ` @default {`, + `

    Option 3

    `, + ` }`, + `}`, `
    `, - `
    }`, - `
    }`, - `@if (nest) {
      `, - `@for (item of items; track item) {
    • {{item.text}}
      • }`, - `
    }`, - `
    }`, + `
    `, + `}`, + `
    `, + `}`, + `@if (nest) {`, + `
      `, + `@for (item of items; track item) {`, + `
    • {{item.text}}
      • `, + `}`, + `
    `, + `}`, + `
    `, + `}`, ].join('\n')); }); + + it('complicated case', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf, NgFor} from '@angular/common'; + interface Item { + id: number; + text: string; + } + + @Component({ + imports: [NgFor, NgIf], + templateUrl: './comp.html' + }) + class Comp { + } + `); + + writeFile('/comp.html', [ + `
    `, + ``, + ``, + `{{ breadcrumb.label }}`, + ``, + ``, + `{{ breadcrumb.label }}`, + ``, + ``, + ``, + `{{ breadcrumb.label }}`, + ``, + `
    `, + ].join('\n')); + + await runMigration(); + const content = tree.readContent('/comp.html'); + const result = [ + `@for (breadcrumb of breadcrumbItems(); track breadcrumb) {`, + `
    `, + `@if (breadcrumb.path) {\n`, + `@if (breadcrumb.isExternal) {\n`, + `{{ breadcrumb.label }}\n`, + `} @else {\n`, + `{{ breadcrumb.label }}\n`, + `}\n`, + `} @else {\n`, + `{{ breadcrumb.label }}\n`, + `}`, + `
    `, + `}`, + ].join('\n'); + + expect(content).toBe(result); + }); + + it('long file', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf, NgFor} from '@angular/common'; + interface Item { + id: number; + text: string; + } + + @Component({ + imports: [NgFor, NgIf], + templateUrl: './comp.html' + }) + class Comp { + } + `); + + writeFile('/comp.html', [ + `
    `, + ``, + ``, + ``, + ``, + `
    `, + ``, + ``, + `Case A`, + ``, + ``, + `Case B`, + ``, + ``, + `Case C`, + ``, + ``, + `Case D`, + ``, + ``, + `Case E`, + ``, + ``, + ``, + `
    `, + `
    `, + ].join('\n')); + + await runMigration(); + + const content = tree.readContent('/comp.html'); + const result = [ + `
    `, + `@if (stuff) {`, + ``, + `}`, + `@if (shouldDoIt) {\n`, + `\n`, + `}`, + `@if (`, + `shouldWhat`, + `) {`, + `
    \n`, + `@switch (currentCase()) {`, + ` @case (A) {`, + ` `, + `Case A`, + ``, + ` }`, + ` @case (B) {`, + ` `, + `Case B`, + ``, + ` }`, + ` @case (C) {`, + ` `, + `Case C`, + ``, + ` }`, + ` @case (D) {`, + ` `, + `Case D`, + ``, + ` }`, + ` @case (E) {`, + ` `, + `Case E`, + ``, + ` }`, + `}\n`, + ``, + `
    `, + `}`, + `
    `, + ].join('\n'); + + expect(content).toBe(result); + }); }); + describe('error handling', () => { it('should log template migration errors to the console', async () => { writeFile('/comp.ts', ` @@ -1127,4 +1797,32 @@ describe('control flow migration', () => { expect(warnOutput.join(' ')).toContain('WARNING: 1 errors occured during your migration'); }); }); + + describe('template removal', () => { + it('should not remove a template thats not used in control flow', async () => { + writeFile('/comp.ts', ` + import {Component} from '@angular/core'; + import {NgIf} from '@angular/common'; + + @Component({ + selector: 'declare-comp', + template: \` + DeclareComp({{name}}) + + {{greeting}} {{logName()}}! + + \` + }) + class DeclareComp implements DoCheck, AfterViewChecked { + @ViewChild('myTmpl') myTmpl!: TemplateRef; + name: string = 'world'; + } + `); + + await runMigration(); + const content = tree.readContent('/comp.ts'); + + expect(content).toContain(''); + }); + }); });