Merge pull request #89 from ml-in-barcelona/fix-83

Remove filtering of optional props in make_js_props_obj
ml-in-barcelona · Dec 20, 2021 · d7290d3 · d7290d3
2 parents 426aac3 + 5dbb256
commit d7290d3
Show file tree

Hide file tree

Showing 7 changed files with 224 additions and 146 deletions.
diff --git a/Makefile b/Makefile
@@ -26,7 +26,7 @@ test: ## Run the unit tests
 	$(DUNE) build @runtest
 
 test-promote: ## Updates snapshots and promotes it to correct
-	$(DUNE) build @runtest
+	$(DUNE) build @runtest --auto-promote
 
 # This is a separate command to run this tests conditionaly on CI, currently the dune setup with (bash "! ./%{main} %{input}")) fails in Windows. We skip it on Windows.
 test-error-msg: ## Run the unit tests for error messages

diff --git a/ppx/ppx.ml b/ppx/ppx.ml
@@ -542,36 +542,31 @@ let makeValue ~loc ~isOptional prop value =
   | Html.Event event ->
       makeEventValue ~loc ~isOptional event.type_ value
 
-let makeJsObj ~loc named_arg_list_with_key_and_ref =
-  let open Str_label in
-  let labelToTuple label =
-    let l = str label in
+let make_js_props_obj ~loc named_arg_list_with_key_and_ref =
+  let label_to_tuple label =
+    let l = Str_label.str label in
     let id = Exp.ident ~loc {txt= Lident l; loc} in
-    let make_tuple raw =
-      match l = "key" with
-      | true ->
-          [%expr
-            [%e Exp.constant ~loc (Const.string l)]
-            , inject (Js_of_ocaml.Js.string [%e raw])]
-      | false ->
-          [%expr [%e Exp.constant ~loc (Const.string l)], inject [%e raw]]
-    in
-    match label with
-    | Optional _ ->
-        [%expr Option.map (fun raw -> [%e make_tuple [%expr raw]]) [%e id]]
-    | Labelled _ ->
-        [%expr Some [%e make_tuple id]]
+    match l with
+    | "key" ->
+        [%expr
+          [%e Exp.constant ~loc (Const.string l)]
+          , inject
+              (Js_of_ocaml.Js.Optdef.option
+                 (Option.map Js_of_ocaml.Js.string [%e id]) )]
+    | "ref" ->
+        [%expr
+          [%e Exp.constant ~loc (Const.string l)]
+          , inject (Js_of_ocaml.Js.Optdef.option [%e id])]
+    | l ->
+        [%expr [%e Exp.constant ~loc (Const.string l)], inject [%e id]]
   in
   [%expr
     obj
-      ( [%e
-          Exp.array ~loc
-            (List.map
-               (fun (label, _, _, _) -> labelToTuple label)
-               named_arg_list_with_key_and_ref )]
-      |> Array.to_list
-      |> List.filter_map (fun x -> x)
-      |> Array.of_list )]
+      [%e
+        Exp.array ~loc
+          (List.map
+             (fun (label, _, _, _) -> label_to_tuple label)
+             named_arg_list_with_key_and_ref )]]
 
 (* Builds the function that takes labelled arguments and generates a JS object *)
 let make_make_props fn_name loc named_arg_list props_type rest =
@@ -597,7 +592,7 @@ let make_make_props fn_name loc named_arg_list props_type rest =
                        [%expr
                          fun () ->
                            let open Js_of_ocaml.Js.Unsafe in
-                           [%e makeJsObj ~loc named_arg_list]]
+                           [%e make_js_props_obj ~loc named_arg_list]]
                    , core_type ) ) ) ]
        , rest ) )
 

diff --git a/ppx/test/input_ocaml.ml b/ppx/test/input_ocaml.ml
@@ -15,3 +15,5 @@ let%component make ~children:kids = div ~id:"foo" ~children:kids ()
 
 let%component make ~children:(first, second) () =
   div ~children:[first; second] ()
+
+let%component make ?(name = "") = div ~children:[name] ()
diff --git a/ppx/test/input_reason.re b/ppx/test/input_reason.re
@@ -177,3 +177,8 @@ let randomElement = <text dx="1 2" dy="3 4" />;
 let make = (~name, ~isDisabled=?, ~onClick=?) => {
   <button name ?onClick disabled=isDisabled />;
 };
+
+[@react.component]
+let make = (~name="joe") => {
+  <div> {Printf.sprintf("`name` is %s", name) |> React.string} </div>;
+};
diff --git a/ppx/test/pp_ocaml.expected b/ppx/test/pp_ocaml.expected
@@ -11,13 +11,13 @@ let make =
         fun () ->
           let open Js_of_ocaml.Js.Unsafe in
             obj
