Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 7726464

Browse filesBrowse files
Ortaweswigham
Orta
and
weswigham
authored
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
Copy full SHA for 7726464

File tree

Expand file treeCollapse file tree

4 files changed

+80
-8
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+80
-8
lines changed

‎src/compiler/scanner.ts

Copy file name to clipboardExpand all lines: src/compiler/scanner.ts
+15-1Lines changed: 15 additions & 1 deletion
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

‎src/services/formatting/formatting.ts

Copy file name to clipboardExpand all lines: src/services/formatting/formatting.ts
+27-6Lines changed: 27 additions & 6 deletions
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);

‎src/services/formatting/formattingScanner.ts

Copy file name to clipboardExpand all lines: src/services/formatting/formattingScanner.ts
+7-1Lines changed: 7 additions & 1 deletion
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 {

‎tests/cases/fourslash/formatTsxClosingAfterJsxText.ts

Copy file name to clipboard
+31Lines changed: 31 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.