Okay cool! Thanks for the clarification. It’s difficult for me to see where the JSX as syntax support ends and the React specific tasks of react-ppx begin.
1 Like
Makes sense. I think the escaping/not escaping question is out of scope for this conversation as it’s at the syntax layer. I’m also not sure it’s even relevant as the best way to handle this might involve converting to camelCase naturally.
Support for web components with custom props is definitely not existing in the work I’ve done so far but I think adding support for them should be straightforward. The basic idea would be to do something like:
module AmpImg = {
@react.component({wc: "amp-img"})
external make: (~src: string) => React.element = "";
};
To be clear, these externals could be written today. Just a bit messy for the author as a lot of the code is a bit creative with types.
1 Like
This is a large part of why the proposal is to do this in two steps. The v0.10 change is basically only two things: deletion of things that have been long deprecated with compat modes and migration strategies already existing, global renaming of strings. This should be a pretty painless upgrade.
The v1.0 change is newer but it all happens at the ppx layer which versions independently from the code itself. v0.10 will fully support ppx 3 and 4, and v1.0 will only support ppx 4. So your compat mode is to stay in v0.10 while iterating.
@ryyppy has already touched on the coupling specifically but I want to mention that things are already very flexible. If you want a transform you can disable the React and swap for your own.
1 Like
The example you have shown looks good to me. It’s simple and concise.
The fact is that it’s like writing bindings for react components, I like it.
Just to set expectations - I am not committing to add this to the initial ppx v4, but it’s on my radar and hopefully won’t cause much issue. Additionally it’s completely an additive change so doesn’t require a ppx version bump and can be added later.
3 Likes
-
Would it be possible for the React-PPX to ship as a separate independent
ppxinstead of shipping it with the compiler? (it can just be part of therescript-reactpackage) -
Would it be possible to configure on what sources (directories would be great I think) what
ppx's run? This would allow an incremental update, and it would also allow to mix React JSX with another JSX that is not React.
The last would also solve some issues with migration to graphql-ppx from 0.x to 1.0 in large code bases.
1 Like
The ppx is shipped with the compiler because a large percentage of people use it and it improves performance significantly (there is no overhead of creating a new process per file since it’s built in). It’s possible to ship separately but I’m not sure there is much value here directly. Is this ask only related to your second question? Or do you have other reasons?
Good question, but a little outside the scope of these ppx changes. This kind of behavior is already built into the compiler at the file level with this kind of annotation at the top of your file. The intention is for this annotation to be exceptional so it certainly isn’t the most ergonomic. I think that if you want/need this behavior more broadly then you should open a new topic or post an issue on the compiler.
@config({flags: [|"-ppx", "path/to/custom/ppx"|]});
With regards to this proposed change, I don’t expect the set of changes that need to be done in lock-step with upgrade of JSX versions to be large enough to require both to co-exist. Inside of Facebook, this change took ~2 hours of work including lock-step changes, reviewing JS source changes, and automated testing.
It would be good to get info about non-React JSX you’re planning to mix with React JSX. This is pretty rare in JS and I would expect to be rare in ReScript too. It’s possible we could build support for this directly into the JSX, but without some examples it’s hard to say.
1 Like
Do you plan to release v0.10 as reason-react or as rescript-react. I.e., will it affect projects that have something like "^0.8.0" in their package.json?
No, rescript-react will be published as a separate package, and reason-react will not receive any changes, so reason-react projects will not break. We will also keep the old docs for those who still need it.
3 Likes
I hope we upgrade soon anyway, it’s just that we have a lot of ReactDomRe usages that we haven’t quite figured out how to replace. And anyway, wanted to make sure it won’t break suddenly for us on yarn install. Thank you!
It depends a lot on what you’re using it for. If it’s a 0-runtime binding to existing react-dom code then it can still be found in ReactDOM, so it’ll be a simple rename. If it’s for aria- or data- support then you can move to JSX. If you (or anyone) is concerned about their specific situation, please give details about the situation! If you don’t feel comfortable posting here, reach out to me on Discord/Twitter.
1 Like
Well, the majority of the usages are Style, Ref and renderToElementWithId. AFAICS, only the latter is non-zero-cost. Will helpers like this be deprecated?
Anyway, I don’t think it’s gonna be a big deal for us: we can copy the helper and then it’s a simple rename as well.
1 Like
Thanks for the answers. I understand the performance implications (but it’s a trade-off as well, is it really worth the integration for a 10% performance win?). I also hope there would be a way to do something like this in the future without ppx-es that doesn’t have this trade-off (better implementation of ppx’es or macros?). But that is not relevant to this discussion.
An alternative idea is to configure it as if it’s a ppx, but reason-react is special cased (it looks at the version - or hash of the ppx binary - of reason-react, and then calls the vendored ppx directly). This also allows you to run an experimental version of the reason-react ppx while not needing a patched ReScript.
This would simplify configuration (we don’t need to have a special jsx configuration, but we just have to include the ppx)
I am currently working on a native UI, that works both on web and native. It would be nice to be able to drop it into an existing React application (but it might need two JSX transforms in the same app). So if react-jsx would behave more like a ppx, and ppx’es are configurable on a per-folder basis, it would allow these experiments/innovations. Aside from that I can also see a use-case for using JSX for other things, not necessarily UI (see for instance pastel for the command line) in an application where React is also used.
It’s also a way to make sure that everyone is using the same implementation of JSX, without much configuration and extras to install. Goal is to have a simple one-stop installation that just works, and is fully tested with the whole system (compiler, syntax, error reporting, editor-support).
react-ppx is not enabled by default, so you can write your own ppx and hook it up like any other. There is no special compiler support needed for that to work, so you could just fork the react-ppx code and build a custom ppx instead.
We don’t really want to experiment too much around with different jsx ppxes (the less options, the better), so hopefully the jsx v4 change will be stable for quite a while.
2 Likes
Reviving this one - any updates for v1.0?
Also wondering about React 18 support, is that something you’ve had a chance to have a look at? It’s mostly about binding to the new concurrent mode APIs etc, I think.
There are currently some open questions on how to proceed with the new react-ppx. Maybe there will be some fine-tunings in the compiler that will allow us to get rid of the makeProps interface. Will take some more time until we know how to proceed. v1.0 is kinda dependent on that.
Didn’t have the time to look at it (still a lot of code based on v15 / v16). According to the announcements I didn’t see many blockers that prevent users to write quick bindings to those apis?. Are there many ppl / libraries using React v18 successfully already?
Ahh, I see. Thank you for the update.
Yes, writing your own bindings is possible, so it’s no blocker. But it’d be nice to be able to leverage the official ones instead. This might be “local” to rescript-relay, where both internal APIs in the lib + external use of the lib has been concurrent mode compatible for a long time now. I’ve shipped a ReactExperimental binding for the concurrent mode API:s, but it’d be nice to finally get rid of that and move to official bindings now that the concurrent mode features are stable enough to be included in the main React package 
I didn’t have the chance to use concurrent mode yet since our codebase is still based on v15 and v16. If you are running your own v18 API bindings in production already, we can start a discussion on the rescript-react repo and start thinking of including it upstream?
We’d also need to think about concrete documentation on the topic, with proper examples etc.
Sure, let me open an issue for discussion on the repo when I get a chance.