Skip to content

Commit 8bf7525

Browse files
thePunderWomanthePunderWoman
committed
fix(migrations): cf migration - preserve indentation on attribute strings (#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 #53625
1 parent b40bb22 commit 8bf7525

File tree

2 files changed

+110
-3
lines changed

2 files changed

+110
-3
lines changed

‎packages/core/schematics/ng-generate/control-flow-migration/util.ts‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,19 @@ export function formatTemplate(tmpl: string, templateType: string): string {
582582
// <div thing="stuff" [binding]="true"> || <div thing="stuff" [binding]="true"
583583
const openElRegex = /^\s*<([a-z0-9]+)(?![^>]*\/>)[^>]*>?/;
584584

585+
// regex for matching an attribute string that was left open at the endof a line
586+
// so we can ensure we have the proper indent
587+
// <div thing="aefaefwe
588+
const openAttrDoubleRegex = /="([^"]|\\")*$/;
589+
const openAttrSingleRegex = /='([^']|\\')*$/;
590+
591+
// regex for matching an attribute string that was closes on a separate line
592+
// from when it was opened.
593+
// <div thing="aefaefwe
594+
// i18n message is here">
595+
const closeAttrDoubleRegex = /^\s*([^><]|\\")*"/;
596+
const closeAttrSingleRegex = /^\s*([^><]|\\')*'/;
597+
585598
// regex for matching a self closing html element that has no />
586599
// <input type="button" [binding]="true">
587600
const selfClosingRegex = new RegExp(`^\\s*<(${selfClosingList}).+\\/?>`);
@@ -620,6 +633,8 @@ export function formatTemplate(tmpl: string, templateType: string): string {
620633
let i18nDepth = 0;
621634
let inMigratedBlock = false;
622635
let inI18nBlock = false;
636+
let inAttribute = false;
637+
let isDoubleQuotes = false;
623638
for (let [index, line] of lines.entries()) {
624639
depth +=
625640
[...line.matchAll(startMarkerRegex)].length - [...line.matchAll(endMarkerRegex)].length;
@@ -633,7 +648,7 @@ export function formatTemplate(tmpl: string, templateType: string): string {
633648
lineWasMigrated = true;
634649
}
635650
if ((line.trim() === '' && index !== 0 && index !== lines.length - 1) &&
636-
(inMigratedBlock || lineWasMigrated) && !inI18nBlock) {
651+
(inMigratedBlock || lineWasMigrated) && !inI18nBlock && !inAttribute) {
637652
// skip blank lines except if it's the first line or last line
638653
// this preserves leading and trailing spaces if they are already present
639654
continue;
@@ -655,10 +670,25 @@ export function formatTemplate(tmpl: string, templateType: string): string {
655670
indent = indent.slice(2);
656671
}
657672

658-
const newLine =
659-
inI18nBlock ? line : mindent + (line.trim() !== '' ? indent : '') + line.trim();
673+
// if a line ends in an unclosed attribute, we need to note that and close it later
674+
if (!inAttribute && openAttrDoubleRegex.test(line)) {
675+
inAttribute = true;
676+
isDoubleQuotes = true;
677+
} else if (!inAttribute && openAttrSingleRegex.test(line)) {
678+
inAttribute = true;
679+
isDoubleQuotes = false;
680+
}
681+
682+
const newLine = (inI18nBlock || inAttribute) ?
683+
line :
684+
mindent + (line.trim() !== '' ? indent : '') + line.trim();
660685
formatted.push(newLine);
661686

687+
if ((inAttribute && isDoubleQuotes && closeAttrDoubleRegex.test(line)) ||
688+
(inAttribute && !isDoubleQuotes && closeAttrSingleRegex.test(line))) {
689+
inAttribute = false;
690+
}
691+
662692
// this matches any self closing element that actually has a />
663693
if (closeMultiLineElRegex.test(line)) {
664694
// multi line self closing tag

‎packages/core/schematics/test/control_flow_migration_spec.ts‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4755,6 +4755,83 @@ describe('control flow migration', () => {
47554755

47564756
expect(actual).toBe(expected);
47574757
});
4758+
4759+
it('should indent multi-line attribute strings to the right place', async () => {
4760+
writeFile('/comp.ts', `
4761+
import {Component} from '@angular/core';
4762+
import {NgIf} from '@angular/common';
4763+
4764+
@Component({
4765+
templateUrl: './comp.html'
4766+
})
4767+
class Comp {
4768+
show = false;
4769+
}
4770+
`);
4771+
4772+
writeFile('/comp.html', [
4773+
`<div *ngIf="show">show</div>`,
4774+
`<span i18n-message="this is a multi-`,
4775+
` line attribute`,
4776+
` with cool things">`,
4777+
` Content here`,
4778+
`</span>`,
4779+
].join('\n'));
4780+
4781+
await runMigration();
4782+
const actual = tree.readContent('/comp.html');
4783+
const expected = [
4784+
`@if (show) {`,
4785+
` <div>show</div>`,
4786+
`}`,
4787+
`<span i18n-message="this is a multi-`,
4788+
` line attribute`,
4789+
` with cool things">`,
4790+
` Content here`,
4791+
`</span>`,
4792+
].join('\n');
4793+
4794+
expect(actual).toBe(expected);
4795+
});
4796+
4797+
it('should indent multi-line attribute strings as single quotes to the right place',
4798+
async () => {
4799+
writeFile('/comp.ts', `
4800+
import {Component} from '@angular/core';
4801+
import {NgIf} from '@angular/common';
4802+
4803+
@Component({
4804+
templateUrl: './comp.html'
4805+
})
4806+
class Comp {
4807+
show = false;
4808+
}
4809+
`);
4810+
4811+
writeFile('/comp.html', [
4812+
`<div *ngIf="show">show</div>`,
4813+
`<span i18n-message='this is a multi-`,
4814+
` line attribute`,
4815+
` with cool things'>`,
4816+
` Content here`,
4817+
`</span>`,
4818+
].join('\n'));
4819+
4820+
await runMigration();
4821+
const actual = tree.readContent('/comp.html');
4822+
const expected = [
4823+
`@if (show) {`,
4824+
` <div>show</div>`,
4825+
`}`,
4826+
`<span i18n-message='this is a multi-`,
4827+
` line attribute`,
4828+
` with cool things'>`,
4829+
` Content here`,
4830+
`</span>`,
4831+
].join('\n');
4832+
4833+
expect(actual).toBe(expected);
4834+
});
47584835
});
47594836

47604837
describe('imports', () => {

0 commit comments

Comments
 (0)