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

WIP: Bindings to useEffect Hook #10

Closed schinns closed 5 years ago

schinns commented 5 years ago

Addressing issue https://github.com/jchavarri/rroo/issues/6.

May-28-2019 21-45-28

schinns commented 5 years ago

@jchavarri I tried resolving conflict here, but I get an infinite loading state on the page (I wonder if it has to do with permissions?).

I pulled down and merged master and resolved conflict locally, but doesn't seem to be passing still. Do I need to undo my merge? Or are you able to resolve the conflict here?

jchavarri commented 5 years ago

hey @benschinn !

I'm not sure what problems you ran into, I was not able to repro, maybe it was some local cache thing? I opened it and it works beautifully! 😍

I fixed the conflicts and pushed also a small change of "promise-like" delay behavior with Lwt, which is OCaml's library to handle asynchronicity. I think your work with effects and this Lwt usage shows really well the potential of leveraging the OCaml ecosystem with React in a seamless way.

One thing I noticed is that the "unmount" effects that are returned in the option don't seem to run 🤔 I guess it's due to some conversion that is remaining. I think though that this PR already includes a lot of upsides on its own, so I'll open a new issue to track that separately.

Thanks again for your work @benschinn ! Very exciting to have support for effects in rroo now 🚀

schinns commented 5 years ago

@jchavarri Thanks for guiding me with the first PR! 😃

I have been thinking about what React implementation on jsoo might look like. You've done a beautiful work so far. I am looking forward to contributing more!

P.S. - I will do another PR for removing the array(unit) from the useEffect0 signature and add a useEffect0 function to the js binding.