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

Hooks using OCaml idioms and equality semantics #154

Closed glennsl closed 2 years ago

glennsl commented 2 years ago

Builds on #141, Addresses #153

This adds a use_effect hook that uses OCaml equality semantics, and partly because of that also provides a much nicer API, in my opinion.

I also added a use_ref hook using an actual ref, because I needed that for use_effect and got tired of having to dealing with React.Ref when OCaml already has a first class ref cell.

If this API style agrees with y'all too I can go on to add equivalent hooks for use_memo and use_callback. I also suppose this is a good opportunity to move the old bindings to the reason-react compatibility layer.

jchavarri commented 2 years ago

@glennsl I am confused by this PR, a lot of the changes were already part of https://github.com/ml-in-barcelona/jsoo-react/pull/141 right?

glennsl commented 2 years ago

Yes, sorry, I have no idea why I based it off that. I'll rebase it shortly, but it's just the three last commits that are relevant.

glennsl commented 2 years ago

CI is fun, huh? Ubuntu fails almost immediately with some fetch error, Windows complains about formatting while MacOS is fine with the exact same formatting...

jchavarri commented 2 years ago

The ubuntu issue seems more on Github side (based on "Not Found [IP: 52.252.75.106 80]" error).

But the formatting one? maybe some bug in ocamlformat? 🤔 https://github.com/ocaml-ppx/ocamlformat/issues/1773

glennsl commented 2 years ago

I think it's with the Ubuntu repositories. It was reported now on Discord as well. It'll probably sort itself out eventually.

The windows formatting error seems to be just with this PR, in files that it doesn't even touch. And it seems to be complaining about blank lines following end-of-line comments, which (annoyingly) is enforced by the formatter locally. I know because I tried to remove them too. Maybe by some fluke this CI box got a newer version of ocamlformat? Will probably sort itself out next round as well then.

glennsl commented 2 years ago

I am wondering if the old bindings should remain available in Core? If not, how could it be made available in reason-react compat layer without making it public?

Good point. I wonder if virtual libraries somehow could make it possible to expose these to other "internal" libraries without making it part of the public API.

glennsl commented 2 years ago

I don't know why the ppx output is suddenly formatted differently on CI now... :shrug:

jchavarri commented 2 years ago

Thanks!

jchavarri commented 2 years ago

I don't know why the ppx output is suddenly formatted differently on CI now... 🤷

Fixed in #163 hopefully.