-
Notifications
You must be signed in to change notification settings - Fork 64
Topic/react 0.14 #56
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
Topic/react 0.14 #56
Conversation
Just checking the status of this or upgrading to react 0.14 in general? Thanks! |
@codedmart If the breaking changes of react 0.14 don't impact you then I believe you should be able to use react 0.14 with the purescript-react bindings at version 0.5.0. However, without the changes in this PR and @paf31 Do you think this PR looks okay to be merged? I am definitely open to suggestions and discussion on any of these changes. |
I'm playing catch-up on PRs this week after a vacation, but I'll get to this soon... |
Thanks @paf31! |
Loving the change to use the npm |
@ethul Yes, very sorry about the delay. This email has been in my inbox for weeks now 😞 If you're willing, I was going to ask you if you'd like to be maintainer or co-maintainer on |
No problem. I am happy to help maintain. I can merge these this weekend if On Saturday, 23 January 2016, Phil Freeman [email protected] wrote:
|
@ethul You should have full access to the repo now. The PR looks good to me, but I haven't spent any time with React 0.14 yet, so I'm not fully sure. If you say it's good to go, I'm sure it is. 👍 Do you think Thermite will need any big changes? |
I think it looks good, spent a little while using it and no issues. |
@paf31 Thanks for the repo access. I will double-check on what changes might be necessary for Thermite. @nwolverson Thanks for testing it out. Good to know. |
@paf31 Thermite compiles with no changes. I think the only breaking change is that |
@paf31 Also, the Thermite example may need an update since PS - Do you think moving |
Ok, sounds good. Thanks very much for working on this. When you're happy, could you please make a release? I was planning to work on Thermite today anyway, so I'll update it to be compatible. |
Great. And very welcome. I will make a release of everything, but I might not get it out until On Sunday, 24 January 2016, Phil Freeman [email protected] wrote:
|
These changes update the API and reflect the split of react and react-dom that was introduced in version 0.14.
As a result, I have a separate purescript-react-dom repository set up for the react-dom bindings. Also, I moved out the example to purescript-react-example since it depends on
purescript-react
andpurescript-react-dom
. I'd be happy to move the new repos topurescript-contrib
if we decide to go ahead with these changes.One item I wanted to open up for discussion is how react.js is imported in the foreign code. I've added a
require('react')
line to import react, and I have added apackage.json
with react as apeerDependency
for compatibility tracking. Going this route seemed appropriate given that we are using commonjs as a standard output for purescript. But making this change means tools like wepack and browserify need to be used for the JS assembly. Maybe this is an okay requirement. That being said, maybe we can make a failover during the require statement to add compatibility for react being available globally. I am open to ideas and suggests on this though.Additionally, if we merge #54, I'd be happy to resolve the conflicts if we decide to go ahead with these changes.