Skip to content
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

React 16 support #892

Merged
merged 3 commits into from
Oct 13, 2017
Merged

React 16 support #892

merged 3 commits into from
Oct 13, 2017

Conversation

anmonteiro
Copy link
Contributor

This supersedes #883

@anmonteiro
Copy link
Contributor Author

I don't know if this is ready to be merged yet.

I'm tracking down what seems like missing externs, as our app is misbehaving with advanced compilation.

@swannodette
Copy link
Member

Ok ready for this when you are.

@anmonteiro
Copy link
Contributor Author

There's still an issue here, which breaks this line:

(let [factory (js/React.createFactory (js/React.createClass rdesc))]

because React.createClass is not in React 16 anymore.

I didn't fix it because I was lazy.

We can either fix it in another PR or bring in create-react-class from CLJSJS solely for that namespace.

@anmonteiro
Copy link
Contributor Author

Other than that I fixed the needed externs issues

@anmonteiro
Copy link
Contributor Author

Also need to fix some server-side rendering stuff. We don't need to generate checksums or React IDs anymore

@dupuchba
Copy link

@anmonteiro can you plz explain why checksum/ids are not needed anymore ?
Plus, do you need help for this PR ?

@anmonteiro
Copy link
Contributor Author

@dupuchba from https://reactjs.org/blog/2017/09/26/react-v16.0.html

In addition, React 16 is better at hydrating server-rendered HTML once it reaches the client. It no longer requires the initial render to exactly match the result from the server. Instead, it will attempt to reuse as much of the existing DOM as possible. No more checksums!

@swannodette swannodette merged commit 6f0c3ee into omcljs:master Oct 13, 2017
@ItamarShDev
Copy link

Is there a plan to create a new version containing it?

@dark4eg
Copy link

dark4eg commented Dec 10, 2017

@ItamarShDev

lein install

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

Successfully merging this pull request may close these issues.

5 participants