reason-react
reason-react copied to clipboard
reasonml
Cleanup for document filenames
I'm still unclear on the file names. We have ReasonReact.re and React.re, and ReactEvent.re and ReactEventRe.re. I'm not sure what the Reason* prefix and *Re suffix indicate if anything. If there is a pattern to the naming, it would be helpful to explain that in the README. Or if the naming is actually inconsistent, then it would be nice to fix it.
That might be the occasion to only expose the current official API with modules without prefix or suffix (moving ReactDOMRe to ReactDOM for instance)
ReasonReact is the old API. Since it's pretty easy to migrate from that to the new API, I don't think it's relevant any longer. I think the documentation is fairly sufficient, but an explicit statement to that effect might be helpful.
Renaming ReactDOMRe to ReactDOM would be really nice.
Yes I agree this is a mess. Will be working to make this more consistent. Here is my current plan. Let me know if any pieces of this don't make sense.
React - continued support
ReactEvent - continued support
ReasonReact - deprecation in 0.9, remove in 1.0
ReasonReactOptimizedCreateClass - see ReasonReact
ReactDOMRe - move to ReactDOM and massive deletion of apis - pending support for bs.as in bs.obj, as well as React.jsx in the ppx and repo
ReactDOMServerRe - see ReactDOMRe
ReasonReactRouter - continued support
ReactEventRe - deprecation in 0.9, remove in 1.0
ReasonReactCompat - see ReasonReact
If all goes to plan this means that in the 1.0 release we have:
React
ReactEvent
ReactDOM
ReactDOMServer
ReasonReactRouter
Where * means that this is just bindings for a JS module or JS functionality. Reason* means that the module carries a Reason runtime.
In OCaml libraries, I follow these rules:
A single library provides a single namespace, which we realize in OCaml with a module. So there is a single top-level module. Everything else goes under this module. Usually, the top-level module consists strictly of sub-modules, not values or types, i.e. we use dune's auto-wrapping. Less frequently, it makes sense to include values and types also in the top-level module, so you manually write this top-level module. I think the latter is the correct design here. So it makes sense that there is a React module with types like component, etc. But I'm wondering if it would be right to make React.Event, React.DOM, React.DOM.Server instead of ReactEvent, ReactDOM, and ReactDOMServer? It depends also on the corresponding JS names. If ReactEvent, ReactDOM, and ReactDOMServer are consistent with the JS world, then that could be an argument for preferring those names over the dot syntax.
ReasonReactRouter violates one of our other rules, which is to not put the name of the language in the module. It is of course a Reason module, so no reason to state that in the name. It also violates the above rule that a single library provide a single name space. Could this module be called ReactRouter (or as above, React.Router). Are we not calling it ReactRouter because that would confuse it with the JS react-router library? If so, FWIW, that confusion wasn't avoided for me. For some time, until I actually used it, I assumed ReasonReactRouter is a Reason binding to JS's react-router and wondered why it is here instead of a separate bs-react-router library.
Where * means that this is just bindings for a JS module or JS functionality. Reason* means that the module carries a Reason runtime.
Sorry, I didn't understand this. Perhaps it invalidates some of my above points.
This got eventually much better on 0.11, and can be improved with dune.
There's a bit of room for improvement (fe. ReactEvent -> React.Event) which can happen at some point but those breaking changes are hard to sync and bring not much benefit rather than organisation.
