From 831eaa93337cfedd045a6bbc6092f95504264824 Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 7 May 2025 17:52:28 +0200 Subject: [PATCH 1/5] Indent children --- compiler/core/js_dump.ml | 68 ++++++++++++++++++++++++++++++---------- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/compiler/core/js_dump.ml b/compiler/core/js_dump.ml index 19488c3fc3..9e3558c985 100644 --- a/compiler/core/js_dump.ml +++ b/compiler/core/js_dump.ml @@ -1070,6 +1070,34 @@ and expression_desc cxt ~(level : int) f x : cxt = P.string f "..."; expression ~level:13 cxt f e) +and print_indented_list (f : P.t) (parent_expr_level : int) (cxt : cxt) + (items : 'a list) (print_item_func : int -> cxt -> P.t -> 'a -> cxt) : cxt = + if List.length items = 0 then cxt + else + P.group f 1 (fun () -> + (* Increment indent level by 1 for this block of items *) + P.newline f; + (* Start the block on a new, fully indented line for the first item *) + let rec process_items current_cxt_for_fold remaining_items = + match remaining_items with + | [] -> + current_cxt_for_fold + (* Base case for recursion, though initial check avoids empty items *) + | [last_item] -> + (* Print the last item, but DO NOT print a newline after it *) + print_item_func parent_expr_level current_cxt_for_fold f last_item + | current_item :: next_items -> + let cxt_after_current = + print_item_func parent_expr_level current_cxt_for_fold f + current_item + in + P.newline f; + (* Add a newline AFTER the current item, to prepare for the NEXT item *) + process_items cxt_after_current next_items + in + (* Initial call to the recursive helper; initial check ensures items is not empty *) + process_items cxt items) + and print_jsx cxt ?(spread_props : J.expression option) ?(key : J.expression option) ~(level : int) f (fnName : string) (tag : J.expression) (fields : (string * J.expression) list) : cxt = @@ -1134,6 +1162,23 @@ and print_jsx cxt ?(spread_props : J.expression option) next)) cxt props in + + let print_one_child expr_level_for_child current_cxt_for_child f_format + child_expr = + let child_is_jsx_itself = + match child_expr.J.expression_desc with + | J.Call (_, _, {call_transformed_jsx = is_jsx}) -> is_jsx + | _ -> false + in + if not child_is_jsx_itself then P.string f_format "{"; + let next_cxt = + expression ~level:expr_level_for_child current_cxt_for_child f_format + child_expr + in + if not child_is_jsx_itself then P.string f_format "}"; + next_cxt + in + match children_opt with | None -> P.string f "<"; @@ -1141,29 +1186,18 @@ and print_jsx cxt ?(spread_props : J.expression option) P.string f "/>"; cxt | Some children -> - let child_is_jsx child = - match child.J.expression_desc with - | J.Call (_, _, {call_transformed_jsx = is_jsx}) -> is_jsx - | _ -> false - in - P.string f "<"; let cxt = cxt |> print_tag |> print_props in - P.string f ">"; - if List.length children > 0 then P.newline f; - let cxt = - List.fold_left - (fun acc e -> - if not (child_is_jsx e) then P.string f "{"; - let next = expression ~level acc f e in - if not (child_is_jsx e) then P.string f "}"; - P.newline f; - next) - cxt children + let cxt_after_children = + print_indented_list f level cxt children print_one_child in + let cxt = cxt_after_children in + + P.newline f; + (* Newline before the closing tag, uses parent's indent level *) P.string f ""; From 6ce7b1d22df97ec8cad13d73047e4f3510fb7f07 Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 7 May 2025 18:08:12 +0200 Subject: [PATCH 2/5] Print props on different lines --- compiler/core/js_dump.ml | 69 +++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 22 deletions(-) diff --git a/compiler/core/js_dump.ml b/compiler/core/js_dump.ml index 9e3558c985..0f934a356c 100644 --- a/compiler/core/js_dump.ml +++ b/compiler/core/js_dump.ml @@ -1126,19 +1126,19 @@ and print_jsx cxt ?(spread_props : J.expression option) else None) fields in - let print_props cxt = + let print_props cxt props = (* If a key is present, should be printed before the spread props, This is to ensure tools like ESBuild use the automatic JSX runtime *) let cxt = match key with | None -> cxt - | Some key -> + | Some k -> P.string f " key={"; - let cxt = expression ~level:0 cxt f key in + let cxt_k = expression ~level:0 cxt f k in P.string f "} "; - cxt + cxt_k in - let props = List.filter (fun (n, _) -> n <> "children") fields in + let cxt = match spread_props with | None -> cxt @@ -1150,17 +1150,32 @@ and print_jsx cxt ?(spread_props : J.expression option) in if List.length props = 0 then cxt else - (List.fold_left (fun acc (n, x) -> - P.space f; - let prop_name = Js_dump_property.property_key_string n in - - P.string f prop_name; - P.string f "="; - P.string f "{"; - let next = expression ~level:0 acc f x in - P.string f "}"; - next)) - cxt props + P.group f 1 (fun () -> + P.newline f; + let rec process_remaining_props acc_cxt props_to_process = + match props_to_process with + | [] -> acc_cxt + | (n, x) :: [] -> + let prop_name = Js_dump_property.property_key_string n in + P.string f prop_name; + P.string f "="; + P.string f "{"; + let next_cxt = expression ~level:0 acc_cxt f x in + P.string f "}"; + next_cxt + | (n, x) :: tail -> + let prop_name = Js_dump_property.property_key_string n in + P.string f prop_name; + P.string f "="; + P.string f "{"; + let cxt_after_current_prop_value = + expression ~level:0 acc_cxt f x + in + P.string f "}"; + P.newline f; + process_remaining_props cxt_after_current_prop_value tail + in + process_remaining_props cxt props) in let print_one_child expr_level_for_child current_cxt_for_child f_format @@ -1179,15 +1194,26 @@ and print_jsx cxt ?(spread_props : J.expression option) next_cxt in + let props = List.filter (fun (n, _) -> n <> "children") fields in + + (* Actual printing of JSX element starts here *) + P.string f "<"; + let cxt = print_tag cxt in + let cxt = print_props cxt props in + (* print_props handles its own block and updates cxt *) + + let has_multiple_props = List.length props > 0 in + match children_opt with | None -> - P.string f "<"; - let cxt = cxt |> print_tag |> print_props in + (* Self-closing tag *) + if has_multiple_props then P.newline f; P.string f "/>"; cxt | Some children -> - P.string f "<"; - let cxt = cxt |> print_tag |> print_props in + (* Tag with children *) + let has_children = List.length children > 0 in + if has_multiple_props || has_children then P.newline f; P.string f ">"; let cxt_after_children = @@ -1196,8 +1222,7 @@ and print_jsx cxt ?(spread_props : J.expression option) let cxt = cxt_after_children in P.newline f; - - (* Newline before the closing tag, uses parent's indent level *) + (* For closing *) P.string f ""; From 5715bc2d51fdc85663087b5619f7515e9a5e5d85 Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 7 May 2025 18:20:20 +0200 Subject: [PATCH 3/5] Improve opening tag when no props are present --- compiler/core/js_dump.ml | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/compiler/core/js_dump.ml b/compiler/core/js_dump.ml index 0f934a356c..51affc0b54 100644 --- a/compiler/core/js_dump.ml +++ b/compiler/core/js_dump.ml @@ -1213,16 +1213,25 @@ and print_jsx cxt ?(spread_props : J.expression option) | Some children -> (* Tag with children *) let has_children = List.length children > 0 in - if has_multiple_props || has_children then P.newline f; + (* Newline for ">" only if props themselves were multi-line. Children alone don't push ">" to a new line. *) + if has_multiple_props then P.newline f; P.string f ">"; let cxt_after_children = - print_indented_list f level cxt children print_one_child + if has_children then + (* Only call print_indented_list if there are children *) + print_indented_list f level cxt children print_one_child + else cxt + (* No children, no change to context here, no newlines from children block *) in let cxt = cxt_after_children in - P.newline f; - (* For closing *) + (* The closing "" goes on a new line if the opening part was multi-line (due to props) + OR if there were actual children printed (which always makes the element multi-line). + *) + let element_content_was_multiline = has_multiple_props || has_children in + if element_content_was_multiline then P.newline f; + P.string f ""; From a3d412510cf41524c526439ce34ed0d309cf26b7 Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 7 May 2025 18:21:15 +0200 Subject: [PATCH 4/5] Add additional test --- tests/tests/src/preserve_jsx_test.mjs | 129 +++++++++++++++++++------- tests/tests/src/preserve_jsx_test.res | 11 +++ 2 files changed, 107 insertions(+), 33 deletions(-) diff --git a/tests/tests/src/preserve_jsx_test.mjs b/tests/tests/src/preserve_jsx_test.mjs index 8af8542ac1..013c9e5404 100644 --- a/tests/tests/src/preserve_jsx_test.mjs +++ b/tests/tests/src/preserve_jsx_test.mjs @@ -16,31 +16,41 @@ let Icon = { }; let _single_element_child =
-

-{"Hello, world!"} -

+

+ {"Hello, world!"} +

; let _multiple_element_children =
-

-{"Hello, world!"} -

- +

+ {"Hello, world!"} +

+
; let _single_element_fragment = <> -{Primitive_option.some()} + {Primitive_option.some()} ; let _multiple_element_fragment = <> - - + + ; -let _unary_element_with_props = ; +let _unary_element_with_props = ; -let _container_element_with_props_and_children =
-{"Hello, world!"} +let _container_element_with_props_and_children =
+ {"Hello, world!"}
; let baseProps = { @@ -50,43 +60,63 @@ let baseProps = { let newrecord = {...baseProps}; -let _unary_element_with_spread_props = ; +let _unary_element_with_spread_props = ; let newrecord$1 = {...baseProps}; -let _container_with_spread_props =
-{"Hello, world!"} - +let _container_with_spread_props =
+ {"Hello, world!"} +
; let baseChildren = [ - {"Hello, world!"} + {"Hello, world!"} , - {"Hello, world!"} + {"Hello, world!"} ]; -let _container_with_spread_children =
-{baseChildren} +let _container_with_spread_children =
+ {baseChildren}
; let newrecord$2 = {...baseProps}; -let _container_with_spread_props_and_children =
-{baseChildren} +let _container_with_spread_props_and_children =
+ {baseChildren}
; let newrecord$3 = {...baseProps}; -let _unary_element_with_spread_props_keyed = ; +let _unary_element_with_spread_props_keyed = ; let newrecord$4 = {...baseProps}; -let _container_with_spread_props_keyed =
-{"Hello, world!"} - +let _container_with_spread_props_keyed =
+ {"Hello, world!"} +
; let _unary_element_with_only_spread_props = ; @@ -98,7 +128,7 @@ let A = {}; function Preserve_jsx_test$B(props) { return

- {"Hello, world!"} + {"Hello, world!"}

; } @@ -107,14 +137,14 @@ let B = { }; let _external_component_with_children = - - + + ; function make(props) { return

- {"foo"} - {props["\\\"MyWeirdProp\""]} + {"foo"} + {props["\\\"MyWeirdProp\""]}

; } @@ -122,7 +152,39 @@ let MyWeirdComponent = { make: make }; -let _escaped_jsx_prop = ; +let _escaped_jsx_prop = ; + +let _large_component =
{}} + onMouseDown={param => {}} +> +

{}} + onMouseDown={param => {}} + > + {"Hello, world!"} +

+ {}} + onMouseDown={param => {}} + > + {"Hello, world!"} + +

