Closed anmonteiro closed 1 year ago
In the following example:
let foo = React.useCallback((_a, _b) => Js.log("HELLO"));
Maybe this could be added to the tests.
I tried to understand better why this change works, and why it doesn't break any other existing cases. Leaving here a note for the record.
The reason is that uncurry
is only necessary if the callback function is called from the JavaScript side. For useCallback
, this never happens: React doesn't check or call the function itself, nor the # of arguments it has. In other words, the function will only be called from the OCaml side.
We preserve the uncurried callbacks in
React.Uncurried.useCallback*
The above makes me wonder about the Uncurried
version. If we know the function will always be called from OCaml side, is there a point to keep the mel.uncurry
attribute in that one?
For useCallback, this never happens: React doesn't check or call the function itself, nor the # of arguments it has. In other words, the function will only be called from the OCaml side.
This isn't true. I caught the bug when passing it to JavaScript, e.g.:
module X = {
[@mel.module "foobar"] [@react.component]
external make: (~takesFunction: (a, b) => unit) => React.element = "default";
};
module FromOCaml = {
[@react.component]
let make = () => {
let myFn = React.useCallback0((a,b)=>a+b);
<X takesFunction=myFn />;
};
};
I think in the case you mention, the takesFunction
prop should be using @u
attribute to uncurry. Otherwise it can be trivially broken. E.g. what happens if you do this?
let myFn = React.useCallback0((a) => {let f = (b) =>a+b; f});
<X takesFunction=myFn />;
I think it breaks even without useCallback
, so the problem does not seem related to React.useCallback
at all?
That's true. Are you suggesting we remove [@mel.uncurry]
from the Uncurried
module too?
It's also not possible to use uncurried functions in useCallback
. Playground example:
So perhaps the Uncurried
module should specify those explicitly? Something like this (better naming suggestions appreciated):
module Uncurried = {
[@mel.module "react"]
external useCallback1_0:
((. 'input) => 'output, [@mel.as {json|[]|json}] _) =>
((. 'input) => 'output) =
"useCallback";
[@mel.module "react"]
external useCallback2_0:
((. 'input1, 'input2) => 'output, [@mel.as {json|[]|json}] _) =>
((. 'input1, 'input2) => 'output) =
"useCallback";
};
Are you suggesting we remove
[@mel.uncurry]
from theUncurried
module too?
Not really. With the message above I just wanted to express that the code in the react
JS library (which we bind against) will never call the function passed to useCallback
. So using uncurry
or not in this case does not make any difference as you noted originally :)
I am not sure about the support for uncurried functions... I need to think about it more.
I think we could simplify things quite a bit if we get rid of the function type on the useCallback
binding. So instead of 'input => 'output
we just use 'a
.
I know this might sound unsafe, but ReactJS itself doesn't care about whatever is on the value passed to useCallback
. And from OCaml side, things will be type safe because whatever gets passed to useCallback
comes through the other side. You can potentially call useCallback(5)
but you could never "call" it.
[@mel.module "react"]
external useCallback0: ('a, [@mel.as {json|[]|json}] _) => 'a = "useCallback";
let x = useCallback0((. a, b) => a + b);
let y = x(. 2, 3);
Seems more correct than the current situation anyway.
@jchavarri used your suggestion. Added a generic 'fn
argument to the useCallback
functions, and removed them from the Uncurried
module, since they'd be the same.
It's minor but there's a runtime check of being sure that useCallback gets a function. That's probably why the callback type was added in the first place.
It's minor but there's a runtime check of being sure that useCallback gets a function. That's probably why the callback type was added in the first place.
Do you mean in React.js proper? That's even better, then. React can make sure we're passing a function (though it'll be caught at development time).
Seems React just swallows whatever is passed to it: https://stackblitz.com/edit/react-playground-practice-13ux64?file=index.js
@jchavarri I got a TS error on JSX and didn't realised useCallback eats everything.
React.useCallback
can take a function of multiple arguments. In the following example:The logging never occurs with
[@mel.uncurry]
because Melange generates the following code:We preserve the uncurried callbacks in
React.Uncurried.useCallback*