Put each jsx tag on own line (#574)

* Put each JSX tag on its own line

* Put each JSX tag nested in fragment on its own line

* Refactor code to be more DRY

* Avoid list allocations while looking for nested JSX exprs

* Simplify “has nested jsx” check, do not recurse in non-list child exprs

* Bring back recursion while looking for nested JSX, add more tests

* Minor code styling

* Put JSX children on own lines when more than two found

* Simplify and more precise name for nested jsx search function

Author: nkrkv

src/res_printer.ml

@@ -99,6 +99,18 @@ let hasCommentBelow tbl loc =
   | [] -> false
   | exception Not_found -> false
 
+let hasNestedJsxOrMoreThanOneChild expr =
+  let rec loop inRecursion expr =
+    match expr.Parsetree.pexp_desc with
+    | Pexp_construct
+        ({txt = Longident.Lident "::"}, Some {pexp_desc = Pexp_tuple [hd; tail]})
+      ->
+      if inRecursion || ParsetreeViewer.isJsxExpression hd then true
+      else loop true tail
+    | _ -> false
+  in
+  loop false expr
+
 let printMultilineCommentContent txt =
   (* Turns
    *         |* first line
@@ -3694,6 +3706,12 @@ and printJsxExpression lident args cmtTbl =
       true
     | _ -> false
   in
+  let lineSep =
+    match children with
+    | Some expr ->
+      if hasNestedJsxOrMoreThanOneChild expr then Doc.hardLine else Doc.line
+    | None -> Doc.line
+  in
   Doc.group
     (Doc.concat
        [
@@ -3728,10 +3746,10 @@ and printJsxExpression lident args cmtTbl =
                       Doc.line;
                       (match children with
                       | Some childrenExpression ->
-                        printJsxChildren childrenExpression cmtTbl
+                        printJsxChildren childrenExpression ~sep:lineSep cmtTbl
                       | None -> Doc.nil);
                     ]);
-               Doc.line;
+               lineSep;
                Doc.text "</";
                name;
                Doc.greaterThan;
@@ -3741,24 +3759,28 @@ and printJsxExpression lident args cmtTbl =
 and printJsxFragment expr cmtTbl =
   let opening = Doc.text "<>" in
   let closing = Doc.text "</>" in
-  (* let (children, _) = ParsetreeViewer.collectListExpressions expr in *)
+  let lineSep =
+    if hasNestedJsxOrMoreThanOneChild expr then Doc.hardLine else Doc.line
+  in
   Doc.group
     (Doc.concat
        [
          opening;
          (match expr.pexp_desc with
          | Pexp_construct ({txt = Longident.Lident "[]"}, None) -> Doc.nil
-         | _ -> Doc.indent (Doc.concat [Doc.line; printJsxChildren expr cmtTbl]));
-         Doc.line;
+         | _ ->
+           Doc.indent
+             (Doc.concat [Doc.line; printJsxChildren expr ~sep:lineSep cmtTbl]));
+         lineSep;
          closing;
        ])
 
-and printJsxChildren (childrenExpr : Parsetree.expression) cmtTbl =
+and printJsxChildren (childrenExpr : Parsetree.expression) ~sep cmtTbl =
   match childrenExpr.pexp_desc with
   | Pexp_construct ({txt = Longident.Lident "::"}, _) ->
     let children, _ = ParsetreeViewer.collectListExpressions childrenExpr in
     Doc.group
-      (Doc.join ~sep:Doc.line
+      (Doc.join ~sep
          (List.map
             (fun (expr : Parsetree.expression) ->
               let leadingLineCommentPresent =

tests/printer/expr/expected/callback.res.txt

@@ -230,7 +230,9 @@ let decoratorTags =
   items
   ->Js.Array2.filter(items => {items.category === Decorators})
   ->Belt.Array.map(item => {
-    <span className="mr-2" key=item.name> <Tag text={item.name} /> </span>
+    <span className="mr-2" key=item.name>
+      <Tag text={item.name} />
+    </span>
   })
 
 // Comments should still be printed here (callback in last position)

tests/printer/expr/expected/exoticIdent.res.txt

@@ -61,4 +61,8 @@ lazy \"let"
 @let
 let x = 1
 
-let x = <div \"aria-foo"=\"type"> \"module" \"let" </div>
+let x =
+  <div \"aria-foo"=\"type">
+    \"module"
+    \"let"
+  </div>

tests/printer/expr/expected/jsx.res.txt

@@ -5,6 +5,37 @@ let x = <Foo.Bar.Baz className="container" />
 let x = <Foo.bar className="container" />
 let x = <Foo.baz className="multiline" />
 
+// https://github.com/rescript-lang/syntax/issues/570
+let x =
+  <A>
+    <B>
+      <C>
+        <D />
+        <E />
+      </C>
+      <F>
+        <G />
+        <H />
+      </F>
+    </B>
+  </A>
+let x =
+  <A>
+    {children}
+    <B />
+  </A>
+let x =
+  <A>
+    <B />
+    {children}
+  </A>
+let x = <A> {a} </A>
+let x =
+  <A>
+    {a}
+    {b}
+  </A>
+
 let x = <div className="container" className2="container2" className3="container3" onClick />
 
 let nav =
@@ -58,7 +89,9 @@ let nav3 =
 
 let avatarSection =
   <>
-    <div style={{"zIndex": "1", "opacity": opacityUser}}> <Avatar user={user} size={45} /> </div>
+    <div style={{"zIndex": "1", "opacity": opacityUser}}>
+      <Avatar user={user} size={45} />
+    </div>
     {user.email !== viewer.email
       ? <div
           style={{
@@ -258,8 +291,14 @@ let x = <MyComponent sidebar={<div> test </div>} nav={<Navbar />} />
 
 // https://github.com/rescript-lang/syntax/issues/113
 <div> {Js.log(a <= 10)} </div>
-<div> <div> {Js.log(a <= 10)} </div> </div>
-<div> <div onClick={_ => Js.log(a <= 10)}> <div> {Js.log(a <= 10)} </div> </div> </div>
+<div>
+  <div> {Js.log(a <= 10)} </div>
+</div>
+<div>
+  <div onClick={_ => Js.log(a <= 10)}>
+    <div> {Js.log(a <= 10)} </div>
+  </div>
+</div>
 
 module App = {
   @react.component
@@ -271,7 +310,9 @@ module App = {
 module App = {
   @react.component
   let make = () => {
-    <> <div /> </>
+    <>
+      <div />
+    </>
   }
 }
 

tests/printer/expr/jsx.res

@@ -7,6 +7,12 @@ let x = <Foo.baz
  className="multiline"
 />
 
+// https://github.com/rescript-lang/syntax/issues/570
+let x = <A> <B> <C> <D /> <E /> </C> <F> <G /> <H /> </F> </B> </A>
+let x = <A> {children} <B/> </A>
+let x = <A> <B/> {children} </A>
+let x = <A> {a} </A>
+let x = <A> {a} {b} </A>
 
 let x =
   <div

tests/printer/other/expected/fatSlider.res.txt

@@ -50,7 +50,8 @@ let make = (~min=50, ~max=250, ~meterSuffix=?) => {
       />
     </div>
     <div className="meter text-4xl font-bold text-gray-600 mt-4">
-      {values[0]->Js.Int.toString->string} {meterSuffix->Belt.Option.getWithDefault(null)}
+      {values[0]->Js.Int.toString->string}
+      {meterSuffix->Belt.Option.getWithDefault(null)}
     </div>
   </div>
 }

tests/printer/pattern/expected/constant.res.txt

@@ -33,7 +33,9 @@ switch science {
   | None => React.null
   | Some(1.0) => React.null
   | Some(uploadProgress) =>
-    <div className=Styles.fill> <StudioUploadProgressIndicator progress=uploadProgress /> </div>
+    <div className=Styles.fill>
+      <StudioUploadProgressIndicator progress=uploadProgress />
+    </div>
   }}
 </div>