Skip to content

Commit 36f7623

Browse files
armanio123armanio123
authored
Fix jsx formatting (#42671)
* Refactored scanJsxToken when is formatting * Added bug regression test * Simplify JsxText formatting * Renamed isFormatting to allowMultilineJsxText * Updated baselines
1 parent 1e9b21f commit 36f7623

File tree

8 files changed

+119
-56
lines changed

8 files changed

+119
-56
lines changed

‎src/compiler/scanner.ts

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ namespace ts {
4040
scanJsxIdentifier(): SyntaxKind;
4141
scanJsxAttributeValue(): SyntaxKind;
4242
reScanJsxAttributeValue(): SyntaxKind;
43-
reScanJsxToken(): JsxTokenSyntaxKind;
43+
reScanJsxToken(allowMultilineJsxText?: boolean): JsxTokenSyntaxKind;
4444
reScanLessThanToken(): SyntaxKind;
4545
reScanQuestionToken(): SyntaxKind;
4646
reScanInvalidIdentifier(): SyntaxKind;
@@ -2223,9 +2223,9 @@ namespace ts {
22232223
return token = scanTemplateAndSetTokenValue(/* isTaggedTemplate */ true);
22242224
}
22252225

2226-
function reScanJsxToken(): JsxTokenSyntaxKind {
2226+
function reScanJsxToken(allowMultilineJsxText = true): JsxTokenSyntaxKind {
22272227
pos = tokenPos = startPos;
2228-
return token = scanJsxToken();
2228+
return token = scanJsxToken(allowMultilineJsxText);
22292229
}
22302230

22312231
function reScanLessThanToken(): SyntaxKind {
@@ -2242,7 +2242,7 @@ namespace ts {
22422242
return token = SyntaxKind.QuestionToken;
22432243
}
22442244

2245-
function scanJsxToken(): JsxTokenSyntaxKind {
2245+
function scanJsxToken(allowMultilineJsxText = true): JsxTokenSyntaxKind {
22462246
startPos = tokenPos = pos;
22472247

22482248
if (pos >= end) {
@@ -2266,19 +2266,11 @@ namespace ts {
22662266

22672267
// First non-whitespace character on this line.
22682268
let firstNonWhitespace = 0;
2269-
let lastNonWhitespace = -1;
22702269

22712270
// These initial values are special because the first line is:
22722271
// firstNonWhitespace = 0 to indicate that we want leading whitespace,
22732272

22742273
while (pos < end) {
2275-
2276-
// We want to keep track of the last non-whitespace (but including
2277-
// newlines character for hitting the end of the JSX Text region)
2278-
if (!isWhiteSpaceSingleLine(char)) {
2279-
lastNonWhitespace = pos;
2280-
}
2281-
22822274
char = text.charCodeAt(pos);
22832275
if (char === CharacterCodes.openBrace) {
22842276
break;
@@ -2297,8 +2289,6 @@ namespace ts {
22972289
error(Diagnostics.Unexpected_token_Did_you_mean_or_rbrace, pos, 1);
22982290
}
22992291

2300-
if (lastNonWhitespace > 0) lastNonWhitespace++;
2301-
23022292
// FirstNonWhitespace is 0, then we only see whitespaces so far. If we see a linebreak, we want to ignore that whitespaces.
23032293
// i.e (- : whitespace)
23042294
// <div>----
@@ -2308,15 +2298,19 @@ namespace ts {
23082298
if (isLineBreak(char) && firstNonWhitespace === 0) {
23092299
firstNonWhitespace = -1;
23102300
}
2301+
else if (!allowMultilineJsxText && isLineBreak(char) && firstNonWhitespace > 0) {
2302+
// Stop JsxText on each line during formatting. This allows the formatter to
2303+
// indent each line correctly.
2304+
break;
2305+
}
23112306
else if (!isWhiteSpaceLike(char)) {
23122307
firstNonWhitespace = pos;
23132308
}
23142309

23152310
pos++;
23162311
}
23172312

2318-
const endPosition = lastNonWhitespace === -1 ? pos : lastNonWhitespace;
2319-
tokenValue = text.substring(startPos, endPosition);
2313+
tokenValue = text.substring(startPos, pos);
23202314

23212315
return firstNonWhitespace === -1 ? SyntaxKind.JsxTextAllWhiteSpaces : SyntaxKind.JsxText;
23222316
}

‎src/services/formatting/formatting.ts

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -659,11 +659,6 @@ namespace ts.formatting {
659659
if (tokenInfo.token.end > node.end) {
660660
break;
661661
}
662-
if (node.kind === SyntaxKind.JsxText) {
663-
// Intentation rules for jsx text are handled by `indentMultilineCommentOrJsxText` inside `processChildNode`; just fastforward past it here
664-
formattingScanner.advance();
665-
continue;
666-
}
667662
consumeTokenAndAdvanceScanner(tokenInfo, node, nodeDynamicIndentation, node);
668663
}
669664

@@ -756,22 +751,6 @@ namespace ts.formatting {
756751
const childIndentation = computeIndentation(child, childStartLine, childIndentationAmount, node, parentDynamicIndentation, effectiveParentStartLine);
757752

758753
processNode(child, childContextNode, childStartLine, undecoratedChildStartLine, childIndentation.indentation, childIndentation.delta);
759-
if (child.kind === SyntaxKind.JsxText) {
760-
const range: TextRange = { pos: child.getStart(), end: child.getEnd() };
761-
if (range.pos !== range.end) { // don't indent zero-width jsx text
762-
const siblings = parent.getChildren(sourceFile);
763-
const currentIndex = findIndex(siblings, arg => arg.pos === child.pos);
764-
const previousNode = siblings[currentIndex - 1];
765-
if (previousNode) {
766-
// The jsx text needs no indentation whatsoever if it ends on the same line the previous sibling ends on
767-
if (sourceFile.getLineAndCharacterOfPosition(range.end).line !== sourceFile.getLineAndCharacterOfPosition(previousNode.end).line) {
768-
// The first line is (already) "indented" if the text starts on the same line as the previous sibling element ends on
769-
const firstLineIsIndented = sourceFile.getLineAndCharacterOfPosition(range.pos).line === sourceFile.getLineAndCharacterOfPosition(previousNode.end).line;
770-
indentMultilineCommentOrJsxText(range, childIndentation.indentation, firstLineIsIndented, /*indentFinalLine*/ false, /*jsxStyle*/ true);
771-
}
772-
}
773-
}
774-
}
775754

776755
childContextNode = node;
777756

@@ -930,7 +909,7 @@ namespace ts.formatting {
930909
switch (triviaItem.kind) {
931910
case SyntaxKind.MultiLineCommentTrivia:
932911
if (triviaInRange) {
933-
indentMultilineCommentOrJsxText(triviaItem, commentIndentation, /*firstLineIsIndented*/ !indentNextTokenOrTrivia);
912+
indentMultilineComment(triviaItem, commentIndentation, /*firstLineIsIndented*/ !indentNextTokenOrTrivia);
934913
}
935914
indentNextTokenOrTrivia = false;
936915
break;
@@ -1073,7 +1052,7 @@ namespace ts.formatting {
10731052
return indentationString !== sourceFile.text.substr(startLinePosition, indentationString.length);
10741053
}
10751054

1076-
function indentMultilineCommentOrJsxText(commentRange: TextRange, indentation: number, firstLineIsIndented: boolean, indentFinalLine = true, jsxTextStyleIndent?: boolean) {
1055+
function indentMultilineComment(commentRange: TextRange, indentation: number, firstLineIsIndented: boolean, indentFinalLine = true) {
10771056
// split comment in lines
10781057
let startLine = sourceFile.getLineAndCharacterOfPosition(commentRange.pos).line;
10791058
const endLine = sourceFile.getLineAndCharacterOfPosition(commentRange.end).line;
@@ -1111,19 +1090,13 @@ namespace ts.formatting {
11111090
}
11121091

11131092
// shift all parts on the delta size
1114-
let delta = indentation - nonWhitespaceColumnInFirstPart.column;
1093+
const delta = indentation - nonWhitespaceColumnInFirstPart.column;
11151094
for (let i = startIndex; i < parts.length; i++ , startLine++) {
11161095
const startLinePos = getStartPositionOfLine(startLine, sourceFile);
11171096
const nonWhitespaceCharacterAndColumn =
11181097
i === 0
11191098
? nonWhitespaceColumnInFirstPart
11201099
: SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(parts[i].pos, parts[i].end, sourceFile, options);
1121-
if (jsxTextStyleIndent) {
1122-
// skip adding indentation to blank lines
1123-
if (isLineBreak(sourceFile.text.charCodeAt(getStartPositionOfLine(startLine, sourceFile)))) continue;
1124-
// reset delta on every line
1125-
delta = indentation - nonWhitespaceCharacterAndColumn.column;
1126-
}
11271100
const newIndentation = nonWhitespaceCharacterAndColumn.column + delta;
11281101
if (newIndentation > 0) {
11291102
const indentationString = getIndentationString(newIndentation, options);

‎src/services/formatting/formattingScanner.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -124,13 +124,7 @@ namespace ts.formatting {
124124
}
125125

126126
function shouldRescanJsxText(node: Node): boolean {
127-
const isJSXText = isJsxText(node);
128-
if (isJSXText) {
129-
const containingElement = findAncestor(node.parent, p => isJsxElement(p));
130-
if (!containingElement) return false; // should never happen
131-
return !isParenthesizedExpression(containingElement.parent);
132-
}
133-
return false;
127+
return isJsxText(node);
134128
}
135129

136130
function shouldRescanSlashToken(container: Node): boolean {
@@ -252,7 +246,7 @@ namespace ts.formatting {
252246
return scanner.scanJsxIdentifier();
253247
case ScanAction.RescanJsxText:
254248
lastScanAction = ScanAction.RescanJsxText;
255-
return scanner.reScanJsxToken();
249+
return scanner.reScanJsxToken(/* allowMultilineJsxText */ false);
256250
case ScanAction.RescanJsxAttributeValue:
257251
lastScanAction = ScanAction.RescanJsxAttributeValue;
258252
return scanner.reScanJsxAttributeValue();

‎tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3989,7 +3989,7 @@ declare namespace ts {
39893989
scanJsxIdentifier(): SyntaxKind;
39903990
scanJsxAttributeValue(): SyntaxKind;
39913991
reScanJsxAttributeValue(): SyntaxKind;
3992-
reScanJsxToken(): JsxTokenSyntaxKind;
3992+
reScanJsxToken(allowMultilineJsxText?: boolean): JsxTokenSyntaxKind;
39933993
reScanLessThanToken(): SyntaxKind;
39943994
reScanQuestionToken(): SyntaxKind;
39953995
reScanInvalidIdentifier(): SyntaxKind;

‎tests/baselines/reference/api/typescript.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3989,7 +3989,7 @@ declare namespace ts {
39893989
scanJsxIdentifier(): SyntaxKind;
39903990
scanJsxAttributeValue(): SyntaxKind;
39913991
reScanJsxAttributeValue(): SyntaxKind;
3992-
reScanJsxToken(): JsxTokenSyntaxKind;
3992+
reScanJsxToken(allowMultilineJsxText?: boolean): JsxTokenSyntaxKind;
39933993
reScanLessThanToken(): SyntaxKind;
39943994
reScanQuestionToken(): SyntaxKind;
39953995
reScanInvalidIdentifier(): SyntaxKind;

‎tests/cases/fourslash/formattingJsxTexts.ts renamed to ‎tests/cases/fourslash/formattingJsxTexts1.ts

File renamed without changes.

‎tests/cases/fourslash/formattingJsxTexts2.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//@Filename: file.tsx
4+
//// const a = (
5+
//// <div>
6+
//// foo
7+
//// </div>
8+
//// );
9+
////
10+
//// const b = (
11+
//// <div>
12+
//// { foo }
13+
//// </div>
14+
//// );
15+
////
16+
//// const c = (
17+
//// <div>
18+
//// foo
19+
//// { foobar }
20+
//// bar
21+
//// </div>
22+
//// );
23+
////
24+
//// const d =
25+
//// <div>
26+
//// foo
27+
//// </div>;
28+
////
29+
//// const e =
30+
//// <div>
31+
//// { foo }
32+
//// </div>
33+
////
34+
//// const f =
35+
//// <div>
36+
//// foo
37+
//// { foobar }
38+
//// bar
39+
//// </div>
40+
41+
format.document();
42+
43+
verify.currentFileContentIs(
44+
`const a = (
45+
<div>
46+
foo
47+
</div>
48+
);
49+
50+
const b = (
51+
<div>
52+
{foo}
53+
</div>
54+
);
55+
56+
const c = (
57+
<div>
58+
foo
59+
{foobar}
60+
bar
61+
</div>
62+
);
63+
64+
const d =
65+
<div>
66+
foo
67+
</div>;
68+
69+
const e =
70+
<div>
71+
{foo}
72+
</div>
73+
74+
const f =
75+
<div>
76+
foo
77+
{foobar}
78+
bar
79+
</div>`);

‎tests/cases/fourslash/formattingJsxTexts3.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// Github issue #41925
4+
5+
//@Filename: file.tsx
6+
//// function foo() {
7+
//// const bar = "Oh no";
8+
////
9+
//// return (
10+
//// <div>"{bar}"</div>
11+
//// )
12+
//// }
13+
14+
format.document();
15+
16+
verify.currentFileContentIs(
17+
`function foo() {
18+
const bar = "Oh no";
19+
20+
return (
21+
<div>"{bar}"</div>
22+
)
23+
}`);

0 commit comments

Comments
 (0)