-
Notifications
You must be signed in to change notification settings - Fork 347
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
React.forwardRef is unsafe #421
Comments
I investigated this a bit as part of the work in rroo. The [@react.component]
let make = () => <X ref=12345 />; I believe one fix could be:
[@bs.module "react"]
external forwardRef:
([@bs.uncurry] (('props, ReactDOMRe.domRef) => React.element)) =>
React.component('props) =
"";
If anyone wants to explore how these changes would look like, here's a version of [@bs.module "react"]
external forwardRef:
([@bs.uncurry] (('props, ReactDOMRe.domRef) => React.element)) =>
React.component('props) =
"";
module X = {
[@react.component]
let make =
forwardRef(theRef => {
<div ref=theRef> "ForwardRef"->React.string </div>;
});
};
// Would compile to:
module X = {
[@bs.obj]
external makeProps:
(~key: string=?, ~ref: ReactDOMRe.domRef=?, unit) => Js.t({.}) =
"";
let make =
forwardRef(
{
let test = (_props: Js.t({.}), theRef) => {
ReactDOMRe.createDOMElementVariadic(
"div",
~props=ReactDOMRe.domProps(~ref=theRef, ()),
[|"ForwardRef"->React.string|],
);
};
test;
},
);
}; Then calling <X ref={theRef => theRef->React.Ref.current->Js.log} /> would fail with:
|
Here's the related PR in rroo, in case it helps: ml-in-barcelona/jsoo-react#19. The required changes in the ppx are tiny: https://github.com/jchavarri/rroo/pull/19/files#diff-ec2648e0bf16200f53280d61f4d4ee33. One thing to note is that I had to move |
Hey folks, For the sake of cleaning up the repo (and given how old this issue is) I'm going to close this out. Thank you and sorry about the delay. Please re-open if its still relevant! |
Re-opening since this might still be relevant |
Usecase:
This compiles b/c
theRef
type is inferred in the callback, but it crashes at runtime since in this casetheRef
is of typeJs.nullable(Dom.element)
.The text was updated successfully, but these errors were encountered: