RFC: Typesafe "heterogenous" dependency arrays for React hooks

[cknitt]

There are currently the following issues with dependency arrays for React hooks, for example useEffect which is defined as follows:

@module("react")
external useEffect: (@uncurry (unit => option<unit => unit>), 'deps) => unit = "useEffect"

1. It is not ensured by the type system that 'deps is actually an array or a tuple.
2. For 0 or 1 dependencies, an array must be used, but for more than 1 dependencies (of different types), a tuple must be used. This is confusing for newcomers.
3. When moving from two dependencies (a, b) to one, it is easy to just delete b and end up with (a) instead of [a] which is incorrect and can lead to issues at runtime.

Idea:

1. Define an abstract type dependency.
2. In the bindings for useEffect, useCallback etc., type the dependency arrays as array<dependency>.
3. Give the user conversion functions for manual conversion of dependency arrays/tuples to this type.
4. Auto-convert all the standard use cases directly in the JSX PPX, so that the user can just write the following like they would in JS:

let useEffectTest0 = () => {    
  React.useEffect(() => None, [])
}

let useEffectTest1 = (a: string) => {    
  React.useEffect(() => None, [a])
}

let useEffectTest2 = (a: string, b: int) => {    
  React.useEffect(() => None, [a, b])
}

let useEffectTest3 = (a: string, b: int, c: float) => {    
  React.useEffect(() => None, [a, b, c])
}

In this usage (invocation of React.useEffect/useCallback etc. with an array or tuple as the second parameter), the JSX PPX would e.g. convert [a, b, c] to JsxPPXReactSupport.unsafeToDependencyArray((a, b, c)) (with zero cost).

This PR is a proof of concept for this idea.

[mununki]

This seems be working based on the textual match in AST. In case of newly value binding would not work. e.g. let ue = React.useEffect. And it is a little hesitant to manipulate the expression used by the user directly.
Couldn't we use a wrapper function for the React.useEffect function to accomplish our purpose?

[mununki]

Previous discussion for reference: rescript-lang/rescript-react#97

[cknitt]

Couldn't we use a wrapper function for the React.useEffect function to accomplish our purpose?

The PR has two purposes:

1. At compile time, make sure that something passed as a dependency array actually is an array in JS, i.e., it is an array or tuple in ReScript.
2. Offer a unified syntax to avoid the confusion of [], [a] vs. (a, b), (a, b, c), ...

I do not see how either of these could be accomplished via a wrapper function.

[mununki]

Downside of manipulating the expression directly based on the textual match. Instead, we can use the attribute such as @deps that is more recommneded way for ppx.

useEffect(_ => None, @deps [1, "2"]) // something like

The built-in ppx would transform the array, maybe like

@unboxed
type rec dep = Dep('a):dep

useEffect(_ => None, [Dep(1), Dep("2")])

IMHO, this is more typesafe.

[cknitt]

useEffect(_ => None, @deps [1, "2"]) // something like

@mununki That's also a possible, and more explicit approach, and I agree that it makes it clearer that some "magic" is happening here with that "heterogenous" array.

The downside is that having to add @deps is again something new to learn for people coming from JS, and that it would cause additional migration effort for people who have already migrated to ReScript 11 / latest rescript-react.

Also, while @deps is nice and short, it should probably rather be @react.deps.

@zth What are your thoughts on this?

[zth]

Sorry, I missed this. I like the general idea of making this part of using React simpler/more intuitive, but I'm also afraid of making arrays appear heterogenous. Pretty sure that'll lead to confusion about why it's not allowed anywhere else.

I had one idea related to this, that I'm not sure if it's practically feasible, but might be something worth thinking about - what if we introduced either a type, or an annotation, that would let us enforce using a type that has a specific runtime representation?

@module("react")
external useMemo: (unit => 'value, @array 'deps) => 'value = "useMemo"

@array probably isn't the best name, but maybe you get the idea. It'd force you to either use any array, or any tuple as they're also arrays at runtime.

Again, might not be feasible. Me and @cristianoc had a discussion about this but I honestly don't remember what we said.

Anyway, the effect would be that we'd get rid of "useN", but we'd still have array vs tuples. So, one less issue, but not everything solved.

[DZakh]

I'd like to have a more general solution

[cknitt]

Again, might not be feasible. Me and @cristianoc had a discussion about this but I honestly don't remember what we said.

Anyway, the effect would be that we'd get rid of "useN", but we'd still have array vs tuples. So, one less issue, but not everything solved.

I quite like the @array idea, too, even if we still have array vs tuples then.

[cknitt]

Closing this as this approach is too hacky. 🙂