React 18

[whitchapman]

Closes #34

Per @tom-sherman's instructions, I forked his react-18 branch and added a couple of commits. I attempted to mirror the React 18.1 documentation:

• upgraded to react and react-dom 18.1
• deprecated ReactDOM.hydrate, ReactDOM.unmountComponentAtNode
• added ReactDOM.Client to encapsulate createRoot, hydrateRoot, and Root module

I'm happy to modify as needed and do any additional work to close #34, but I'm pretty new to ReScript (background in Clojure and OCaml) so please advise on the following:

• I do not know how to run tests correctly. Running node test/React__test.bs.js did not work
• Introducing the Client module is my attempt to mirror React 18.1 but may not be the proper way to do things in ReScript
• There is probably a much more efficient way to detect changes between the alpha and release of React 18 than hunting through documentation and javascript code.
• If someone with more experience lists the remaining work, I have some time to tackle it.

[jauniuseitmantis]

maybe somebody have any updates about timeline for merging this PR?

[tom-sherman]

I think these changes would be good land as a stacked diff on top of #35 (ie. after it's been merged)

@rickyvetter what do you think?

[whitchapman]

@tom-sherman I'll rebase and force push whenever #35 is merged, as well as, mark it ready for review.

[whitchapman]

I marked this ready for review for my last 2 commits. This will eventually be rebased once #35 is merged.

[cknitt]

@whitchapman @tom-sherman Thank you for your work and sorry for the very long delay. I am currently trying to clean up the repo and to move the pull requests forward.

Would it be possible to have a single React 18 PR (either this one or #35) rebased on master for review? And the other one to be closed?

[whitchapman]

@whitchapman @tom-sherman Thank you for your work and sorry for the very long delay. I am currently trying to clean up the repo and to move the pull requests forward.

Would it be possible to have a single React 18 PR (either this one or #35) rebased on master for review? And the other one to be closed?

Absolutely, whatever is easier. I forked @tom-sherman 's repo, so would it be better for me to cherry pick b254874 so that #46 is has all 5 commits? What do you think Tom?

[tom-sherman]

Sounds good!

[whitchapman]

Ok, I force pushed with all 5 commits -- I think I did it correctly; although, there are conflicts.

[whitchapman]

@tom-sherman Since my fork is from your fork of the original repo, I think you need to sync your fork first before I can sync mine -- how did this get to be so complicated!?

[cknitt]

@whitchapman Couldn't you just rebase on the lastest master (d5d9168) and force push?

[whitchapman]

My fork doesn't have that commit since Tom's fork doesn't have that commit. I would need to refork directly from rescript-lang and open a new PR -- definitely doable. I'll give it a go.

[cknitt]

IMHO it shouldn't matter if your fork has that commit, you should be able to add the rescript-lang/rescript-react repo as a remote in your locally checked out project dir and perform a fetch, then you should have that commit locally and be able to rebase on it.

[whitchapman]

Ok, @cknitt I did what you suggested which resulted in modifying the PR to remove the test changes since the test folder has been removed in master.

[cknitt]

I think that's fine if the tests have been removed in the meantime.
Looking at the changes, is everything still there that you would expect (except for the removed tests)?

CI is still failing, you need to make some small adjustments in the V3 compatibility modules to fit the changes.

[whitchapman]

Yes, my changes are as expected. It has been several months since I last looked at this, and I'm not familiar with the V3 changes. Is there a resource for the V3 changes? I can look at this later today.

[cknitt]

Looking at the CI errors, I think it's just about adapting to these changes:

• ReactDOM.Experimental.root -> ReactDOM.Client.Root.t
• useTransition does not take a transitionConfig anymore

[cknitt]

Sorry!!! Looking again at the discussion in the original PR (#35 (comment)) and also at the TypeScript typedefs for useTransition, I realize my previous comment was wrong and this definition

external useTransition: unit => (bool, (. unit => unit) => unit) = "useTransition"

is actually correct. Let's change it to this in both the React and React_V3 modules.

I think then we are done. 🎉

     * The `useTransition` hook returns two values in an array.
     *
     * The first is a boolean, React’s way of informing us whether we’re waiting for the transition to finish.
     * The second is a function that takes a callback. We can use it to tell React which state we want to defer.

[cknitt]

Looks good to me! 🎉

@tom-sherman Anything to add from your part?

[cknitt]

Ok, merging now. In case anything is still missing or incorrect we'll address it in separate PRs.