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

We need this as well for proper 0.14 server support. #118

Merged
merged 3 commits into from
Oct 14, 2015
Merged

We need this as well for proper 0.14 server support. #118

merged 3 commits into from
Oct 14, 2015

Conversation

rstudner
Copy link
Contributor

Have to expose ReactDOMServer as well for the server rendering per 0.14 upgrade docs.

@justin808
Copy link
Member

We'll update this PR with the new gem version once we push that.

@justin808
Copy link
Member

@rstudner @dylangrafmyre Let's update the react_on_rail gem soon and push to heroku.

@dylangrafmyre
Copy link
Contributor

@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.

@justin808
Copy link
Member

Cool! Just trying to keep up! Closing this one! Rocking!

@justin808 justin808 closed this Oct 13, 2015
@rstudner
Copy link
Contributor Author

@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)

@justin808 justin808 reopened this Oct 14, 2015
@justin808
Copy link
Member

@rstudner You are correct. I was under the impression that @dylangrafmyre did this.
We also need the update to the server one as well: https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/webpack.server.js#L30

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.

@alex35mil
Copy link
Member

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.

@rstudner
Copy link
Contributor Author

@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.

@rstudner
Copy link
Contributor Author

@justin808 rebased against master and pushed, build passed.

@@ -353,7 +353,7 @@ DEPENDENCIES
rails-html-sanitizer
rails_12factor
rainbow
react_on_rails (~> 0.1.8)
react_on_rails (~> 1.0.0.pre)
Copy link
Member

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?

@justin808
Copy link
Member

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.

justin808 added a commit that referenced this pull request Oct 14, 2015
We need this as well for proper 0.14 server support.
@justin808 justin808 merged commit 192c64e into shakacode:master Oct 14, 2015
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.

None yet

4 participants