Skip to content

Commit 7726464

Browse files
Ortaweswigham
Orta
andauthored
De-duplicate indentations in JSX Texts (microsoft#36552)
* WIP on making the JSX text node not include whitespace * Scans to the last newline for JSX correctly * Handle JSX closing element wrapping * Offload all jsx text indentation handling to indentMultilineCommentOrJsxText * Switch from find node -> find inde in formatting Co-authored-by: Wesley Wigham <wwigham@gmail.com>
1 parent 1c42fd4 commit 7726464

File tree

4 files changed

+80
-8
lines changed

4 files changed

+80
-8
lines changed

Diff for: src/compiler/scanner.ts

+15-1
Original file line numberDiff line numberDiff line change
@@ -2068,10 +2068,19 @@ namespace ts {
20682068

20692069
// First non-whitespace character on this line.
20702070
let firstNonWhitespace = 0;
2071+
let lastNonWhitespace = -1;
2072+
20712073
// These initial values are special because the first line is:
20722074
// firstNonWhitespace = 0 to indicate that we want leading whitespace,
20732075

20742076
while (pos < end) {
2077+
2078+
// We want to keep track of the last non-whitespace (but including
2079+
// newlines character for hitting the end of the JSX Text region)
2080+
if (!isWhiteSpaceSingleLine(char)) {
2081+
lastNonWhitespace = pos;
2082+
}
2083+
20752084
char = text.charCodeAt(pos);
20762085
if (char === CharacterCodes.openBrace) {
20772086
break;
@@ -2084,6 +2093,8 @@ namespace ts {
20842093
break;
20852094
}
20862095

2096+
if (lastNonWhitespace > 0) lastNonWhitespace++;
2097+
20872098
// FirstNonWhitespace is 0, then we only see whitespaces so far. If we see a linebreak, we want to ignore that whitespaces.
20882099
// i.e (- : whitespace)
20892100
// <div>----
@@ -2096,10 +2107,13 @@ namespace ts {
20962107
else if (!isWhiteSpaceLike(char)) {
20972108
firstNonWhitespace = pos;
20982109
}
2110+
20992111
pos++;
21002112
}
21012113

2102-
tokenValue = text.substring(startPos, pos);
2114+
const endPosition = lastNonWhitespace === -1 ? pos : lastNonWhitespace;
2115+
tokenValue = text.substring(startPos, endPosition);
2116+
21032117
return firstNonWhitespace === -1 ? SyntaxKind.JsxTextAllWhiteSpaces : SyntaxKind.JsxText;
21042118
}
21052119

Diff for: src/services/formatting/formatting.ts

+27-6
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,11 @@ namespace ts.formatting {
649649
if (tokenInfo.token.end > node.end) {
650650
break;
651651
}
652+
if (node.kind === SyntaxKind.JsxText) {
653+
// Intentation rules for jsx text are handled by `indentMultilineCommentOrJsxText` inside `processChildNode`; just fastforward past it here
654+
formattingScanner.advance();
655+
continue;
656+
}
652657
consumeTokenAndAdvanceScanner(tokenInfo, node, nodeDynamicIndentation, node);
653658
}
654659

@@ -736,10 +741,21 @@ namespace ts.formatting {
736741
const childIndentation = computeIndentation(child, childStartLine, childIndentationAmount, node, parentDynamicIndentation, effectiveParentStartLine);
737742

738743
processNode(child, childContextNode, childStartLine, undecoratedChildStartLine, childIndentation.indentation, childIndentation.delta);
739-
740744
if (child.kind === SyntaxKind.JsxText) {
741745
const range: TextRange = { pos: child.getStart(), end: child.getEnd() };
742-
indentMultilineCommentOrJsxText(range, childIndentation.indentation, /*firstLineIsIndented*/ true, /*indentFinalLine*/ false);
746+
if (range.pos !== range.end) { // don't indent zero-width jsx text
747+
const siblings = parent.getChildren(sourceFile);
748+
const currentIndex = findIndex(siblings, arg => arg.pos === child.pos);
749+
const previousNode = siblings[currentIndex - 1];
750+
if (previousNode) {
751+
// The jsx text needs no indentation whatsoever if it ends on the same line the previous sibling ends on
752+
if (sourceFile.getLineAndCharacterOfPosition(range.end).line !== sourceFile.getLineAndCharacterOfPosition(previousNode.end).line) {
753+
// The first line is (already) "indented" if the text starts on the same line as the previous sibling element ends on
754+
const firstLineIsIndented = sourceFile.getLineAndCharacterOfPosition(range.pos).line === sourceFile.getLineAndCharacterOfPosition(previousNode.end).line;
755+
indentMultilineCommentOrJsxText(range, childIndentation.indentation, firstLineIsIndented, /*indentFinalLine*/ false, /*jsxStyle*/ true);
756+
}
757+
}
758+
}
743759
}
744760

745761
childContextNode = node;
@@ -1039,7 +1055,7 @@ namespace ts.formatting {
10391055
return indentationString !== sourceFile.text.substr(startLinePosition, indentationString.length);
10401056
}
10411057

1042-
function indentMultilineCommentOrJsxText(commentRange: TextRange, indentation: number, firstLineIsIndented: boolean, indentFinalLine = true) {
1058+
function indentMultilineCommentOrJsxText(commentRange: TextRange, indentation: number, firstLineIsIndented: boolean, indentFinalLine = true, jsxTextStyleIndent?: boolean) {
10431059
// split comment in lines
10441060
let startLine = sourceFile.getLineAndCharacterOfPosition(commentRange.pos).line;
10451061
const endLine = sourceFile.getLineAndCharacterOfPosition(commentRange.end).line;
@@ -1070,7 +1086,7 @@ namespace ts.formatting {
10701086
const nonWhitespaceColumnInFirstPart =
10711087
SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(startLinePos, parts[0].pos, sourceFile, options);
10721088

1073-
if (indentation === nonWhitespaceColumnInFirstPart.column) {
1089+
if (indentation === nonWhitespaceColumnInFirstPart.column && !jsxTextStyleIndent) {
10741090
return;
10751091
}
10761092

@@ -1081,14 +1097,19 @@ namespace ts.formatting {
10811097
}
10821098

10831099
// shift all parts on the delta size
1084-
const delta = indentation - nonWhitespaceColumnInFirstPart.column;
1100+
let delta = indentation - nonWhitespaceColumnInFirstPart.column;
10851101
for (let i = startIndex; i < parts.length; i++ , startLine++) {
10861102
const startLinePos = getStartPositionOfLine(startLine, sourceFile);
10871103
const nonWhitespaceCharacterAndColumn =
10881104
i === 0
10891105
? nonWhitespaceColumnInFirstPart
10901106
: SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(parts[i].pos, parts[i].end, sourceFile, options);
1091-
1107+
if (jsxTextStyleIndent) {
1108+
// skip adding indentation to blank lines
1109+
if (isLineBreak(sourceFile.text.charCodeAt(getStartPositionOfLine(startLine, sourceFile)))) continue;
1110+
// reset delta on every line
1111+
delta = indentation - nonWhitespaceCharacterAndColumn.column;
1112+
}
10921113
const newIndentation = nonWhitespaceCharacterAndColumn.column + delta;
10931114
if (newIndentation > 0) {
10941115
const indentationString = getIndentationString(newIndentation, options);

Diff for: src/services/formatting/formattingScanner.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,13 @@ namespace ts.formatting {
122122
}
123123

124124
function shouldRescanJsxText(node: Node): boolean {
125-
return node.kind === SyntaxKind.JsxText;
125+
const isJSXText = isJsxText(node);
126+
if (isJSXText) {
127+
const containingElement = findAncestor(node.parent, p => isJsxElement(p));
128+
if (!containingElement) return false; // should never happen
129+
return !isParenthesizedExpression(containingElement.parent);
130+
}
131+
return false;
126132
}
127133

128134
function shouldRescanSlashToken(container: Node): boolean {
+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/// <reference path="fourslash.ts"/>
2+
// @Filename: foo.tsx
3+
////
4+
////const a = (
5+
//// <div>
6+
//// text
7+
//// </div>
8+
////)
9+
////const b = (
10+
//// <div>
11+
//// text
12+
//// twice
13+
//// </div>
14+
////)
15+
////
16+
17+
18+
format.document();
19+
verify.currentFileContentIs(`
20+
const a = (
21+
<div>
22+
text
23+
</div>
24+
)
25+
const b = (
26+
<div>
27+
text
28+
twice
29+
</div>
30+
)
31+
`);

0 commit comments

Comments
 (0)