Really cool! Thanks for all the great work so far, I think this definitely has the potential to make life easier! I played with the example repo you set-up a bit (really helpful to get started) and have a few questions:
I think having props
be a record type is great and makes it easier to use in different places, but looking at it closer it also seems like it’s a record without any concrete types, so from a type system perspective its usefulness is very limited.
Prop types
An example snippet:
module Test = {
@react.component
let make = (~s) => React.string(s)
}
@react.component
let make = () => {
let str : Test.props<int> = {
s: 1
};
<Test s={str.s} />
}
This produces the following error:
32 ┆ };
33 ┆
34 ┆ <Test s={str.s} />
This has type: int
Somewhere wanted: string
You can convert int to string with Belt.Int.toString.
I would’ve expected the error not to be on s={str.s}
but on the definition of str
to begin with (and would expect to be able to use Test.props
without arguments to the type constructor). Having the props as a record with abstract types isn’t very helpful if you want to create functions that prepare or deal with those prop records
Prop spread
Is it correct that this release does not yet allow us to do
module Test = {
@react.component
let make = (~s) => React.string(s)
}
@react.component
let make = () => {
let str : Test.props<string> = {
s: "String"
};
<Test {...str} />
}
The compiler doesn’t complain about the propspread syntax in the component but it does say that s
is not present, which is confusing:
32 │ };
33 │
34 │ <Test {...str} />
35 │ }
36 │
Some record fields are undefined: s
Error message
Can we improve the error message that is being provided for missing properties?
module Test = {
@react.component
let make = (~str, ~int) => React.string(str ++ " " ++ (int->Belt.Int.toString))
}
@react.component
let make = () => {
<Test />
}
Provides the following error message and we can debate whether it’s better or worse than what we had.
28 │ @react.component
29 │ let make = () => {
30 │ <Test />
31 │ }
32 │
Some record fields are undefined: str
It’s better because it doesn’t provide the “expected function of type” and shows the entire makeProps
function, which was often unhelpful to newcomers.
However, I think it’s also worse in a few important ways:
- The error message requires knowledge of the internal implementation of JSX. I don’t know that a JSX element is represented by a record, there’s no record in my code. Why is a record field missing?
- I only see a single missing required field at a time. This means I need to go back 'n forth with the compiler a few times, rather than being made aware of all missing properties.
Thanks again for all the great work so far! I hope we can use the above feedback to make this even better