Skip to content

Commit

Permalink
fix(migrations): cf migration - preserve indentation on attribute str…
Browse files Browse the repository at this point in the history
…ings (angular#53625)

During formatting, attribute indentation is changed, and that can affect internationalized strings. This fix detects if an attribute value string is left open and skips formatting on those lines.

PR Close angular#53625
  • Loading branch information
thePunderWoman committed Dec 18, 2023
1 parent b40bb22 commit 8bf7525
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,19 @@ export function formatTemplate(tmpl: string, templateType: string): string {
// <div thing="stuff" [binding]="true"> || <div thing="stuff" [binding]="true"
const openElRegex = /^\s*<([a-z0-9]+)(?![^>]*\/>)[^>]*>?/;

// regex for matching an attribute string that was left open at the endof a line
// so we can ensure we have the proper indent
// <div thing="aefaefwe
const openAttrDoubleRegex = /="([^"]|\\")*$/;
const openAttrSingleRegex = /='([^']|\\')*$/;

// regex for matching an attribute string that was closes on a separate line
// from when it was opened.
// <div thing="aefaefwe
// i18n message is here">
const closeAttrDoubleRegex = /^\s*([^><]|\\")*"/;
const closeAttrSingleRegex = /^\s*([^><]|\\')*'/;

// regex for matching a self closing html element that has no />
// <input type="button" [binding]="true">
const selfClosingRegex = new RegExp(`^\\s*<(${selfClosingList}).+\\/?>`);
Expand Down Expand Up @@ -620,6 +633,8 @@ export function formatTemplate(tmpl: string, templateType: string): string {
let i18nDepth = 0;
let inMigratedBlock = false;
let inI18nBlock = false;
let inAttribute = false;
let isDoubleQuotes = false;
for (let [index, line] of lines.entries()) {
depth +=
[...line.matchAll(startMarkerRegex)].length - [...line.matchAll(endMarkerRegex)].length;
Expand All @@ -633,7 +648,7 @@ export function formatTemplate(tmpl: string, templateType: string): string {
lineWasMigrated = true;
}
if ((line.trim() === '' && index !== 0 && index !== lines.length - 1) &&
(inMigratedBlock || lineWasMigrated) && !inI18nBlock) {
(inMigratedBlock || lineWasMigrated) && !inI18nBlock && !inAttribute) {
// skip blank lines except if it's the first line or last line
// this preserves leading and trailing spaces if they are already present
continue;
Expand All @@ -655,10 +670,25 @@ export function formatTemplate(tmpl: string, templateType: string): string {
indent = indent.slice(2);
}

const newLine =
inI18nBlock ? line : mindent + (line.trim() !== '' ? indent : '') + line.trim();
// if a line ends in an unclosed attribute, we need to note that and close it later
if (!inAttribute && openAttrDoubleRegex.test(line)) {
inAttribute = true;
isDoubleQuotes = true;
} else if (!inAttribute && openAttrSingleRegex.test(line)) {
inAttribute = true;
isDoubleQuotes = false;
}

const newLine = (inI18nBlock || inAttribute) ?
line :
mindent + (line.trim() !== '' ? indent : '') + line.trim();
formatted.push(newLine);

if ((inAttribute && isDoubleQuotes && closeAttrDoubleRegex.test(line)) ||
(inAttribute && !isDoubleQuotes && closeAttrSingleRegex.test(line))) {
inAttribute = false;
}

// this matches any self closing element that actually has a />
if (closeMultiLineElRegex.test(line)) {
// multi line self closing tag
Expand Down
77 changes: 77 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4755,6 +4755,83 @@ describe('control flow migration', () => {

expect(actual).toBe(expected);
});

it('should indent multi-line attribute strings to the right place', 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', [
`<div *ngIf="show">show</div>`,
`<span i18n-message="this is a multi-`,
` line attribute`,
` with cool things">`,
` Content here`,
`</span>`,
].join('\n'));

await runMigration();
const actual = tree.readContent('/comp.html');
const expected = [
`@if (show) {`,
` <div>show</div>`,
`}`,
`<span i18n-message="this is a multi-`,
` line attribute`,
` with cool things">`,
` Content here`,
`</span>`,
].join('\n');

expect(actual).toBe(expected);
});

it('should indent multi-line attribute strings as single quotes to the right place',
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', [
`<div *ngIf="show">show</div>`,
`<span i18n-message='this is a multi-`,
` line attribute`,
` with cool things'>`,
` Content here`,
`</span>`,
].join('\n'));

await runMigration();
const actual = tree.readContent('/comp.html');
const expected = [
`@if (show) {`,
` <div>show</div>`,
`}`,
`<span i18n-message='this is a multi-`,
` line attribute`,
` with cool things'>`,
` Content here`,
`</span>`,
].join('\n');

expect(actual).toBe(expected);
});
});

describe('imports', () => {
Expand Down

0 comments on commit 8bf7525

Please sign in to comment.