ml-in-barcelona / jsoo-react

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

consistent project-wide formatting #155

Closed glennsl closed 2 years ago

glennsl commented 2 years ago

This started with me wanting to pin ocamlformat to a specific version because #154 failed CI despite not failing locally because of a new version introducing a different. Then I went on to unify all the .ocamlformat files into one project-wide configuration, and finally added some tweaks to it that I've found to work well, because I found the code I added in #154 got absolutely butchered compared to the version I have in my own project.

For comparison:

let use_effect ~on f =
  let last_value = React.use_ref on in

  (* runs initially *)
  React.use_effect0 (fun () ->
      f ();
      None);

  (* runs always *)
  React.use_effect (fun () ->
      if on <> React.Ref.current last_value then begin
        React.Ref.set_current last_value on;
        f ()
      end;
      None)
let use_effect ~on f =
  let last_value = Core.use_ref on in
  Core.use_effect_once (fun () -> f () ; None) ;
  Core.use_effect_always (fun () ->
      if on <> Core.Ref.current last_value then (
        Core.Ref.set_current last_value on ;
        f () ) ;
      None )

The reasoning for the configuration choices is as follows:

Feel free to disagree though! I find all of these add value individually, and would rather have either one of them than none at all.

Also, this obviously can't be merged unless all other PRs have already been merged, or it would create a huge mess. So Draft for now.

glennsl commented 2 years ago

Rebased on main

jchavarri commented 2 years ago

Thanks!