search

ml-in-barcelona / jsoo-react

js_of_ocaml bindings for ReactJS. Based on ReasonReact.
https://ml-in-barcelona.github.io/jsoo-react
MIT License
138 stars 19 forks source link

Convert API to snake case #138

Closed glennsl closed 2 years ago

glennsl commented 2 years ago

While we're at it making breaking changes, I'd like to follow up with some reorganization and renaming to make the API more ergonomic, such as giving useEffect0 etc. more meaningful names, dropping unnecessary repetition and verbosity, such as in React.Dom.create_dom_element_variadic, using more idiomatic OCaml constructs such as labeled arguments instead of putting _with_X in function names, and generally try to clean things up a bit. But I'll do that in a later PR once this has been approved and merged.

glennsl commented 2 years ago

On a more generic level, my main concern is that this is effectively removing ergonomic support for Reason syntax.

This seems to conflate syntax and ergonomics with convention and familiarity. I don't think there's anything about the Reason syntax that actually makes snake_cased identifiers less ergonomic. I suppose it is in Reason's "spirit" to use the more familiar camelCased identifiers, but operating in the OCaml ecosystem with this convention has always been quite awkward and unergonomic, I think, because most of the ecosystem does not follow that convention and so you have to remember which APIs use which convention. It was only in the BuckleScript ecosystem that this worked well, because that ecosystem was built from the ground up on those conventions.

Not that familiarity can't also be a goal, of course, but even if it's feasible to maintain two different naming conventions for jsoo-react, how feasible is it really to do so for the rest of the ecosystem? One can perhaps hope that the relative popularity of Rust has made the combination of snake_case and Algol-style syntax more familiar, so that it wouldn't be that much of a hurdle.

jchavarri commented 2 years ago

I suppose it is in Reason's "spirit" to use the more familiar camelCased identifiers, but operating in the OCaml ecosystem with this convention has always been quite awkward and unergonomic,

Yeah what I meant is that historically Reason has "inherited" JavaScript camel case in the APIs. Even Js_of_ocaml has followed similar approach as well (example).

But more recent packages like Brr moves in the same direction as this PR: using OCaml snake case names everywhere, which I think makes more sense and removes confusion in the long term.

If there are users of jsoo-react that want to keep using Reason syntax and would like some "flavour" of the jsoo-react library using Reason/camel case names I think we can discuss how to implement it in a (hopefully automated) way to reduce maintenance costs.

jchavarri commented 2 years ago

Thanks again for your work on this, @glennsl !