-
Notifications
You must be signed in to change notification settings - Fork 380
We need this as well for proper 0.14 server support. #118
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
Conversation
|
We'll update this PR with the new gem version once we push that. |
|
@rstudner @dylangrafmyre Let's update the react_on_rail gem soon and push to heroku. |
|
@justin808 what do you mean? The react_on_rails gem has already been updated to 1.0.0.pre as of last night for master. |
|
Cool! Just trying to keep up! Closing this one! Rocking! |
|
@justin808 Not sure why this was closed. ReactDOM has been added to the client.base, but exposig ReactDOMServer still needs to go into the server.rails no? (all this PR is for) |
|
@rstudner You are correct. I was under the impression that @dylangrafmyre did this. Please rebase these changes on top of master, and if the build passes, let's merge. I'm curious as to how the builds worked if this was not there. |
I think because old renderers are deprecated, not removed from core React package. |
|
@alexfedoseev is correct. ReactDOM & ReactDOMServer are not required for 0.14, you just get deprecation warnings. Thus, the build will pass just fine without the change. |
|
@justin808 rebased against master and pushed, build passed. |
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.
There's something wrong with this PR, as this is already on master:
https://github.com/shakacode/react-webpack-rails-tutorial/blob/master/Gemfile.lock#L356
@rstudner Any ideas?
|
@josiasds @dylangrafmyre This line is not right: We should not be using ReactDom from the server. See examples in this directory: https://github.com/shakacode/react_on_rails/tree/master/spec/dummy/client |
|
Sorry for the confusion @josiasds @dylangrafmyre, but my prior comment was incorrect. I just noticed that it is 'client.base' and not just 'base'. The line above is right. |
We need this as well for proper 0.14 server support.
Have to expose ReactDOMServer as well for the server rendering per 0.14 upgrade docs.