-
Notifications
You must be signed in to change notification settings - Fork 348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: wrap the React library #783
Conversation
@@ -0,0 +1,1117 @@ | |||
/* First time reading an OCaml/Reason/BuckleScript file? */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep ReactDOM as a toplevel, since it matches what react exposes and keep it as a good default, where 1 module from reason-react binds to one exportable from react.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would need a new library, e.g. react.dom
, which I don't think is a good tradeoff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kind of like the new library actually (since react.native) can be their own thing also...
84a4d83
to
4d81337
Compare
* feat: wrap the React library (#783) * use the react use callback fix --------- Co-authored-by: Antonio Nuno Monteiro <[email protected]>
CHANGES: * Wrap the `React` library, exposing just a single top-level module (@anmonteiro in [reasonml/reason-react#783](reasonml/reason-react#783)) * Re-organise toplevel modules (@davesnx in [reasonml/reason-react#794](reasonml/reason-react#794)) * Require and adapt to Melange v3 (@anmonteiro in [reasonml/reason-react#821](reasonml/reason-react#821))
fixes #770
supersedes #773