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 <[email protected]>
jablko · Feb 3, 2020 · 7726464 · 7726464
1 parent 1c42fd4
commit 7726464
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 8 deletions.
diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts
@@ -2068,10 +2068,19 @@ namespace ts {
 
             // First non-whitespace character on this line.
             let firstNonWhitespace = 0;
+            let lastNonWhitespace = -1;
+
             // These initial values are special because the first line is:
             // firstNonWhitespace = 0 to indicate that we want leading whitespace,
 
             while (pos < end) {
+
+                // We want to keep track of the last non-whitespace (but including
+                // newlines character for hitting the end of the JSX Text region)
+                if (!isWhiteSpaceSingleLine(char)) {
+                    lastNonWhitespace = pos;
+                }
+
                 char = text.charCodeAt(pos);
                 if (char === CharacterCodes.openBrace) {
                     break;
@@ -2084,6 +2093,8 @@ namespace ts {
                     break;
                 }
 
+                if (lastNonWhitespace > 0) lastNonWhitespace++;
+
                 // FirstNonWhitespace is 0, then we only see whitespaces so far. If we see a linebreak, we want to ignore that whitespaces.
                 // i.e (- : whitespace)
                 //      <div>----
@@ -2096,10 +2107,13 @@ namespace ts {
                 else if (!isWhiteSpaceLike(char)) {
                     firstNonWhitespace = pos;
                 }
+
                 pos++;
             }
 
-            tokenValue = text.substring(startPos, pos);
+            const endPosition = lastNonWhitespace === -1 ? pos : lastNonWhitespace;
+            tokenValue = text.substring(startPos, endPosition);
+
             return firstNonWhitespace === -1 ? SyntaxKind.JsxTextAllWhiteSpaces : SyntaxKind.JsxText;
         }
 

diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts
@@ -649,6 +649,11 @@ namespace ts.formatting {
                 if (tokenInfo.token.end > node.end) {
                     break;
                 }
+                if (node.kind === SyntaxKind.JsxText) {
+                    // Intentation rules for jsx text are handled by `indentMultilineCommentOrJsxText` inside `processChildNode`; just fastforward past it here
+                    formattingScanner.advance();
+                    continue;
+                }
                 consumeTokenAndAdvanceScanner(tokenInfo, node, nodeDynamicIndentation, node);
             }
 
@@ -736,10 +741,21 @@ namespace ts.formatting {
                 const childIndentation = computeIndentation(child, childStartLine, childIndentationAmount, node, parentDynamicIndentation, effectiveParentStartLine);
 
                 processNode(child, childContextNode, childStartLine, undecoratedChildStartLine, childIndentation.indentation, childIndentation.delta);
-
                 if (child.kind === SyntaxKind.JsxText) {
                     const range: TextRange = { pos: child.getStart(), end: child.getEnd() };
-                    indentMultilineCommentOrJsxText(range, childIndentation.indentation, /*firstLineIsIndented*/ true, /*indentFinalLine*/ false);
+                    if (range.pos !== range.end) { // don't indent zero-width jsx text
+                        const siblings = parent.getChildren(sourceFile);
+                        const currentIndex = findIndex(siblings, arg => arg.pos === child.pos);
+                        const previousNode = siblings[currentIndex - 1];
+                        if (previousNode) {
+                            // The jsx text needs no indentation whatsoever if it ends on the same line the previous sibling ends on
+                            if (sourceFile.getLineAndCharacterOfPosition(range.end).line !== sourceFile.getLineAndCharacterOfPosition(previousNode.end).line) {
+                                // The first line is (already) "indented" if the text starts on the same line as the previous sibling element ends on
+                                const firstLineIsIndented = sourceFile.getLineAndCharacterOfPosition(range.pos).line === sourceFile.getLineAndCharacterOfPosition(previousNode.end).line;
+                                indentMultilineCommentOrJsxText(range, childIndentation.indentation, firstLineIsIndented, /*indentFinalLine*/ false, /*jsxStyle*/ true);
+                            }
+                        }
+                    }
                 }
 
                 childContextNode = node;
@@ -1039,7 +1055,7 @@ namespace ts.formatting {
             return indentationString !== sourceFile.text.substr(startLinePosition, indentationString.length);
         }
 
-        function indentMultilineCommentOrJsxText(commentRange: TextRange, indentation: number, firstLineIsIndented: boolean, indentFinalLine = true) {
+        function indentMultilineCommentOrJsxText(commentRange: TextRange, indentation: number, firstLineIsIndented: boolean, indentFinalLine = true, jsxTextStyleIndent?: boolean) {
             // split comment in lines
             let startLine = sourceFile.getLineAndCharacterOfPosition(commentRange.pos).line;
             const endLine = sourceFile.getLineAndCharacterOfPosition(commentRange.end).line;
@@ -1070,7 +1086,7 @@ namespace ts.formatting {
             const nonWhitespaceColumnInFirstPart =
                 SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(startLinePos, parts[0].pos, sourceFile, options);
 
-            if (indentation === nonWhitespaceColumnInFirstPart.column) {
+            if (indentation === nonWhitespaceColumnInFirstPart.column && !jsxTextStyleIndent) {
                 return;
             }
 
@@ -1081,14 +1097,19 @@ namespace ts.formatting {
             }
 
             // shift all parts on the delta size
-            const delta = indentation - nonWhitespaceColumnInFirstPart.column;
+            let delta = indentation - nonWhitespaceColumnInFirstPart.column;
             for (let i = startIndex; i < parts.length; i++ , startLine++) {
                 const startLinePos = getStartPositionOfLine(startLine, sourceFile);
                 const nonWhitespaceCharacterAndColumn =
                     i === 0
                         ? nonWhitespaceColumnInFirstPart
                         : SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(parts[i].pos, parts[i].end, sourceFile, options);
-
+                if (jsxTextStyleIndent) {
+                    // skip adding indentation to blank lines
+                    if (isLineBreak(sourceFile.text.charCodeAt(getStartPositionOfLine(startLine, sourceFile)))) continue;
+                    // reset delta on every line
+                    delta = indentation - nonWhitespaceCharacterAndColumn.column;
+                }
                 const newIndentation = nonWhitespaceCharacterAndColumn.column + delta;
                 if (newIndentation > 0) {
                     const indentationString = getIndentationString(newIndentation, options);

diff --git a/src/services/formatting/formattingScanner.ts b/src/services/formatting/formattingScanner.ts
@@ -122,7 +122,13 @@ namespace ts.formatting {
         }
 
         function shouldRescanJsxText(node: Node): boolean {
-            return node.kind === SyntaxKind.JsxText;
+            const isJSXText = isJsxText(node);
+            if (isJSXText) {
+                const containingElement = findAncestor(node.parent, p => isJsxElement(p));
+                if (!containingElement) return false; // should never happen
+                return !isParenthesizedExpression(containingElement.parent);
+            }
+            return false;
         }
 
         function shouldRescanSlashToken(container: Node): boolean {

diff --git a/tests/cases/fourslash/formatTsxClosingAfterJsxText.ts b/tests/cases/fourslash/formatTsxClosingAfterJsxText.ts
@@ -0,0 +1,31 @@
+/// <reference path="fourslash.ts"/>
+// @Filename: foo.tsx
+////
+////const a = (
+////    <div>
+////        text
+////               </div>
+////)
+////const b = (
+////    <div>
+////        text
+////      twice
+////               </div>
+////)
+////
+
+
+format.document();
+verify.currentFileContentIs(`
+const a = (
+    <div>
+        text
+    </div>
+)
+const b = (
+    <div>
+        text
+        twice
+    </div>
+)
+`);