Handle JSX whitespace separately from fbt whitespace (#4717)

* Hande JSX whitespace separately from fbt whitespace

* Extra test cases

* Further tweak to JSX handling

* Possible improvement of single character text

* More tests

* Pull out separator code

* More tests for fbt

src/language-js/printer-estree.js

@@ -4731,6 +4731,30 @@ function isJSXWhitespaceExpression(node) {
   );
 }
 
+function separatorNoWhitespace(isFacebookTranslationTag, child) {
+  if (isFacebookTranslationTag) {
+    return "";
+  }
+
+  if (child.length === 1) {
+    return softline;
+  }
+
+  return hardline;
+}
+
+function separatorWithWhitespace(isFacebookTranslationTag, child) {
+  if (isFacebookTranslationTag) {
+    return hardline;
+  }
+
+  if (child.length === 1) {
+    return softline;
+  }
+
+  return hardline;
+}
+
 // JSX Children are strange, mostly for two reasons:
 // 1. JSX reads newlines into string values, instead of skipping them like JS
 // 2. up to one whitespace between elements within a line is significant,
@@ -4743,7 +4767,13 @@ function isJSXWhitespaceExpression(node) {
 // This requires that we give it an array of alternating
 // content and whitespace elements.
 // To ensure this we add dummy `""` content elements as needed.
-function printJSXChildren(path, options, print, jsxWhitespace) {
+function printJSXChildren(
+  path,
+  options,
+  print,
+  jsxWhitespace,
+  isFacebookTranslationTag
+) {
   const n = path.getValue();
   const children = [];
 
@@ -4762,7 +4792,9 @@ function printJSXChildren(path, options, print, jsxWhitespace) {
           children.push("");
           words.shift();
           if (/\n/.test(words[0])) {
-            children.push(hardline);
+            children.push(
+              separatorWithWhitespace(isFacebookTranslationTag, words[1])
+            );
           } else {
             children.push(jsxWhitespace);
           }
@@ -4791,21 +4823,19 @@ function printJSXChildren(path, options, print, jsxWhitespace) {
 
         if (endWhitespace !== undefined) {
           if (/\n/.test(endWhitespace)) {
-            children.push(hardline);
+            children.push(
+              separatorWithWhitespace(
+                isFacebookTranslationTag,
+                getLast(children)
+              )
+            );
           } else {
             children.push(jsxWhitespace);
           }
         } else {
-          // Ideally this would be a `hardline` to allow a break between
-          // tags and text.
-          // Unfortunately Facebook have a custom translation pipeline
-          // (https://github.com/prettier/prettier/issues/1581#issuecomment-300975032)
-          // that uses the JSX syntax, but does not follow the React whitespace
-          // rules.
-          // Ensuring that we never have a break between tags and text in JSX
-          // will allow Facebook to adopt Prettier without too much of an
-          // adverse effect on formatting algorithm.
-          children.push("");
+          children.push(
+            separatorNoWhitespace(isFacebookTranslationTag, getLast(children))
+          );
         }
       } else if (/\n/.test(text)) {
         // Keep (up to one) blank line between tags/expressions/text.
@@ -4824,12 +4854,14 @@ function printJSXChildren(path, options, print, jsxWhitespace) {
 
       const next = n.children[i + 1];
       const directlyFollowedByMeaningfulText =
-        next && isMeaningfulJSXText(next) && !/^[ \n\r\t]/.test(rawText(next));
+        next && isMeaningfulJSXText(next);
       if (directlyFollowedByMeaningfulText) {
-        // Potentially this could be a hardline as well.
-        // See the comment above about the Facebook translation pipeline as
-        // to why this is an empty string.
-        children.push("");
+        const firstWord = rawText(next)
+          .trim()
+          .split(matchJsxWhitespaceRegex)[0];
+        children.push(
+          separatorNoWhitespace(isFacebookTranslationTag, firstWord)
+        );
       } else {
         children.push(hardline);
       }
@@ -4916,7 +4948,18 @@ function printJSXElement(path, options, print) {
   const rawJsxWhitespace = options.singleQuote ? "{' '}" : '{" "}';
   const jsxWhitespace = ifBreak(concat([rawJsxWhitespace, softline]), " ");
 
-  const children = printJSXChildren(path, options, print, jsxWhitespace);
+  const isFacebookTranslationTag =
+    n.openingElement &&
+    n.openingElement.name &&
+    n.openingElement.name.name === "fbt";
+
+  const children = printJSXChildren(
+    path,
+    options,
+    print,
+    jsxWhitespace,
+    isFacebookTranslationTag
+  );
 
   const containsText =
     n.children.filter(child => isMeaningfulJSXText(child)).length > 0;

tests/jsx-fbt/__snapshots__/jsfmt.spec.js.snap

@@ -0,0 +1,225 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`test.js 1`] = `
+x =
+  <fbt>
+    <fbt:param>
+      First
+    </fbt:param>,
+    <fbt:param>
+      Second
+    </fbt:param>
+  </fbt>
+
+x =
+  <fbt>
+    <fbt:param>
+      First
+    </fbt:param>
+    ,
+    <fbt:param>
+      Second
+    </fbt:param>
+  </fbt>
+
+x =
+  <fbt>
+    <fbt:param>First</fbt:param>,<fbt:param>Second</fbt:param>
+  </fbt>
+
+x =
+  <fbt>
+    <fbt:param>
+      First
+    </fbt:param>,<fbt:param>
+      Second
+    </fbt:param>
+  </fbt>
+
+x =
+  <fbt desc="example 1">
+    Prefix comes before
+    <fbt:param>
+      <b>
+        suffix
+      </b>
+    </fbt:param>
+  </fbt>
+
+x =
+  <fbt desc="example 2">
+    Prefix comes before
+    <fbt:param name="bold stuff">
+      <b>
+      suffix
+      </b>
+    </fbt:param>
+    <fbt:param name="a link">
+      <link href="#">
+        suffix
+      </link>
+    </fbt:param>
+  </fbt>
+
+x =
+  <fbt desc="example 3">
+    Count Chocula knows the the number
+    <fbt:enum enum-range={['one', 'two', 'three']} value={getValue()} />
+    is awesome
+  </fbt>
+
+x = (
+  <fbt>
+    {hour}:{minute}:{second}
+  </fbt>
+);
+
+x = (
+  <fbt>
+    {hour}
+    :
+    {minute}
+    :
+    {second}
+  </fbt>
+);
+
+x = (
+  <fbt>
+    {hour}:
+    {minute}:
+    {second}
+  </fbt>
+);
+
+first = (
+  <fbt>
+    Text<br />
+    More text<br />
+    And more<br />
+  </fbt>
+);
+
+second = (
+  <fbt>
+    Text<br />More text<br />And more<br />
+  </fbt>
+);
+
+third = (
+  <fbt>
+    Text
+    <br />
+    More text
+    <br />
+    And more
+    <br />
+  </fbt>
+);
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+x = (
+  <fbt>
+    <fbt:param>First</fbt:param>,
+    <fbt:param>Second</fbt:param>
+  </fbt>
+);
+
+x = (
+  <fbt>
+    <fbt:param>First</fbt:param>
+    ,
+    <fbt:param>Second</fbt:param>
+  </fbt>
+);
+
+x = (
+  <fbt>
+    <fbt:param>First</fbt:param>,<fbt:param>Second</fbt:param>
+  </fbt>
+);
+
+x = (
+  <fbt>
+    <fbt:param>First</fbt:param>,<fbt:param>Second</fbt:param>
+  </fbt>
+);
+
+x = (
+  <fbt desc="example 1">
+    Prefix comes before
+    <fbt:param>
+      <b>suffix</b>
+    </fbt:param>
+  </fbt>
+);
+
+x = (
+  <fbt desc="example 2">
+    Prefix comes before
+    <fbt:param name="bold stuff">
+      <b>suffix</b>
+    </fbt:param>
+    <fbt:param name="a link">
+      <link href="#">suffix</link>
+    </fbt:param>
+  </fbt>
+);
+
+x = (
+  <fbt desc="example 3">
+    Count Chocula knows the the number
+    <fbt:enum enum-range={["one", "two", "three"]} value={getValue()} />
+    is awesome
+  </fbt>
+);
+
+x = (
+  <fbt>
+    {hour}:{minute}:{second}
+  </fbt>
+);
+
+x = (
+  <fbt>
+    {hour}
+    :
+    {minute}
+    :
+    {second}
+  </fbt>
+);
+
+x = (
+  <fbt>
+    {hour}:
+    {minute}:
+    {second}
+  </fbt>
+);
+
+first = (
+  <fbt>
+    Text<br />
+    More text<br />
+    And more<br />
+  </fbt>
+);
+
+second = (
+  <fbt>
+    Text<br />More text<br />And more<br />
+  </fbt>
+);
+
+third = (
+  <fbt>
+    Text
+    <br />
+    More text
+    <br />
+    And more
+    <br />
+  </fbt>
+);
+
+`;

tests/jsx-fbt/jsfmt.spec.js

@@ -0,0 +1 @@
+run_spec(__dirname, ["flow"]);

tests/jsx-fbt/test.js

@@ -0,0 +1,115 @@
+x =
+  <fbt>
+    <fbt:param>
+      First
+    </fbt:param>,
+    <fbt:param>
+      Second
+    </fbt:param>
+  </fbt>
+
+x =
+  <fbt>
+    <fbt:param>
+      First
+    </fbt:param>
+    ,
+    <fbt:param>
+      Second
+    </fbt:param>
+  </fbt>
+
+x =
+  <fbt>
+    <fbt:param>First</fbt:param>,<fbt:param>Second</fbt:param>
+  </fbt>
+
+x =
+  <fbt>
+    <fbt:param>
+      First
+    </fbt:param>,<fbt:param>
+      Second
+    </fbt:param>
+  </fbt>
+
+x =
+  <fbt desc="example 1">
+    Prefix comes before
+    <fbt:param>
+      <b>
+        suffix
+      </b>
+    </fbt:param>
+  </fbt>
+
+x =
+  <fbt desc="example 2">
+    Prefix comes before
+    <fbt:param name="bold stuff">
+      <b>
+      suffix
+      </b>
+    </fbt:param>
+    <fbt:param name="a link">
+      <link href="#">
+        suffix
+      </link>
+    </fbt:param>
+  </fbt>
+
+x =
+  <fbt desc="example 3">
+    Count Chocula knows the the number
+    <fbt:enum enum-range={['one', 'two', 'three']} value={getValue()} />
+    is awesome
+  </fbt>
+
+x = (
+  <fbt>
+    {hour}:{minute}:{second}
+  </fbt>
+);
+
+x = (
+  <fbt>
+    {hour}
+    :
+    {minute}
+    :
+    {second}
+  </fbt>
+);
+
+x = (
+  <fbt>
+    {hour}:
+    {minute}:
+    {second}
+  </fbt>
+);
+
+first = (
+  <fbt>
+    Text<br />
+    More text<br />
+    And more<br />
+  </fbt>
+);
+
+second = (
+  <fbt>
+    Text<br />More text<br />And more<br />
+  </fbt>
+);
+
+third = (
+  <fbt>
+    Text
+    <br />
+    More text
+    <br />
+    And more
+    <br />
+  </fbt>
+);

tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap

@@ -141,7 +141,8 @@ break_components = (
     <Foo />
     <Bar>
       <p>
-        foo<span>bar bar bar</span>
+        foo
+        <span>bar bar bar</span>
       </p>
       <h1>
         <span>
@@ -209,7 +210,8 @@ 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>
 );
 

tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap

@@ -132,7 +132,8 @@ long_open_long_children = (
         colour="blue"
         size="large"
         submitLabel="Sign in with Google"
-      />d
+      />
+      d
     </BaseForm>
     <BaseForm
       url="/auth/google"

tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap

@@ -161,10 +161,12 @@ const render7B = () => (
       <span /> Dont break plz
     </span>
     <span>
-      <span />Dont break plz
+      <span />
+      Dont break plz
     </span>
     <span>
-      Dont break plz<span />
+      Dont break plz
+      <span />
     </span>
   </div>
 );

tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap

@@ -116,6 +116,16 @@ facebook_translation_leave_text_around_tag =
     </span>)
   </div>
 
+x =
+  <div>
+    <span>
+      First second third fourth fifth sixth seventh
+    </span>,
+    (<span>
+      Second
+    </span>)
+  </div>
+
 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
@@ -217,10 +227,25 @@ with_text_fill_line =
 line_after_br =
   <div>
     Text<br />
-    More text <br />
+    More text<br />
     And more<br />
   </div>
 
+line_after_br =
+  <div>
+    Text<br />More text<br />And more<br />
+  </div>
+
+line_after_br =
+  <div>
+    Text
+    <br />
+    More text
+    <br />
+    And more
+    <br />
+  </div>
+
 line_after_br_2 = <div>A<br />B<br />C</div>
 
 br_followed_by_whitespace = <div><br /> text</div>
@@ -315,6 +340,79 @@ x =
   <p>
     text text text text text text text text text text text text text text text<br />text text text text text text
   </p>;
+
+x =
+  <div>
+    <div>
+      First
+    </div>-
+    <div>
+      Second
+    </div>
+  </div>
+
+x =
+  <div>
+    <div>
+      First
+    </div>
+    -
+    <div>
+      Second
+    </div>
+  </div>
+
+x =
+  <div>
+    <div>First</div>-<div>Second</div>
+  </div>
+
+x =
+  <div>
+    <div className="first" tabIndex="1">
+      First
+    </div>-
+    <div className="second" tabIndex="2">
+      Second
+    </div>
+  </div>
+
+x =
+  <div>
+    <div className="first" tabIndex="1">
+      First
+    </div>
+    -
+    <div className="second" tabIndex="2">
+      Second
+    </div>
+  </div>
+
+x =
+  <div>
+    <div className="first" tabIndex="1">First</div>-<div className="second" tabIndex="2">Second</div>
+  </div>
+
+x =
+  <div>
+    {hour}:{minute}:{second}
+  </div>
+
+x =
+  <div>
+    {hour}
+    :
+    {minute}
+    :
+    {second}
+  </div>
+
+x =
+  <div>
+    {hour}:
+    {minute}:
+    {second}
+  </div>
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // Wrapping text
 x = (
@@ -365,18 +463,34 @@ x = (
 
 x = (
   <div>
-    before<div>
+    before
+    <div>
       Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur at
       mollis lorem.
-    </div>after
+    </div>
+    after
   </div>
 );
 
 x = (
   <div>
-    before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}
+    before
     {stuff}
-    {stuff}after{stuff}after
+    after
+    {stuff}
+    after
+    {stuff}
+    after
+    {stuff}
+    after
+    {stuff}
+    after
+    {stuff}
+    {stuff}
+    {stuff}
+    after
+    {stuff}
+    after
   </div>
 );
 
@@ -401,7 +515,8 @@ function DiffOverview(props) {
       <div className="alert alert-info">
         <p>
           This diff overview is computed against the current list of records in
-          this collection and the list it contained on <b>{humanDate(since)}</b>.
+          this collection and the list it contained on <b>{humanDate(since)}</b>
+          .
         </p>
         <p>
           <b>Note:</b> <code>last_modified</code> and <code>schema</code> record
@@ -474,13 +589,44 @@ facebook_translation_leave_text_around_tag = (
   </div>
 );
 
+x = (
+  <div>
+    <span>First second third fourth fifth sixth seventh</span>, (
+    <span>Second</span>)
+  </div>
+);
+
 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>
 );
 
@@ -595,9 +741,34 @@ with_text_fill_line = (
 
 line_after_br = (
   <div>
-    Text<br />
-    More text <br />
-    And more<br />
+    Text
+    <br />
+    More text
+    <br />
+    And more
+    <br />
+  </div>
+);
+
+line_after_br = (
+  <div>
+    Text
+    <br />
+    More text
+    <br />
+    And more
+    <br />
+  </div>
+);
+
+line_after_br = (
+  <div>
+    Text
+    <br />
+    More text
+    <br />
+    And more
+    <br />
   </div>
 );
 
@@ -713,9 +884,82 @@ x = <div> </div>;
 // ----------
 x = (
   <p>
-    text text text text text text text text text text text text text text text<br />text
-    text text text text text
+    text text text text text text text text text text text text text text text
+    <br />
+    text text text text text text
   </p>
 );
 
+x = (
+  <div>
+    <div>First</div>-<div>Second</div>
+  </div>
+);
+
+x = (
+  <div>
+    <div>First</div>-<div>Second</div>
+  </div>
+);
+
+x = (
+  <div>
+    <div>First</div>-<div>Second</div>
+  </div>
+);
+
+x = (
+  <div>
+    <div className="first" tabIndex="1">
+      First
+    </div>
+    -
+    <div className="second" tabIndex="2">
+      Second
+    </div>
+  </div>
+);
+
+x = (
+  <div>
+    <div className="first" tabIndex="1">
+      First
+    </div>
+    -
+    <div className="second" tabIndex="2">
+      Second
+    </div>
+  </div>
+);
+
+x = (
+  <div>
+    <div className="first" tabIndex="1">
+      First
+    </div>
+    -
+    <div className="second" tabIndex="2">
+      Second
+    </div>
+  </div>
+);
+
+x = (
+  <div>
+    {hour}:{minute}:{second}
+  </div>
+);
+
+x = (
+  <div>
+    {hour}:{minute}:{second}
+  </div>
+);
+
+x = (
+  <div>
+    {hour}:{minute}:{second}
+  </div>
+);
+
 `;

tests/jsx-text-wrap/test.js

@@ -113,6 +113,16 @@ facebook_translation_leave_text_around_tag =
     </span>)
   </div>
 
+x =
+  <div>
+    <span>
+      First second third fourth fifth sixth seventh
+    </span>,
+    (<span>
+      Second
+    </span>)
+  </div>
+
 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
@@ -214,10 +224,25 @@ with_text_fill_line =
 line_after_br =
   <div>
     Text<br />
-    More text <br />
+    More text<br />
     And more<br />
   </div>
 
+line_after_br =
+  <div>
+    Text<br />More text<br />And more<br />
+  </div>
+
+line_after_br =
+  <div>
+    Text
+    <br />
+    More text
+    <br />
+    And more
+    <br />
+  </div>
+
 line_after_br_2 = <div>A<br />B<br />C</div>
 
 br_followed_by_whitespace = <div><br /> text</div>
@@ -312,3 +337,76 @@ x =
   <p>
     text text text text text text text text text text text text text text text<br />text text text text text text
   </p>;
+
+x =
+  <div>
+    <div>
+      First
+    </div>-
+    <div>
+      Second
+    </div>
+  </div>
+
+x =
+  <div>
+    <div>
+      First
+    </div>
+    -
+    <div>
+      Second
+    </div>
+  </div>
+
+x =
+  <div>
+    <div>First</div>-<div>Second</div>
+  </div>
+
+x =
+  <div>
+    <div className="first" tabIndex="1">
+      First
+    </div>-
+    <div className="second" tabIndex="2">
+      Second
+    </div>
+  </div>
+
+x =
+  <div>
+    <div className="first" tabIndex="1">
+      First
+    </div>
+    -
+    <div className="second" tabIndex="2">
+      Second
+    </div>
+  </div>
+
+x =
+  <div>
+    <div className="first" tabIndex="1">First</div>-<div className="second" tabIndex="2">Second</div>
+  </div>
+
+x =
+  <div>
+    {hour}:{minute}:{second}
+  </div>
+
+x =
+  <div>
+    {hour}
+    :
+    {minute}
+    :
+    {second}
+  </div>
+
+x =
+  <div>
+    {hour}:
+    {minute}:
+    {second}
+  </div>