Skip to content

Updates for React 0.13 #40

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

Merged
merged 5 commits into from
Sep 1, 2015

Conversation

ethul
Copy link
Contributor

@ethul ethul commented Sep 1, 2015

I've made some updates to make the API compatible with React 0.13.

The render functions are now a bit more bare-bones, but I'd be happy to add back some of the convenience functions like renderToBody, etc.

Also, the mkUI now returns a foreign data type representing a React class. We could make the function return a React factory (which would give us props -> UI), but perhaps mkUIFactory would be a better name for that behaviour.

These changes are definitely up for discussion. It is a first-attempt at the API. Note that I still have to make updates to the README and the tutorial. I wanted to get thoughts on the API changes first.

PS - I've moved react to a dev-dependency. I am wondering if we can allow the end user to be responsible for ensuring react is present. There are some use-cases where bringing in react through bower is unnecessary (e.g., if webpack is being used).

Refactored the API to match React 0.13.

Resolves purescript-contrib#34
@paf31
Copy link
Contributor

paf31 commented Sep 1, 2015

Looks good to me, thanks!

@paf31
Copy link
Contributor

paf31 commented Sep 1, 2015

Maybe I can reimplement my React Native bindings on top of this too, as well as Thermite.

@ethul
Copy link
Contributor Author

ethul commented Sep 1, 2015

Great! I will update the readme and tutorial accordingly.

On Monday, August 31, 2015, Phil Freeman notifications@github.com wrote:

Maybe I can reimplement my React Native bindings on top of this too, as
well as Thermite.


Reply to this email directly or view it on GitHub
#40 (comment)
.

@ethul
Copy link
Contributor Author

ethul commented Sep 1, 2015

I've updated the README. But I am wondering if this is still the best spot for a tutorial. Should we keep this?

@paf31
Copy link
Contributor

paf31 commented Sep 1, 2015

I only kept the tutorial because I was doing the minimum to update for 0.7, but I think you're right and it can be removed now.

@ethul
Copy link
Contributor Author

ethul commented Sep 1, 2015

I've removed the tutorial.

The only other thing I was thinking about was naming. Is it work transitioning to names like ReactElement and ReactClass, etc., instead of UI and UIClass?

I am okay with names as they are, but wondered if it might be worth the switch.

@paf31
Copy link
Contributor

paf31 commented Sep 1, 2015

Yes we should break as many things as we can before the major version bump I guess. I think renaming to React* is a worthwhile change.

@ethul
Copy link
Contributor Author

ethul commented Sep 1, 2015

Agreed. I will make the updates to the names accordingly.

On Tuesday, September 1, 2015, Phil Freeman notifications@github.com
wrote:

Yes we should break as many things as we can before the major version bump
I guess. I think renaming to React* is a worthwhile change.


Reply to this email directly or view it on GitHub
#40 (comment)
.

@ethul
Copy link
Contributor Author

ethul commented Sep 1, 2015

Rename done. Let me know if there is anything you'd like me to incorporate into this PR.

Thanks!

paf31 added a commit that referenced this pull request Sep 1, 2015
@paf31 paf31 merged commit 28321c5 into purescript-contrib:master Sep 1, 2015
@paf31
Copy link
Contributor

paf31 commented Sep 1, 2015

💯 Thanks!

@ethul
Copy link
Contributor Author

ethul commented Sep 1, 2015

Thank you!

On Tuesday, September 1, 2015, Phil Freeman notifications@github.com
wrote:

[image: 💯] Thanks!


Reply to this email directly or view it on GitHub
#40 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants