reasonml / reason-react

Reason bindings for ReactJS
https://reasonml.github.io/reason-react/
MIT License
3.25k stars 347 forks source link

typed keys 2 #813

Open jchavarri opened 10 months ago

jchavarri commented 10 months ago

Alternative take to having typed keys based on the ideas from #812.

Centers the typing around the React.array function. The only case where keys should be added is when children are variable size arrays, and not literals. So React.array might be the best spot to place the restriction.

The issue is that React.array is used in the ppx to convert literals of children like <div> foo bar </div>. To cover that case, a new function React.unsafeArray is added.

The type errors look like:

File "test/React__test.re", line 140, characters 41-46:
140 |       ReactDOM.Client.render(root, <div> array->React.array </div>)
                                               ^^^^^
Error: This expression has type React.element array
       but an expression was expected of type React.elementKeyed array
       Type React.element is not compatible with type React.elementKeyed

One downside is that usages of key outside arrays now would trigger a type error. This makes sense, because why would key be added if no needed, but who knows what could break. Also, in all cases the error happens far away from where the component is defined.

E.g. for this code:

module Foo = {
  [@react.component]
  let make = () => {
    <div key="hey"> 2->React.int </div>;
  };
};

let t = <div> <Foo /> </div>;

the error is:

File "test/React__test.re", line 376, characters 14-21:
376 | let t = <div> <Foo /> </div>;
                    ^^^^^^^
Error: This expression has type <  > Js.t -> React.elementKeyed
       but an expression was expected of type
         <  > Js.t React.component = <  > Js.t -> React.element
       Type React.elementKeyed is not compatible with type React.element
andreypopp commented 10 months ago

This makes sense, because why would key be added if no needed

Sometimes it's useful to have key specified outside of an array to re-mount the element. Ref to React docs: https://react.dev/reference/react/useState#resetting-state-with-a-key