Implement server-side rendering

[johnthethird]

Add an additional view helper react_server_component with the same interface/functionality as react_component with the exception that the body of the tag will be filled with a server rendering of the HTML markup. (Im not sure if it should be a different method name or perhaps just an argument to react_component? )

For thread-safety we use the ConnectionPool class from @mperham to control access to the JS VMs. It is performing well under load in my tests.

[zpao]

This isn't true anymore, it's sync in master which will be out in 0.9.

[quark-zju]

Follow existing pattern of Rails, I think something like :prerender => true in options is better than another method.

And here is a even more fun/crazy idea taking advantage of the existing method render:

react_component(:Foo) # render in client
render react_component(:Foo) # pre-render in server

This is tricky. But it is implementable by returning a fake Hash object with a special to_s and providing special partial_path, locals. However, it is probably confusing users so don't acturally do it.

[johnthethird]

OK, I rebased everything on top of master, and force-pushed (not sure if that was a faux pas). I implemented @quark-zju suggestion for :prerender => true instead of a separate view helper . I also made it so it should work with 0.8 and 0.9 versions of the renderComponentToString API.

[jtmalinowski]

Yeah, you have to force-push after rebase.
I think it would be valuable to be able to pass data (say JSON) to the component that will be rendered, not sure if you thought about that?

[johnthethird]

@JakubMal Yes, the view helper react_component takes a component name, and an args hash that will be passed in to the component as the initial props. This works both for server-side rendering and the client-side rendering. So something like

<%= react_component "TodoList", {:todos => ['todo1', 'todo2', 'todo3']} %>

gets turned into

<div data-react-class="TodoList" data-react-props="{&quot;todos&quot;:[&quot;todo1&quot;,&quot;todo2&quot;,&quot;todo3&quot;]}">
  ...
</div>

and then the react_ujs.js can unobtrusively turn that into a React component.

[robertfall]

I just want to chime in and say I would love to see this added to react-rails.

We're very keen to start using this fork, but obviously sticking to the authoritative source is more supported in the long run. Is there something holding up the Pull Request being merged in that I can help with?

[zpao]

@JakubMal, what do you think?

[zpao]

Also, @johnthethird, could you sign the CLA?

[johnthethird]

@zpao no problem, CLA has been signed.

[jtmalinowski]

@zpao will let you know here over the weekend

[jtmalinowski]

@robertfall this PR is tied to 1.0.0.pre, and it (1.0.0.pre) needs to be ironed out a bit first.

[robertfall]

So once 1.0.0.pre is finished up there should be no problem merging this in? Unless of course changes in 1.0.0.pre necessitate changes in this PR.

Is there anything we can help with for 1.0.0.pre?

[jtmalinowski]

@robertfall I'm testing this PR this weekend, if it's okay it'll be merged right after rebasing. See issues for problems that need to be reviewed before going 1.0.0.rc / 1.0.0

[jtmalinowski]

@johnthethird could you rebase as soon as you can? I don't want to leave this hanging for too long and I'm pretty sure other PRs will be conflicting. I'll confirm this is good to go sometime today or tomorrow, but in the meantime it would be great if you rebased.

[johnthethird]

@JakubMal OK, I rebased, added a commit to change to the sync version of renderComponentToString, ran the tests, and pushed.

[jtmalinowski]

@johnthethird
First, thanks for this! It is going to add a great value to our little react-rails! And surely you put a lot of effort into this.

I think we'll be able to iron out a couple of issues that I highlighted soon.
Also, don't hesitate to correct me wherever I'm wrong 👍

[bensmithett]

I'm having issues running a pretty simple sample app in production mode, getting this output. Don't know enough to know if I've done something stupid or if this is a real issue, either way thought I'd flag it here in case the README needs tweaking to avoid this.

[johnthethird]

@JakubMal I will review your comments and make any appropriate changes. Also, I was able to reproduce @bensmithett issue, so I will take a look at that as well.

[johnthethird]

I changed the way the initializer works, which should also fix issue #30 and #31as well. @bensmithett can you give it a try and see if it works for you now?

[bensmithett]

@johnthethird yep all good now 👍

[jtmalinowski]

@johnthethird could you PR 940c7f2 separately, if you don't mind? I doesn't really fit this PR...

[jtmalinowski]

@johnthethird I just mailed you, why did you close this?
EDIT: I'm not sure what happened here - please email me if something is not right!

[johnthethird]

I force-pushed a new commit for this feature, re-based on the work I teased out to fix #31. It would certainly look cleaner to just open a new PR, but I figured the conversation here was probably worth something.

[keithpitt]

This should also watch for files that match: "*.js.jsx.coffee"

[jtmalinowski]

This is getting lengthy, just as I didn't want. I think it'll be sensible to just merge as soon as you rebase, and then work iteratively from there. @johnthethird what do you think?

[johnthethird]

@JakubMal OK, rebased and ready to merge into master.

[petehunt]

pingping

[jtmalinowski]

Hi! @petehunt This should get merged today 👍

[jtmalinowski]

I'll submit unresolved problems from here as new issues to streamline working on this.

[keithpitt]

This is so very awesome. Thank you guys for your awesome open source work!

Internet high fives!

[bensmithett]

😄 🎊 what @keithpitt said, thanks guys!

[nelix]

<3 been waiting for this one

[knwang]

very excited about this!