Allow string option transformations for native elements
#92
Conversation
| @@ -17,3 +17,5 @@ let%component make ~children:(first, second) () = | |||
| div ~children:[first; second] () | |||
|
|
|||
| let%component make ?(name = "") = div ~children:[name] () | |||
|
|
|||
| let%component make () = a ?href:(Some "https://opam.ocaml.org") () | |||
There was a problem hiding this comment.
Could we add a test in test_jsoo_react please? ppx tests will swallow all the things as long as they're syntactically valid, so they don't allow to detect type errors.
There was a problem hiding this comment.
I wanted to add them as example, but forgot we have tests that run the browser 💯
Fixed in 7c03b8b (#92)
test/test_jsoo_react.re
Outdated
| @@ -92,6 +92,32 @@ let testOptionalProps = () => { | |||
| }); | |||
| }; | |||
|
|
|||
| let testOptionalAttributes = () => { | |||
There was a problem hiding this comment.
Maybe we could call this test testOptionalPropsLowercase and rename the other one to testOptionalPropsUppercase?
| @@ -440,7 +440,7 @@ let rec makeFunsForMakePropsBody list args = | |||
| let makeAttributeValue ~loc ~isOptional (type_ : Html.attributeType) value = | |||
| match (type_, isOptional) with | |||
| | String, true -> | |||
| [%expr Js_of_ocaml.Js.string ([%e value] : string option)] | |||
| [%expr Option.map Js_of_ocaml.Js.string ([%e value] : string option)] | |||
There was a problem hiding this comment.
This does not affect any other cases because we only convert the values for the String case right?
There was a problem hiding this comment.
Exactly, any other case is handled with OptDef.
|
Just want to chime in and say thanks for this @davesnx ❤️! I bumped up against this when trying to create components that used |
This PR fixes a remaining bug for optional parameters, from #81 (comment)
Was introduced by #84.