fix: splitting jsx text correctly (#5006)

Fixes: #4941
prettier · Sep 2, 2018 · 0c7c1bf · 0c7c1bf
1 parent 9120689
commit 0c7c1bf
Show file tree

Hide file tree

Showing 4 changed files with 182 additions and 57 deletions.
diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js
@@ -4732,16 +4732,24 @@ function isJSXWhitespaceExpression(node) {
   );
 }
 
-function separatorNoWhitespace(isFacebookTranslationTag, child) {
+function separatorNoWhitespace(
+  isFacebookTranslationTag,
+  child,
+  childNode,
+  nextNode
+) {
   if (isFacebookTranslationTag) {
     return "";
   }
 
-  if (child.length === 1) {
-    return softline;
+  if (
+    (childNode.type === "JSXElement" && !childNode.closingElement) ||
+    (nextNode && (nextNode.type === "JSXElement" && !nextNode.closingElement))
+  ) {
+    return hardline;
   }
 
-  return hardline;
+  return softline;
 }
 
 function separatorWithWhitespace(isFacebookTranslationTag, child) {
@@ -4834,8 +4842,14 @@ function printJSXChildren(
             children.push(jsxWhitespace);
           }
         } else {
+          const next = n.children[i + 1];
           children.push(
-            separatorNoWhitespace(isFacebookTranslationTag, getLast(children))
+            separatorNoWhitespace(
+              isFacebookTranslationTag,
+              getLast(children),
+              child,
+              next
+            )
           );
         }
       } else if (/\n/.test(text)) {
@@ -4861,7 +4875,12 @@ function printJSXChildren(
           .trim()
           .split(matchJsxWhitespaceRegex)[0];
         children.push(
-          separatorNoWhitespace(isFacebookTranslationTag, firstWord)
+          separatorNoWhitespace(
+            isFacebookTranslationTag,
+            firstWord,
+            child,
+            next
+          )
         );
       } else {
         children.push(hardline);
@@ -4987,12 +5006,20 @@ function printJSXElement(path, options, print) {
       children[i] === jsxWhitespace &&
       children[i + 1] === "" &&
       children[i + 2] === jsxWhitespace;
+    const isPairOfHardOrSoftLines =
+      (children[i] === softline &&
+        children[i + 1] === "" &&
+        children[i + 2] === hardline) ||
+      (children[i] === hardline &&
+        children[i + 1] === "" &&
+        children[i + 2] === softline);
 
     if (
       (isPairOfHardlines && containsText) ||
       isPairOfEmptyStrings ||
       isLineFollowedByJSXWhitespace ||
-      isDoubleJSXWhitespace
+      isDoubleJSXWhitespace ||
+      isPairOfHardOrSoftLines
     ) {
       children.splice(i, 2);
     } else if (isJSXWhitespaceFollowedByLine) {

diff --git a/tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap
@@ -141,8 +141,7 @@ break_components = (
     <Foo />
     <Bar>
       <p>
-        foo
-        <span>bar bar bar</span>
+        foo<span>bar bar bar</span>
       </p>
       <h1>
         <span>
@@ -210,8 +209,7 @@ not_broken_end = (
 not_broken_begin = (
   <div>
     <br /> long text long text long text long text long text long text long text
-    long text
-    <link>url</link> long text long text
+    long text<link>url</link> long text long text
   </div>
 );
 

diff --git a/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap
@@ -413,6 +413,45 @@ x =
     {minute}:
     {second}
   </div>
+
+x = <div><strong>text here</strong>.<br /></div>
+
+x = <div>Sales tax estimated using a rate of {salesTax * 100}%.</div>
+
+x = <div>
+  {title}&nbsp;
+</div>
+
+x = <div><span/>bar</div>
+  
+x = <div>
+  <span>
+    <strong>{name}</strong>’s{' '}
+  </span>
+  Hello <strong>world</strong>.<br />
+  <Text>You {type}ed this shipment to</Text>
+</div>
+
+x = <HelpBlock>
+  {parameter.Description}: {errorMsg}
+</HelpBlock>
+  
+x = <label>
+  {value} solution{plural}
+</label>
+  
+x = <span>Copy &quot;{name}&quot;</span>
+  
+x = <BasicText light>(avg. {value}/5)</BasicText>
+  
+x = <p>
+  Use the <code>Button</code>'s
+</p>;
+
+this_really_should_split_across_lines =
+  <div>
+    before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after
+  </div>
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // Wrapping text
 x = (
@@ -474,23 +513,9 @@ x = (
 
 x = (
   <div>
-    before
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
+    before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}
     {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    {stuff}
-    {stuff}
-    after
-    {stuff}
-    after
+    {stuff}after{stuff}after
   </div>
 );
 
@@ -598,35 +623,9 @@ x = (
 
 this_really_should_split_across_lines = (
   <div>
-    before
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
+    before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}
+    after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}
+    after{stuff}after{stuff}after
   </div>
 );
 
@@ -774,7 +773,11 @@ line_after_br = (
 
 line_after_br_2 = (
   <div>
-    A<br />B<br />C
+    A
+    <br />
+    B
+    <br />
+    C
   </div>
 );
 
@@ -962,4 +965,62 @@ x = (
   </div>
 );
 
+x = (
+  <div>
+    <strong>text here</strong>.
+    <br />
+  </div>
+);
+
+x = <div>Sales tax estimated using a rate of {salesTax * 100}%.</div>;
+
+x = <div>{title}&nbsp;</div>;
+
+x = (
+  <div>
+    <span />
+    bar
+  </div>
+);
+
+x = (
+  <div>
+    <span>
+      <strong>{name}</strong>’s{" "}
+    </span>
+    Hello <strong>world</strong>.
+    <br />
+    <Text>You {type}ed this shipment to</Text>
+  </div>
+);
+
+x = (
+  <HelpBlock>
+    {parameter.Description}: {errorMsg}
+  </HelpBlock>
+);
+
+x = (
+  <label>
+    {value} solution{plural}
+  </label>
+);
+
+x = <span>Copy &quot;{name}&quot;</span>;
+
+x = <BasicText light>(avg. {value}/5)</BasicText>;
+
+x = (
+  <p>
+    Use the <code>Button</code>'s
+  </p>
+);
+
+this_really_should_split_across_lines = (
+  <div>
+    before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}
+    after{stuff}after
+  </div>
+);
+
 `;
diff --git a/tests/jsx-text-wrap/test.js b/tests/jsx-text-wrap/test.js
@@ -410,3 +410,42 @@ x =
     {minute}:
     {second}
   </div>
+
+x = <div><strong>text here</strong>.<br /></div>
+
+x = <div>Sales tax estimated using a rate of {salesTax * 100}%.</div>
+
+x = <div>
+  {title}&nbsp;
+</div>
+
+x = <div><span/>bar</div>
+
+x = <div>
+  <span>
+    <strong>{name}</strong>’s{' '}
+  </span>
+  Hello <strong>world</strong>.<br />
+  <Text>You {type}ed this shipment to</Text>
+</div>
+
+x = <HelpBlock>
+  {parameter.Description}: {errorMsg}
+</HelpBlock>
+
+x = <label>
+  {value} solution{plural}
+</label>
+
+x = <span>Copy &quot;{name}&quot;</span>
+
+x = <BasicText light>(avg. {value}/5)</BasicText>
+
+x = <p>
+  Use the <code>Button</code>'s
+</p>;
+
+this_really_should_split_across_lines =
+  <div>
+    before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after
+  </div>