[Call for help] Test the React JSX v4

We’d like to share the progress of React JSX v4 and the example project to test. The first version of React JSX v4 was merged to the master branch in the compiler and syntax recently. The rescript-react is working in progress for compatibility with v4.

The major changes in the JSX v4 are;

  • the props type of component (no more magic with makeProps, etc.)
  • the new jsx transform api (react>=17)

Due to the substantial changes, more thorough and real usage tests are needed.

Hope to have as many real usage cases as we can from you. Please feel free to report any issues, or make a PR to suggest.

The example project https://github.com/mattdamon108/react-jsx-ppx-v4

This example project has dependencies as below

Migration instruction https://github.com/rescript-lang/syntax/blob/master/cli/JSXV4UPGRADE.md

19 Likes

Side note - @moondaddi has done some phenomenal work on this, over a long period of time. Really well done! :clap:

10 Likes

Are there (code) migration instructions somewhere?
Should I expect to make code changes when I’m using the new compiler and React bindings but the old react-jsx: 3, or is this not supported?
What does classic mean and what are the other options?

What is the idiomatic way to use React.memoCustomCompareProps with the new props type?
The following does not work:

module Foo = {
  @react.component
  let make = React.memoCustomCompareProps(
    (~foo: bool) => React.null,
    (nextProps, currentProps) => nextProps.foo == currentProps.foo,
  )
}

Fails with:

rescript: [17/63] src/SpotDetails.ast
FAILED: src/SpotDetails.ast

  We've found a bug for you!
  /home/bart.schuurmans/engineering/lumi/frontend/stallingsnet/src/SpotDetails.res:5:14-8:3

   3 │ module Foo = {
   4 │   @react.component
   5 │   let make = React.memoCustomCompareProps(
   6 │     (~foo: bool) => React.null,
   7 │     (nextProps, currentProps) => nextProps.foo == currentProps.foo,
   8 │   )
   9 │ }
  10 │ 

  react.component calls can only be on function definitions or component wrappers (forwardRef, memo).

I’ve tried it in my small personal project, works fine. There was a small issue with React.Fragment before I changed

"reason": { "react-jsx": 3 },

to

"jsx": {
  "version": 4,
  "mode": "classic"
},

It has to be updated though, here is the upgrade instruction. https://github.com/rescript-lang/syntax/blob/master/cli/JSXV4UPGRADE.md
I also added it to the original post.

It is intended that no need to change the code with the new compiler and new React bindings for JSX v4. react-jsx: 3 configuration still works with the new compiler and old React bindings@0.10.3. There is a breaking change in the rescript-react between v3 and v4.

Thank you for the report. Would you mind to leave an issue?

There is a breaking change between v3 and v4. So, the old configuration "reason": { "react-jsx": 3 } is not working with the new React binding.

Is the code snippet compiled with v3? I’ve tested it with the compiler v10 and v3, it failed to be compiled.

I’ve found a way to make it work, but the way I did it before no longer works: https://github.com/Minnozz/rescript-react-jsx-4/blob/main/src/MemoCustomCompareProps.res

module TestMemoCustomComparePropsOld = {
  let memo = React.memoCustomCompareProps(
    _,
    (nextProps, currentProps) => nextProps.foo == currentProps.foo,
  )

  @react.component
  and make = memo((~foo: string) => React.string(foo))
}

I’m not used to memo, so not sure. Is it workable?

Yes, this works :+1:

1 Like

Is there a tag / published npm version we can use to test (both for the compiler and rescript-react) instead of #master? My build system has issues with GitHub references.

Not yet. Is it possible to install it from the local file system? If so, how about installing it after cloning both compiler and React binding into your local.

I can use #master locally without issues, but our (Nix-based) CI has trouble with GitHub dependencies.

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 :smiley:

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:

  1. 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?
  2. 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 :smiley:

4 Likes

Props type

Very good point. There are two reasons in having type parameters for props type. For example,

// implementation
@react.component
let make = (~s) => React.string(s)
// The first reason
// the ppx doesn't know what type s is in the parsetree.
// Therefore, the ppx can make only type props<'s> = {s: 's}

// interface
@react.component
let make = (~s: string) => React.element
// The second reason
// If the ppx makes type props = {s: string}
// There is inconsistency between implementation and interface. 

That is why the compiler doesn’t complain about Test.props<int>.

Prop spread

I don’t think you can remember my first suggestion was. It was the spread props. That was a beginning to contribute the React JSX v4 now. :smiley: Yes, the spread props is available, but there is a restriction. Let me show you the example.

module Test = {
  @react.component
  let make = (~s, ~x) => (s ++ x)->React.string
}

@react.component
let make = () => {
  let str: Test.props<string, string> = {
    s: "s",
    x: "x",
  }

  <Test x="y" {...str} />
}

As you well know, the type of props is the record now in v4. Actually, the spread props is using the spread operator ... in the record. So, another prop is needed at least such as x. <Test x="y" {...str} /> will be transformed roughly.

React.createElement(Test.make, {x: "y", ...str})

Error message

Very good point. We need to improve the error message from the compiler and also the type information from the editor about props type.

I hope my answers helpful for you. Thank you for your feedback!

1 Like

@moondaddi would you add reminder issues for these things, whether fixes or improved docs?

Sure, I’ll add it. :+1:

What if types were required in the implementation? Then it would know. And you could still allow _ to have the whole or parts of a type inferred.

Edit: I suppose it wouldn’t even have to be required. The ppx could just use what type information is present, and leave the rest as type variables.