[Call for help] Test the React JSX v4

41

Sorry right an alpha version of ReScript-react will be published too — not done yet.

42

Great job!

I got a doubt regarding props being a record (or an extensible record). Currently generating a record seems dangerous since many type errors would locate and reference the record that is hidden to users: like [Call for help] Test the React JSX v4 - #16 by Kingdutch or

Seems like one approach would be to use the record on the props and ditch the labelled arguments. Did you think about this approach? Is the end goal?

Sorry for not posting this earlier

43

That’s pretty much what the JSX does: convert labelled argument format to record. If you write it directly without JSX, you can define your own record. The @react.component processing is only supposed to be a (backwards compatible) shortcut going forward, which you don’t necessarily have to use.

44

Thank you for pointing this. I’ll update the schema.

45

The JSX ppx would transform the jsx expressions according to the bsconfig in each libraries respectively. We can make v4 as default https://github.com/rescript-lang/rescript-compiler/pull/5595, but I think there are some trade-offs.
At this moment, any react component libraries needs to be updated with new jsx configuration. e.g. rescript-react. If a library is written without @react.component, then it needs to be re-written the props type instead of makeProps.
Still not sure of making v4 as default which means ignoring the old jsx configuration reason.react-jsx: 3 in the library, and want to hear thoughts from the community.

46

I think what @cknitt is talking about is to make the configuration overridable by consumer’s bsconfig.json.

The same behaviour as package-specs has:

This configuration only applies to you, when you develop the project. When the project is used as a third-party library, the consumer’s own bsconfig.json package-specs overrides the configuration here, logically.

47

Since the “jsx” config is new, this can be done without having breaking changes.
The only question is whether it should be done.
Worth thinking about.

48

Two possible aspects to consider are convenience and risk management.

In terms of convenience, for example, it can be nice if noting needs to be done for any dependency that still compiles and works fine under V4.

In terms of risks, for example, it can be problematic if a dependency compiles fine, but under the hood has some sort of interaction with JS which makes assumptions on the runtime representation of components. Then all still compiles, but crashes.

1 Like
49

OK @cknitt and others: would you have a go with this https://github.com/rescript-lang/rescript-compiler/pull/5661
and report feedback?

It should (hopefully) pass jsx config to dependencies (and rebuild when the jsx config changes).

1 Like
50

Thanks a lot @cristianoc!

I just tested this quickly in a large project of ours (yarn workspaces monorepo with web app, mobile app and common code).

Passing on the JSX config to dependencies works fine.

The following dependencies were having issues with the passed through JSX 4 classic setting:

  • @rescript/react (only the RescriptReactErrorBoundary component, and you already mentioned there is a JSX 4 compatible branch for @rescript/react anyway)
  • The rescript-react-navigation bindings because they assume the V3 implementation and directly specify @obj external makeProps functions.
  • rescript-material-ui because it explicitly declares the key prop in its component definitions, e.g.:
@react.component @module("@material-ui/core")
external make: (
  ~children: React.element=?,
  ~classes: Classes.t=?,
  ~className: string=?,
  ~id: string=?,
  ~style: ReactDOM.Style.t=?,
  ~key: string=?,
  ~ref: ReactDOM.domRef=?,
) => React.element = "ScopedCssBaseline"

This gives the error “Two labels are named key”.

At this point I stopped testing further, would need to adapt these libraries first.

Nonetheless, I think propagating the setting is the right way. If a library has any issues with JSX v4, they will come up like the ones above did, if it has none, I think it should work fine in runtime, too.

2 Likes
51

This looks very promising. No peculiar cases have come up, and the learning is faster than without the new feature.

52

@cknitt one might need an option to specify what packages do not inherit configuration.
E.g.: ["rescript-react-navigation", "rescript-react-native"]

53

Why would that be needed? If they don’t inherit the configuration, they will not work with the project bsconfig.json set to JSX4 anyway (error “The record field XXX can’t be found”, like I had on my first attempt).

54

You can set V3 mode in the middle of.a file.

The goal, however is achieved, is to have an escape route to continue using a dependency before it is upgraded, without getting blocked indefinitely.

55

Totally, my question was more regarding why the usage of labelled arguments altogether now that we have records in place. We could even remove [@react.component] decorator altogether and keep just [@JSX] transformations?

Potentially simplifying externals as well.

56

As you well know, JSX ppx works in two parts, definition and application. @react.component is for the definition and @JSX is for the application.

  • @JSX attribute is used by parser and ppx to transform the JSX expression <Foo /> to the normal function with react api. Actually, the parser parses the JSX expression into the normal function first, then ppx transforms it again with react API such as React.createElement. It is not only a substitution of babel transformation by the compiler, also it is a necessary preprocessing for type checking.

  • @react.component has more jobs to do. It transforms the labeled function into the function with one argument, the props in the record. And it changes the function name starting with the capital letter, which is valid to react API. The last job is to make the props type for the type checker.

We can write the react function component without @react.component though, but it makes our life a lot easier. V4 transformation is using the record for the props argument, which is more clean and similar to the js representation.

57

I forgot to mention another issue that I encountered in yesterday’s tests.

  @module("@react-navigation/native") @react.component
  external make: (
    ~ref: ReactNative.Ref.t<serverContainer<'a>>=?,
    ~location: location=?,
    ~children: React.element,
  ) => React.element = "ServerContainer"

giving the error

Unbound type parameter 'a

Can this one and the issue with

  ~key: string=?,

giving the error

Two labels are named key

be fixed in the PPX?

58

Thank you for reporting the issue. I’ll look into it.
BTW, is the component implementation using React.forwardRef though?

59

You can write this today:

module Component = {
  type props = {x:string}
  let make = props => React.string(props.x)
}

let _ = <Component x="" />

The one thing it’s missing is the component will not show up as called “Component” while it would if run through the JSX.

This is kind of still in progress. But in future, we might be able to go without @react.component entirely.
We still need backwards compatibility though.

3 Likes
60

It looks very clean without @react.component now!
Component without @react.component is transpiled to the below js output.

function make(props) {
  return props.x;
}

var Component = {
  make: make
};

...
React.createElement(make, { x: "x" }) <-- make
...

I thought it would not work and throw an error which is complaining about React component function name should start with the capital letter, but it seems fine in my example project. Not sure about it. I’ll look into it too.

Btw, it looks much improved and clean now.