-              ((([|(Option.map
-                      (fun raw ->
-                         ("key", (inject (Js_of_ocaml.Js.string raw)))) key);(
-                   Option.map (fun raw -> ("name", (inject raw))) name)|] |>
-                   Array.to_list)
-                  |> (List.filter_map (fun x -> x)))
-                 |> Array.of_list) in
+              [|("key",
+                  (inject
+                     (Js_of_ocaml.Js.Optdef.option
+                        (Option.map Js_of_ocaml.Js.string key))));("name",
+                                                                    (
+                                                                    inject
+                                                                    name))|] in
   let make =
     ((fun ?(name= "") ->
         React.Dom.createDOMElementVariadic "div"
@@ -67,12 +67,13 @@ let make =
         fun () ->
           let open Js_of_ocaml.Js.Unsafe in
             obj
-              ((([|(Option.map
-                      (fun raw ->
-                         ("key", (inject (Js_of_ocaml.Js.string raw)))) key);(
-                   Some ("children", (inject children)))|] |> Array.to_list)
-                  |> (List.filter_map (fun x -> x)))
-                 |> Array.of_list) in
+              [|("key",
+                  (inject
+                     (Js_of_ocaml.Js.Optdef.option
+                        (Option.map Js_of_ocaml.Js.string key))));("children",
+                                                                    (
+                                                                    inject
+                                                                    children))|] in
   let make =
     ((fun ~children:(first, second) ->
         React.Dom.createDOMElementVariadic "div"
@@ -112,12 +113,13 @@ let make =
         fun () ->
           let open Js_of_ocaml.Js.Unsafe in
             obj
-              ((([|(Option.map
-                      (fun raw ->
-                         ("key", (inject (Js_of_ocaml.Js.string raw)))) key);(
-                   Some ("children", (inject children)))|] |> Array.to_list)
-                  |> (List.filter_map (fun x -> x)))
-                 |> Array.of_list) in
+              [|("key",
+                  (inject
+                     (Js_of_ocaml.Js.Optdef.option
+                        (Option.map Js_of_ocaml.Js.string key))));("children",
+                                                                    (
+                                                                    inject
+                                                                    children))|] in
   let make =
     ((fun ~children:kids ->
         React.Dom.createDOMElementVariadic "div"
@@ -157,12 +159,13 @@ let make =
         fun () ->
           let open Js_of_ocaml.Js.Unsafe in
             obj
-              ((([|(Option.map
-                      (fun raw ->
-                         ("key", (inject (Js_of_ocaml.Js.string raw)))) key);(
-                   Some ("children", (inject children)))|] |> Array.to_list)
-                  |> (List.filter_map (fun x -> x)))
-                 |> Array.of_list) in
+              [|("key",
+                  (inject
+                     (Js_of_ocaml.Js.Optdef.option
+                        (Option.map Js_of_ocaml.Js.string key))));("children",
+                                                                    (
+                                                                    inject
+                                                                    children))|] in
   let make =
     ((fun ~children:(first, second) ->
         fun () ->
@@ -187,3 +190,41 @@ let make =
   fun ~children ->
     fun ?key ->
       fun () -> React.createElement make (make_props ?key ~children ())
+let make =
+  let make_props
+    : ?name:'name ->
+        ?key:string ->
+          unit ->
+            < name: 'name option Js_of_ocaml.Js.readonly_prop   > 
+              Js_of_ocaml.Js.t
+    =
+    fun ?name ->
+      fun ?key ->
+        fun () ->
+          let open Js_of_ocaml.Js.Unsafe in
+            obj
+              [|("key",
+                  (inject
+                     (Js_of_ocaml.Js.Optdef.option
+                        (Option.map Js_of_ocaml.Js.string key))));("name",
+                                                                    (
+                                                                    inject
+                                                                    name))|] in
+  let make =
+    ((fun ?(name= "") ->
+        React.Dom.createDOMElementVariadic "div"
+          ~props:(Js_of_ocaml.Js.Unsafe.obj [||] : React.Dom.domProps) 
+          [name])
+    [@warning "-16"]) in
+  let make
+    (Props :
+      < name: 'name option Js_of_ocaml.Js.readonly_prop   >  Js_of_ocaml.Js.t)
+    =
+    make
+      ?name:(fun (type res) -> fun (type a0) ->
+               fun (a0 : a0 Js_of_ocaml.Js.t) ->
+                 fun (_ : a0 -> < get: res   ;.. >  Js_of_ocaml.Js.gen_prop)
+                   -> (Js_of_ocaml.Js.Unsafe.get a0 "name" : res)
+               (Props : < .. >  Js_of_ocaml.Js.t) (fun x -> x#name)) in
+  fun ?name ->
+    fun ?key -> fun () -> React.createElement make (make_props ?key ?name ())