+ {5} +

+
; export { React, @@ -148,5 +210,6 @@ export { _external_component_with_children, MyWeirdComponent, _escaped_jsx_prop, + _large_component, } /* _single_element_child Not a pure module */ diff --git a/tests/tests/src/preserve_jsx_test.res b/tests/tests/src/preserve_jsx_test.res index de98a5f38a..df29ccbf7c 100644 --- a/tests/tests/src/preserve_jsx_test.res +++ b/tests/tests/src/preserve_jsx_test.res @@ -167,3 +167,14 @@ module MyWeirdComponent = { } let _escaped_jsx_prop = + +let _large_component = +
()} onMouseDown={_ => ()}> +

()} onMouseDown={_ => ()}> + {React.string("Hello, world!")} +

+ ()} onMouseDown={_ => ()}> + {React.string("Hello, world!")} + +

{React.int(5)}

+
From c0fef4804da1818738de73f9be70bc946c0e4150 Mon Sep 17 00:00:00 2001 From: nojaf Date: Thu, 8 May 2025 10:27:27 +0200 Subject: [PATCH 5/5] Improve consistency of prop printing --- compiler/core/js_dump.ml | 81 ++++++++++++++------------- tests/tests/src/preserve_jsx_test.mjs | 30 ++++++---- 2 files changed, 62 insertions(+), 49 deletions(-) diff --git a/compiler/core/js_dump.ml b/compiler/core/js_dump.ml index 51affc0b54..65f1b06a8e 100644 --- a/compiler/core/js_dump.ml +++ b/compiler/core/js_dump.ml @@ -1129,53 +1129,58 @@ and print_jsx cxt ?(spread_props : J.expression option) let print_props cxt props = (* If a key is present, should be printed before the spread props, This is to ensure tools like ESBuild use the automatic JSX runtime *) - let cxt = - match key with - | None -> cxt - | Some k -> - P.string f " key={"; - let cxt_k = expression ~level:0 cxt f k in - P.string f "} "; - cxt_k + let print_key key cxt = + P.string f "key={"; + let cxt_k = expression ~level:0 cxt f key in + P.string f "} "; + cxt_k in - let cxt = - match spread_props with - | None -> cxt - | Some spread -> - P.string f " {..."; - let cxt = expression ~level:0 cxt f spread in - P.string f "} "; - cxt + let print_spread_props spread cxt = + P.string f "{..."; + let cxt = expression ~level:0 cxt f spread in + P.string f "} "; + cxt + in + + let print_prop n x ctx = + let prop_name = Js_dump_property.property_key_string n in + P.string f prop_name; + P.string f "="; + P.string f "{"; + let next_cxt = expression ~level:0 ctx f x in + P.string f "}"; + next_cxt + in + let printable_props = + (match key with + | None -> [] + | Some k -> [print_key k]) + @ (match spread_props with + | None -> [] + | Some spread -> [print_spread_props spread]) + @ List.map (fun (n, x) -> print_prop n x) props in - if List.length props = 0 then cxt + if List.length printable_props = 0 then ( + match children_opt with + | Some _ -> cxt + | None -> + (* Put a space the tag name and /> *) + P.space f; + cxt) else P.group f 1 (fun () -> P.newline f; - let rec process_remaining_props acc_cxt props_to_process = - match props_to_process with + let rec process_remaining_props acc_cxt printable_props = + match printable_props with | [] -> acc_cxt - | (n, x) :: [] -> - let prop_name = Js_dump_property.property_key_string n in - P.string f prop_name; - P.string f "="; - P.string f "{"; - let next_cxt = expression ~level:0 acc_cxt f x in - P.string f "}"; - next_cxt - | (n, x) :: tail -> - let prop_name = Js_dump_property.property_key_string n in - P.string f prop_name; - P.string f "="; - P.string f "{"; - let cxt_after_current_prop_value = - expression ~level:0 acc_cxt f x - in - P.string f "}"; + | print_prop :: [] -> print_prop acc_cxt + | print_prop :: tail -> + let next_cxt = print_prop acc_cxt in P.newline f; - process_remaining_props cxt_after_current_prop_value tail + process_remaining_props next_cxt tail in - process_remaining_props cxt props) + process_remaining_props cxt printable_props) in let print_one_child expr_level_for_child current_cxt_for_child f_format diff --git a/tests/tests/src/preserve_jsx_test.mjs b/tests/tests/src/preserve_jsx_test.mjs index c5b80279f9..6e8a134c72 100644 --- a/tests/tests/src/preserve_jsx_test.mjs +++ b/tests/tests/src/preserve_jsx_test.mjs @@ -8,7 +8,7 @@ let React = {}; let ReactDOM = {}; function Preserve_jsx_test$Icon(props) { - return ; + return ; } let Icon = { @@ -25,11 +25,11 @@ let _multiple_element_children =

{"Hello, world!"}

- +
; let _single_element_fragment = <> - {Primitive_option.some()} + {Primitive_option.some()} ; let _multiple_element_fragment = <> @@ -60,13 +60,15 @@ let baseProps = { let newrecord = {...baseProps}; -let _unary_element_with_spread_props = ; let newrecord$1 = {...baseProps}; -let _container_with_spread_props =
@@ -94,7 +96,8 @@ let _container_with_spread_children =
@@ -103,13 +106,17 @@ let _container_with_spread_props_and_children =
; let newrecord$4 = {...baseProps}; -let _container_with_spread_props_keyed =
@@ -119,7 +126,8 @@ let _container_with_spread_props_keyed =
; -let _unary_element_with_only_spread_props = ; +let _unary_element_with_only_spread_props = ; function QueryClientProvider(props) { return props.children } ; @@ -137,8 +145,8 @@ let B = { }; let _external_component_with_children = - - + + ; function Preserve_jsx_test$MyWeirdComponent(props) {