String literals in example JSX

nkrkv commented 2 years ago

Hello, I’m working on https://github.com/nkrkv/tree-sitter-rescript/. I want to use this repo for acceptance testing because it is big and significant. Once it is added to the test suite, I see quite a lot of errors related to the usage of string literals inside JSX. For example:

https://github.com/cca-io/rescript-material-ui/blob/master/examples/src/examples/Examples.res#L7

  <Grid container=true>
    <Grid item=true md=Grid.Md.\"12">
      <Typography variant=#h4> "ReScript Material-UI Examples" </Typography>
    </Grid>
  </Grid>

I was intrigued, am I missing something, are string literals now valid without { React.string(...) }? But ReScript playground says otherwise:

https://rescript-lang.org/try?code=LYewJgrgNgpgBAIQgF2SAdnAvHA3gKDjgAEAnGAQwGNkA6KkYABwxnWULlmTmAoGt4OABQBKbAD48nIgB4ARijTopAIgASMKFBCq4sgPSLUGCZwC++S0A

So, I’m confused, is it an auto-generation flaw or did I indeed miss something? I’m in doubt because I found 11 entries… Would you shine a light? Thank you.

fhammerschmidt commented 2 years ago

This works because the children type is more relaxed in this bindings - it's just a generic type 'children. The React bindings use their own abstract type element.

We use the same type for every children parameter because it's all generated. But maybe @jsiebern can explain if and why it was necessary, or if we can switch to React.element instead.

jsiebern commented 2 years ago

I believe it was due to the old react bindings and a problem with children being forced to be an array of elements (which the react ppx ensured automatically). Today there should be no forced reason to keep the children generic.

fhammerschmidt commented 2 years ago

Good, will change it to React.element for the 3.0.0 release (which will break some things anyway).

fhammerschmidt commented 2 years ago

Thanks for raising this issue, very good point.

cca-io / rescript-mui

String literals in example JSX #172