search

reasonml / reason-react

Reason bindings for ReactJS
https://reasonml.github.io/reason-react/
MIT License
3.25k stars 347 forks source link

chore: rename ReactDOMServerNode{Only,} #819

Closed anmonteiro closed 9 months ago

anmonteiro commented 10 months ago

follow-up to https://github.com/reasonml/reason-react/pull/815

davesnx commented 10 months ago

I liked node becaues renderToPipeableStream just works in node, eventually splitting react and react-dom seems innevitable in reason-react

I got the feeling we spend too much time on renaming things on all clients of reason-react that any change is really painful for just us.

jchavarri commented 10 months ago

Discussed offline.

Maybe we can look at it from the docs perspective https://react.dev/reference/react-dom/server.

If we follow that arrangement, it would mean we would expose 3 libs:

We can expand with bun or whatever in the future if it also has different API (I suspect it will follow Node).

anmonteiro commented 10 months ago

I wasn't aware that renderToPipeableStream only worked in Node, or about the state of react-dom/server and its different implementations.

In that case, I like Javi's idea.

anmonteiro commented 9 months ago

This PR is now just changing the name of ReactDOMServerNodeOnly to ReactDOMServerNode. NodeOnly felt like a weird name, but I don't have a strong opinion.