fix: splitting jsx text correctly (#5006)

Fixes: #4941
2018-09-02 20:59:28 +03:00 · 2018-09-02 20:59:28 +03:00 · 0c7c1bfde4
parent 91206891cd
commit 0c7c1bfde4
4 changed files with 182 additions and 57 deletions
--- 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) {
--- 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>
 );

--- 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
+    before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}
    {stuff}
-    after
-    {stuff}
-    after
-    {stuff}
-    after
-    {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>
+);
+
 `;
--- 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>