Skip to content

Commit

Permalink
fix(migrations): Switches to multiple passes to fix several reported …
Browse files Browse the repository at this point in the history
…bugs

Rather than migrate all in one pass, this now migrates in a separate pass per control flow item plus one for templates at the end. This resolves issues with multiple control flow items on a single element as well as making sure ng-templates are fully migrated before being moved to new locations.

fixes: angular#52518, angular#52516, angular#52513
  • Loading branch information
thePunderWoman committed Nov 7, 2023
1 parent d01f371 commit e3de2e5
Show file tree
Hide file tree
Showing 4 changed files with 331 additions and 83 deletions.
Expand Up @@ -14,7 +14,7 @@ import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';

import {AnalyzedFile, MigrateError} from './types';
import {analyze, migrateTemplate} from './util';
import {analyze, migrateFor, migrateIf, migrateSwitch, processNgTemplates} from './util';

interface Options {
path: string;
Expand Down Expand Up @@ -84,7 +84,18 @@ function runControlFlowMigration(
for (const [start, end] of ranges) {
const template = content.slice(start, end);
const length = (end ?? content.length) - start;
const {migrated, errors} = migrateTemplate(template);

let ifResult = migrateIf(template);
const forResult = migrateFor(ifResult.migrated);
const switchResult = migrateSwitch(forResult.migrated);

const errors = [
...ifResult.errors,
...forResult.errors,
...switchResult.errors,
];

const migrated = processNgTemplates(switchResult.migrated);

if (migrated !== null) {
update.remove(start, length);
Expand Down
Expand Up @@ -18,6 +18,7 @@ export const switchcase = '*ngSwitchCase';
export const nakedcase = 'ngSwitchCase';
export const switchdefault = '*ngSwitchDefault';
export const nakeddefault = 'ngSwitchDefault';
export const ngtemplate = 'ng-template';
export const commaSeparatedSyntax = new Map([
['(', ')'],
['{', '}'],
Expand All @@ -28,11 +29,13 @@ export const stringPairs = new Map([
[`'`, `'`],
]);

const attributesToMigrate = [
export const ifs = [
ngif,
nakedngif,
boundngif,
ngfor,
];

export const switches = [
ngswitch,
boundcase,
switchcase,
Expand Down Expand Up @@ -111,7 +114,6 @@ export class Template {
count: number = 0;
contents: string = '';
children: string = '';
used: boolean = false;

constructor(el: Element) {
this.el = el;
Expand Down Expand Up @@ -160,20 +162,61 @@ export class AnalyzedFile {
}
}

/** Finds all elements with control flow structural directives. */
export class ElementCollector extends RecursiveVisitor {
/** Finds all elements with ngif structural directives. */
export class IfCollector extends RecursiveVisitor {
readonly elements: ElementToMigrate[] = [];
readonly templates: Map<string, Template> = new Map();

override visitElement(el: Element): void {
if (el.attrs.length > 0) {
for (const attr of el.attrs) {
if (attributesToMigrate.includes(attr.name)) {
if (ifs.includes(attr.name)) {
this.elements.push(new ElementToMigrate(el, attr));
}
}
}
if (el.name === 'ng-template') {
super.visitElement(el, null);
}
}

/** Finds all elements with ngif structural directives. */
export class ForCollector extends RecursiveVisitor {
readonly elements: ElementToMigrate[] = [];

override visitElement(el: Element): void {
if (el.attrs.length > 0) {
for (const attr of el.attrs) {
if (attr.name === ngfor) {
this.elements.push(new ElementToMigrate(el, attr));
}
}
}
super.visitElement(el, null);
}
}

/** Finds all elements with ngif structural directives. */
export class SwitchCollector extends RecursiveVisitor {
readonly elements: ElementToMigrate[] = [];

override visitElement(el: Element): void {
if (el.attrs.length > 0) {
for (const attr of el.attrs) {
if (switches.includes(attr.name)) {
this.elements.push(new ElementToMigrate(el, attr));
}
}
}
super.visitElement(el, null);
}
}

/** Finds all elements with ngif structural directives. */
export class TemplateCollector extends RecursiveVisitor {
readonly elements: ElementToMigrate[] = [];
readonly templates: Map<string, Template> = new Map();

override visitElement(el: Element): void {
if (el.name === ngtemplate) {
for (const attr of el.attrs) {
if (attr.name.startsWith('#')) {
this.elements.push(new ElementToMigrate(el, attr));
Expand Down

0 comments on commit e3de2e5

Please sign in to comment.