Fix forwardRef unsafety + make DOM refs types more idiomatic

bin/Refs.re

@@ -1,25 +1,28 @@
-let map = (f, opt) =>
-  switch (opt) {
-  | Some(s) => Some(f(s))
-  | None => None
-  };
+let log = a => Js_of_ocaml.Firebug.console##log(a);
 
-module FancyButton = {
+module FancyLink = {
   [@react.component]
   let make =
-    React.forwardRef(ref => {
-      let console = Js_of_ocaml.Firebug.console;
-      console##log("FancyButton got ref");
-      console##log(ref);
-      <button ref=?{map(ReactDOM.Ref.domRef, ref)} className="FancyButton">
-        {"Click me!" |> React.string}
-      </button>;
-    });
+    ReactDOM.forwardRef(ref =>
+      <a ?ref href="https://github.com/jchavarri/rroo/" className="FancyLink">
+        {"rroo GitHub repo" |> React.string}
+      </a>
+    );
 };
 
 [@react.component]
 let make = () => {
+  let (show, setShow) = React.useState(() => true);
   // You can now get a ref directly to the DOM button:
-  let ref = React.createRef();
-  <FancyButton ref />;
+  let ref =
+    ReactDOM.Ref.callbackDomRef(ref => {
+      log(Js_of_ocaml.Js.string("Ref is:"));
+      log(ref);
+    });
+  <>
+    <button onClick={_ => setShow(s => !s)}>
+      {"Toggle fancy link" |> React.string}
+    </button>
+    {show ? <FancyLink ref /> : React.null}
+  </>;
 };

lib/React.c

@@ -60,11 +60,6 @@ void createRef () {
   exit(1);
 }
 
-void forwardRef () {
-  fprintf(stderr, "Unimplemented React function `forwardRef`!\n");
-  exit(1);
-}
-
 void useRef () {
   fprintf(stderr, "Unimplemented React function `useRef`!\n");
   exit(1);

lib/React.js

@@ -68,27 +68,19 @@ var useEffect = useEffectFunction(React.useEffect);
 var useLayoutEffect = useEffectFunction(React.useLayoutEffect);
 
 // Provides: Ref_current
-// Requires: React
-var Ref_current = React.current;
+var Ref_current = function(ref, value) {
+  return ref.current;
+};
 
 // Provides: Ref_setCurrent
-// Requires: React
-var Ref_setCurrent = React.setCurrent;
+var Ref_setCurrent = function(ref, value) {
+  ref.current = value;
+};
 
 // Provides: createRef
 // Requires: React
 var createRef = React.createRef;
 
-// Provides: forwardRef
-// Requires: React, caml_js_wrap_callback, jsNullToOption
-var forwardRef = function(callback) {
-  var jsCallback = caml_js_wrap_callback(callback);
-  return React.forwardRef(function(props, jsRef) {
-    var ref = jsNullToOption(jsRef);
-    return jsCallback(props, jsRef);
-  });
-};
-
 // Provides: useRef
 // Requires: React
 var useRef = React.useRef;

lib/React.re

@@ -28,10 +28,6 @@ module Ref = {
 
 external createRef: unit => Ref.t(option('a)) = "createRef";
 
-external forwardRef:
-  (('props, option(Ref.t('a))) => element) => component('props) =
-  "forwardRef";
-
 // module Children = {
 //   external map: (element, element => element) => element = "Children_map";
 //   external forEach: (element, element => unit) => unit = "Children_forEach";

lib/ReactDOM.c

@@ -5,6 +5,21 @@ void render () {
   exit(1);
 }
 
+void domRef () {
+  fprintf(stderr, "Unimplemented ReactDOM function `domRef`!\n");
+  exit(1);
+}
+
+void callbackDomRef () {
+  fprintf(stderr, "Unimplemented ReactDOM function `callbackDomRef`!\n");
+  exit(1);
+}
+
+void forwardRef () {
+  fprintf(stderr, "Unimplemented ReactDOM function `forwardRef`!\n");
+  exit(1);
+}
+
 void domProps () {
   fprintf(stderr, "Unimplemented ReactDOM function `domProps`!\n");
   exit(1);

lib/ReactDOM.js

@@ -5,15 +5,44 @@ var ReactDOM = joo_global_object.ReactDOM;
 // Requires: ReactDOM
 var render = ReactDOM.render;
 
+// Provides: domRef
+// Requires: jsNullToOption
+var domRef = function(ref) {
+  var jsCallback = function(element) {
+    ref.current = jsNullToOption(element);
+  };
+  return jsCallback;
+};
+
+// Provides: callbackDomRef
+// Requires: jsNullToOption
+var callbackDomRef = function(cb) {
+  var jsCallback = function(element) {
+    cb(jsNullToOption(element));
+  };
+  return jsCallback;
+};
+
+// Provides: forwardRef
+// Requires: React, caml_js_wrap_callback
+var forwardRef = function(callback) {
+  var jsCallback = caml_js_wrap_callback(callback);
+  return React.forwardRef(function(props, ref) {
+    // No need to convert refs, as they'll be processed in callbackDomRef and domRef functions
+    return jsCallback(props, ref);
+  });
+};
+
 // Provides: domProps
 // Requires: setIfSome, setIfSomeMap, caml_js_from_string, caml_js_wrap_callback
-var domProps = function(key, ref, alt, async, className, onClick) {
+var domProps = function(key, ref, alt, async, className, href, onClick) {
   var tmp = {};
   setIfSomeMap(tmp, "key", key, caml_js_from_string);
   setIfSome(tmp, "ref", ref);
   setIfSomeMap(tmp, "alt", alt, caml_js_from_string);
   setIfSome(tmp, "async", async);
   setIfSomeMap(tmp, "className", className, caml_js_from_string);
+  setIfSomeMap(tmp, "href", href, caml_js_from_string);
   setIfSomeMap(tmp, "onClick", onClick, caml_js_wrap_callback);
   return tmp;
 };

lib/ReactDOM.re

@@ -16,21 +16,29 @@ let optInj = (~f=?, prop, opt) =>
 type domRef;
 module Ref = {
   type t = domRef;
-  type currentDomRef = React.Ref.t(Js.opt(Dom_html.element));
-  type callbackDomRef = Js.opt(Dom_html.element) => unit;
+  type currentDomRef = React.Ref.t(option(Dom_html.element));
+  type callbackDomRef = option(Dom_html.element) => unit;
 
-  external domRef: currentDomRef => domRef = "%identity";
-  external callbackDomRef: callbackDomRef => domRef = "%identity";
+  external domRef: currentDomRef => domRef = "domRef";
+  external callbackDomRef: callbackDomRef => domRef = "callbackDomRef";
 };
 
+external forwardRef:
+  (('props, option(domRef)) => React.element) => React.component('props) =
+  "forwardRef";
+
 type domProps;
+
+/* Important: the order of these labelled arguments must match the order in which
+the params are listed in the ReactDOM.js external implementation of `domProps` */
 external domProps:
   (
     ~key: string=?,
     ~ref: domRef=?,
     ~alt: string=?,
     ~async: bool=?,
     ~className: string=?,
+    ~href: string=?,
     ~onClick: ReactEvent.Mouse.t => unit=?,
     unit
   ) =>

ppx/Rroo_jsoo_ppx.re

@@ -88,6 +88,9 @@ let keyType = loc =>
     [Typ.constr(~loc, {loc, txt: Lident("string")}, [])],
   );
 
+let refType = loc =>
+  Typ.constr(~loc, {loc, txt: Ldot(Lident("ReactDOM"), "domRef")}, []);
+
 type children('a) =
   | ListLiteral('a)
   | Exact('a);
@@ -1110,7 +1113,7 @@ let jsxMapper = () => {
                   Pat.var({txt: "key", loc: emptyLoc}),
                   "ref",
                   emptyLoc,
-                  None,
+                  Some(refType(emptyLoc)),
                 ),
                 ...namedArgListWithKeyAndRef,
               ]

ppx/test/pp.expected

@@ -36,3 +36,21 @@ let make =
                                                                     ^ name))
                                                        ()))|] in
   Test
+module ForwardRef =
+  struct
+    let makeProps
+      : ?key:string -> ?ref:ReactDOM.domRef -> unit -> <  >  Js_of_ocaml.Js.t
+      =
+      fun ?key ->
+        fun ?ref ->
+          fun _ ->
+            let open Js_of_ocaml.Js.Unsafe in
+              obj [|("ref", (inject ref));("key", (inject key))|]
+    let make =
+      React.forwardRef
+        (let Test$ForwardRef (Props : <  >  Js_of_ocaml.Js.t) theRef =
+           ReactDOM.createDOMElementVariadic "div"
+             ~props:(ReactDOM.domProps ~ref:theRef ())
+             [|("ForwardRef" |. React.string)|] in
+         Test$ForwardRef)
+  end

ppx/test/test.ml

@@ -5,3 +5,11 @@ let make ?(name= "")  =
         ())
     [@JSX ])])
   [@JSX ])[@@react.component ]
+module ForwardRef =
+  struct
+    let make =
+      React.forwardRef
+        (fun theRef ->
+           ((div ~ref:theRef ~children:["ForwardRef" |. React.string] ())
+           [@JSX ]))[@@react.component ]
+  end

ppx/test/test_src.re

@@ -14,3 +14,11 @@ let make = (~name="") => {
     <Hello one="1"> {React.string("Hello " ++ name)} </Hello>
   </>;
 };
+
+module ForwardRef = {
+  [@react.component]
+  let make =
+    React.forwardRef(theRef =>
+      <div ref=theRef> "ForwardRef"->React.string </div>
+    );
+};