-
Notifications
You must be signed in to change notification settings - Fork 469
Preserve JSX #7387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Preserve JSX #7387
Changes from all commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
ffc7666
Add additional node to Pexp_apply
nojaf 92e38cb
Try and pass jsx_element from typed_tree to js_call
nojaf dd56e61
Follow Lprim
nojaf a0c170f
Transform initial simple element
nojaf a98e3b8
Initial fragment support
nojaf 1801da1
WIP extract, good stuff
nojaf ab32462
Print props, catch with key functions
nojaf 6b075c0
Don't pass untyped ast, simple flag is sufficient.
nojaf d0bfedc
Support fragments
nojaf 3d2930e
Unwrap children
nojaf 547dfca
Poor man feature flag
nojaf 09950f1
Remove duplicated in
nojaf 0ab6987
Older camls
nojaf 8e4811b
Revert "Poor man feature flag"
nojaf aa94f6f
Add new -bs-jsx-preserve flag
nojaf e30cab3
WIP, deal with prop spreading
nojaf 773124f
Deal with prop spreading
nojaf 46d3d50
Clean up Lprim and move the information to the call primitive.
cristianoc 1a20f38
Add test for spreading children
nojaf 8929f37
Refactor duplicate code
nojaf cecf67d
Support keyed and prop spreading
nojaf 3fcf1df
Rename test file
nojaf 8b7eb45
Keep key prop before spreading props. Also ensure test can run.
nojaf 90b28f0
Add only spread props case
nojaf faa5618
Give record a name
nojaf 400b550
Use config flag in Js_dump instead
nojaf 3e1867e
Remove helper code
nojaf 32bffd8
Extra call info
nojaf f104529
Detect direct Array as well
nojaf 848b3ab
Don't run with mocha
nojaf aceb0e2
Feedback code review
nojaf File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -515,7 +515,7 @@ and expression_desc cxt ~(level : int) f x : cxt = | |
(* TODO: dump for comments *) | ||
pp_function ?directive ~is_method ~return_unit ~async | ||
~fn_state:default_fn_exp_state cxt f params body env | ||
(* TODO: | ||
(* TODO: | ||
when [e] is [Js_raw_code] with arity | ||
print it in a more precise way | ||
It seems the optimizer already did work to make sure | ||
|
@@ -524,6 +524,116 @@ and expression_desc cxt ~(level : int) f x : cxt = | |
when Ext_list.length_equal el i | ||
]} | ||
*) | ||
(* When -bs-preserve-jsx is enabled, we marked each transformed application node throughout the compilation. | ||
Here we print the transformed application node into a JSX syntax. | ||
The JSX is slightly different from what a user would write, | ||
but it is still valid JSX and is usable by tools like ESBuild. | ||
*) | ||
| Call | ||
( ({ | ||
expression_desc = | ||
J.Var | ||
(J.Qualified | ||
( _, | ||
Some fnName | ||
(* We care about the function name when it is jsxs, | ||
If this is the case, we need to unpack an array later on *) | ||
)); | ||
} as e), | ||
el, | ||
{call_transformed_jsx = true} ) | ||
when !Js_config.jsx_preserve -> ( | ||
(* We match a JsxRuntime.jsx call *) | ||
match el with | ||
| [ | ||
tag; | ||
{ | ||
expression_desc = | ||
(* This is the props javascript object *) | ||
Caml_block (el, _mutable_flag, _, Lambda.Blk_record {fields}); | ||
}; | ||
] -> | ||
(* We extract the props from the javascript object *) | ||
let fields = | ||
Ext_list.array_list_filter_map fields el (fun (f, opt) x -> | ||
match x.expression_desc with | ||
| Undefined _ when opt -> None | ||
| _ -> Some (f, x)) | ||
in | ||
print_jsx cxt ~level f fnName tag fields | ||
| [ | ||
tag; | ||
{ | ||
expression_desc = | ||
Caml_block (el, _mutable_flag, _, Lambda.Blk_record {fields}); | ||
}; | ||
key; | ||
] -> | ||
(* When a component has a key the matching runtime function call will have a third argument being the key *) | ||
let fields = | ||
Ext_list.array_list_filter_map fields el (fun (f, opt) x -> | ||
match x.expression_desc with | ||
| Undefined _ when opt -> None | ||
| _ -> Some (f, x)) | ||
in | ||
print_jsx cxt ~level ~key f fnName tag fields | ||
| [tag; ({expression_desc = J.Seq _} as props)] -> | ||
(* In the case of prop spreading, the expression will look like: | ||
(props.a = "Hello, world!", props) | ||
which is equivalent to | ||
<tag {...props} a="Hello, world!" /> | ||
|
||
We need to extract the props and the spread object. | ||
*) | ||
let fields, spread_props = | ||
let rec visit acc e = | ||
match e.J.expression_desc with | ||
| J.Seq | ||
( { | ||
J.expression_desc = | ||
J.Bin | ||
( Js_op.Eq, | ||
{J.expression_desc = J.Static_index (_, name, _)}, | ||
value ); | ||
}, | ||
rest ) -> | ||
visit ((name, value) :: acc) rest | ||
| _ -> (List.rev acc, e) | ||
in | ||
visit [] props | ||
in | ||
print_jsx cxt ~level ~spread_props f fnName tag fields | ||
| [tag; ({expression_desc = J.Seq _} as props); key] -> | ||
(* In the case of props + prop spreading and key argument *) | ||
let fields, spread_props = | ||
let rec visit acc e = | ||
match e.J.expression_desc with | ||
| J.Seq | ||
( { | ||
J.expression_desc = | ||
J.Bin | ||
( Js_op.Eq, | ||
{J.expression_desc = J.Static_index (_, name, _)}, | ||
value ); | ||
}, | ||
rest ) -> | ||
visit ((name, value) :: acc) rest | ||
| _ -> (List.rev acc, e) | ||
in | ||
visit [] props | ||
in | ||
print_jsx cxt ~level ~spread_props ~key f fnName tag fields | ||
| [tag; ({expression_desc = J.Var _} as spread_props)] -> | ||
(* All the props are spread *) | ||
print_jsx cxt ~level ~spread_props f fnName tag [] | ||
| _ -> | ||
(* This should not happen, we fallback to the general case *) | ||
expression_desc cxt ~level f | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this code branch ever executed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it is, it means we missed something. Could we add some sort of assert in this case? |
||
(Call | ||
( e, | ||
el, | ||
{call_transformed_jsx = false; arity = Full; call_info = Call_ml} | ||
))) | ||
| Call (e, el, info) -> | ||
P.cond_paren_group f (level > 15) (fun _ -> | ||
P.group f 0 (fun _ -> | ||
|
@@ -956,6 +1066,103 @@ and expression_desc cxt ~(level : int) f x : cxt = | |
P.string f "..."; | ||
expression ~level:13 cxt f e) | ||
|
||
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 = | ||
let print_tag cxt = | ||
match tag.expression_desc with | ||
(* "div" or any other primitive tag *) | ||
| J.Str {txt} -> | ||
P.string f txt; | ||
cxt | ||
(* fragment *) | ||
| J.Var (J.Qualified ({id = {name = "JsxRuntime"}}, Some "Fragment")) -> cxt | ||
(* A user defined component or external component *) | ||
| _ -> expression ~level cxt f tag | ||
in | ||
let children_opt = | ||
List.find_map | ||
(fun (n, e) -> | ||
if n = "children" then | ||
if fnName = "jsxs" then | ||
match e.J.expression_desc with | ||
| J.Array (xs, _) | ||
| J.Optional_block ({expression_desc = J.Array (xs, _)}, _) -> | ||
Some xs | ||
| _ -> Some [e] | ||
else Some [e] | ||
else None) | ||
fields | ||
in | ||
let print_props cxt = | ||
(* 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 -> | ||
P.string f " key={"; | ||
let cxt = expression ~level:0 cxt f key in | ||
P.string f "} "; | ||
cxt | ||
in | ||
let props = List.filter (fun (n, _) -> n <> "children") fields 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 | ||
in | ||
if List.length props = 0 then cxt | ||
else | ||
(List.fold_left (fun acc (n, x) -> | ||
P.space f; | ||
P.string f n; | ||
P.string f "="; | ||
P.string f "{"; | ||
let next = expression ~level:0 acc f x in | ||
P.string f "}"; | ||
next)) | ||
cxt props | ||
in | ||
match children_opt with | ||
| None -> | ||
P.string f "<"; | ||
let cxt = cxt |> print_tag |> print_props in | ||
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 | ||
in | ||
|
||
P.string f "</"; | ||
let cxt = print_tag cxt in | ||
P.string f ">"; | ||
cxt | ||
|
||
and property_name_and_value_list cxt f (l : J.property_map) = | ||
iter_lst cxt f l | ||
(fun cxt f (pn, e) -> | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.