View helper to render react component

[quark-zju]

Let's discuss here and if you like the idea, we can talk about details and I will convert this issue into a PR.

To render a react component, currently it requires a <div> and a line of js calling
React.renderComponent. Developers need also take care of JSON and div id, etc. This feels not elegant in Ruby world.

Inspired by other helper method like content_tag, select_tag etc. I think it may be a good idea to make one for react. So a developer can simply write this in view template:

# Take the first example at http://facebook.github.io/react/index.html
render_react_component 'HelloMessage', :name => 'John'

I have a proposed implement:

  def render_react_component(name, params = {}, html_options = {})
    html_options.reverse_merge!{:id => "#{name}-#{SecureRandom::uuid}"}

    html = content_tag(:div, '', html_options)
    html << javascript_tag(
        "$(document).on('ready page:load',function(){" \
          "React.renderComponent(" \
            "#{name}(#{params.to_json})," \
            "document.getElementById('#{html_options[:id]}')" \
          ");" \
        "});",
        defer: 'defer'
    )
    html
  end

Future work could be: optionally use React.renderComponentToString to render initial html at server side.

Thoughts?

[zpao]

I think this is an awesome idea, though I'm not totally sold on the details.

Some concerns:

• jQuery - I'd rather not tie us to that if it can be avoided
• :div - It doesn't actually need to be a div, but that's just what ends up happening a lot. Maybe we can make it an option and default to div?
• How does this play with turbolinks (is that what page:load is for?)?
• Should we exclude children for the time being and only focus on top level components?

[quark-zju]

Thanks for your comments! The above implementation is just a draft. About your concerns:

• Good point and removing jQuery is easy.
• Makes sence but there is a little problem: where to put the tag option? params and html_options do not sound correct place. 3 hashes feel too many. Probably introduce an options and move html_options to options[:html]?
• Sadly it doesn't work as expected. A quick debug indicates turbolinks may have a bug handling inline scripts. If that is the case, I will try to fix turbolinks first.
• Do you mean server rendering stuff? I think so. This PR won't contain (complex) server-rendering stuff. However, I'd like to make it possible to custom static children like content_tag below, for being able to write crawler-freindly code.

<%= content_tag :div, class: "strong" do -%>
  Hello world!
<% end -%>

[jtmalinowski]

👍

[zpao]

Maybe 2 helpers? render_react_component (which uses a <div>) and render_react_component_into_tag which takes an extra arg for :div, :span, etc?

Maybe there's another option for turbolinks... I haven't looked at how Rails does it's built-in JSy stuff works (eg. confirm dialogs) but when I looked years ago they were all inline scripts. I'm guessing that's not the case anymore, but maybe there are some ideas we can borrow for solving this. Or just fix turbolinks :)

For children, I actually meant something like this: React.renderComponent(<Parent><Child /></Parent>, node); There's no way to represent this with what you have. Static children is a good first step following this, but it would be great to be able to next complex structures. Someday.

I think we should leave server rendering out entirely for the moment, but I like where you're headed.

[quark-zju]

I have attached some code :) Have a look and if no big problem, I am going to write some test and update README.

Notes:

• render_* sounds like a controller method, and is long. now it is shorter: react_component, in ActionView.
• To make things simple, and consider there may be other non-HTML options like whether to use server rendering, I combined the tag option and previously html_options to one options. It seems there is no tag attribute. If somebody do need use it, use string as key: options['tags'].
• Since DOMContentLoaded is used, IE <9 is not supported. I think it's okay and will note this in README later to tell the developer to polyfill it if IE8 support is needed.
• Turbolinks stuff now works. It didn't work because there was no removeEventListener, and an uncaught exception because getElementById returns null. Somehow the exception didn't show up in my browser console so I didn't notice it.
• About the children stuff, I would argue it's not needed, or it is too complex/tricky and doesn't worth to implement XHP-style stuff in Ruby. In practise, people can always abstract XHP-style fragments to new react classes, or use plain jsx without this helper.

[zpao]

arguments.callee is deprecated so let's not use it and just use f.

Otherwise, I think my only other comment is that I don't like the JS style. Don't let that spacebar scare you :P

[quark-zju]

Umm, travis status is still unavailable. I doubt whether it runs (I guess it probably need something like apt-get libqt-webkit-dev). I am going to sleep now. If everything is okay I am going to squeeze the commits.

[zpao]

Sorry, I haven't had a chance to look at this again, but hopefully I'll have a chance over the weekend. I'll try to figure out the Travis issue too.

Don't feel obligated to squash. I don't mind a merge commit for something like this where there are substantial changes in each commit. If it were a 10 line diff with 10 commits, then I might have an issue ;)

[quark-zju]

It doesn't matter. I mean I am not hurry, at all. Take your time. Enjoy the weekend and don't be bothered with work stuff :)

[quark-zju]

I found capybara-webkit is not travis-friendly and switched to poltergeist, which uses external phantomjs. After some trial and error it's working now 😃 There may be some wording issues because English is not my native language. Except that, I think it's merge-ready.

[quark-zju]

I'd like to write my thoughts and plans here. Recently I am busy with my graduation and this situation will probably continue for another several weeks. I will finish this PR after that.

The implementation and tests are complete. However, the inline script approach doesn't feel good. Something like using data attributes and initialize them dynamically like jquery-ujs feels better.

Cons:

• Won't work out-of-box. Something like = require react-ujs is required in application.js.

Pros:

• Generated HTML is more elegant.
• It's simple so users don't have to use the helper method. They can write plain HTML, for some (e.g. performance) reason.
• More flexible, users can replace / hook stock react-ujs, to support ancient browsers, for example.
• Play well with Content Security Policy.
• Cache-friendly. No more random ids generated server-side.

Comments? And if this is going to happen, do you think it should belong to another gem like react-ujs (I personally object to it)?

EDIT: Added 2 items in Pros. jquery-rails contains both jquery and jquery-ujs.

[zpao]

I have no opposition to evolving the pattern. So if inline script tags is how we start and we make it better later, fine by me. I'm not writing Rails much anymore so I'm not a good person to decide. But personally, I'd rather have 1 less dependency to have to figure out if I were using this, so all in here seems like the best thing to me.

[quark-zju]

Umm... I am a little confused. Do you mean "react-ujs.js" by "1 less dependency"? I would argue it is an existing practise in rails' jquery and is an optional dependency.

The react and react-ujs pattern is inspired by jquery and jquery-ujs, where the first one is the unmodified version of upstream library and the second one is rails-specific "unobtrusive javascript" thing allowing users to use the library without writing js in view templates (for example, link_to ..., :alert => 'Are you sure?' expands to something like <a ... data-confirm='Are you sure?' />, the real alert box logic is implemented in a jquery-ujs. You can confirm it by inspecting the delete comment button on this github page). Developers decide which one (or both) to include in their application.js. Some developers don't like the ujs stuff, they can simply remove = require ...-ujs line.

To ease developers' life, follow the common practise of other rails js/css gem, a rake task react:install can be provided. It adds 2 lines to application.js.

[zpao]

Oh I see. I didn't realize jquery-rails included both of them, I thought there was another gem dependency, which is what I was against.

In that case, then I think it's fine to have 2 distinct lines in application.js.

[johnthethird]

I have been playing around with your view helper and have one that implements server rendering via ExecJS, which is part of the Asset Pipeline in Rails so everyone should have it. It pulls in a file called components.js from the asset pipeline, which should contain all of your React components and support code, and uses that to server-render the HTML, and then just pops a script tag in to mount the component client-side. Any feedback on the approach is welcome.

https://github.com/johnthethird/react-rails/blob/server-rendering/lib/react/rails/view_helper.rb

[jtmalinowski]

I thought it was gonna be much harder.
One thing I would add is that you don't have to pack all components into components.js, you could just assume that say: MySpecialComponent is in a file called my_special_component.js and let the asset pipeline do the rest.
Another thing is (I haven't tackled that idea in my projects yet) is that sometimes I would want to fetch some data from an http api (otherwise the component that you are rendering will be empty). Implementing that, would give us both faster rendering on the client and (very important!) SEO improvements.

[johnthethird]

Thanks for the feedback. The reason I wanted all the components in one JS is so that I could load it once into the JS VM and cache it, then whenever you need to render its just running the React.renderComponentToString function in the pre-loaded VM. Obviously during development this is not ideal, so it should probably only cache in production.

As far as getting data, why not get it in Ruby on the server and just pass it in? I suppose thats code duplication if you also want to do the same thing on the client. One idea would be to wrap the external data in a Rails API, and for server rendering you grab it directly from Ruby, and on the client you hit the API. Or, maybe you can run the HTTP GET from inside the JS VM in getInitialState or something. Not sure how that would work in multi-threaded Rubies. Kind of a mind-bender!

[quark-zju]

I think it's better to provide an option for filenames, like config.assets.precompile. The user can set it to %w(components.js), %w(component_a.js component_b.js) or Dir[Rails.root.join('app/assets/javascripts/react/*')].map{|f| File.basename(f)}, etc. Use %w(application.js) as the default value so everything works out of box.

JS runtime like therubyracer doesn't have network support. But it's easy to workaround, get the data in Ruby code and pass it as a prop named like initialData.

Anyway, I'd like to implement the ujs stuff first because both renderings can benifit.

[johnthethird]

Im not sure what the advantage is to having it configurable. The helper needs all React components loaded in order to work, so convention over configuration here might make sense. You can always make components.js a Sprockets file that just does //= require_tree ./components/ or whatever your specific needs are. I don't think loading application.js is a good idea since there may be all kinds of stuff in there that may cause problems when loaded into a non-browser environment. By explicitly having a separate components.js file with React-specific code, it cuts down on the surface area for problems.

I like @quark-zju unobtrusive idea, and took a crack at it here:

johnthethird@6ea19d1

Any data- attributes you put on the node get passed in as props to the component.

[quark-zju]

About configurable filenames:

• //= require_tree in both application.js (Rails default) and components.js will cause duplicated code. It's not friendly for new-comers.
• Not all components need to be rendered server-side. Some components like ADs, or something meaningless to the crawler like fb messenger / google hangouts may not need to be rendered server-side. Therefore, the helper doesn't need all react components.
• Just react components may not be enough. They may depend on other libraries or browser features. For example, using markdown to format text, using localStorage to sync across tabs. In this case, server side js context need to include the library and some dummy shim to make the code using browser feature run.
• "convention over configuration" is a famous saying in Rails. However looking at Rails itself, it provides so many configurations. For example, the user can use a.js instead of application.js. Rails' default configuration is good enough to work out-of-box. But Rails won't stop you from deeply configuring it. Following this pattern, the filename should be configurable. Making it work out-of-box is hard. application.js solves 3rd dependencies problem but introduces browser features problem as you said. Maybe mocking document and window is a step towards a final solution.

[johnthethird]

Sprockets is smart enough not to duplicate code with multiple require_trees (at least in Rails4 which I just tested). Making the filename configurable with a sensible default would be fine. I am still against pulling in application.js, but will try it out on a few of my apps to see what the issues may be.

[quark-zju]

@johnthethird If application.js doesn't make sense, I am okay with components.js as default or no default value. Anyway, if browser features are used, server rendering becomes difficult. This branch is already big enough so server rendering is probably in another branch.

@zpao react_ujs is now complete and this branch is probably ready to be merged. Could you take a look?

[zpao]

This has potential for problems (if a component lives outside of the turbolinked node, or however that works), but I think this is fine for an initial version. But beware bug reports :)

[quark-zju]

Turbolinks simply replaces entire body in all cases. Components outside body sounds strange. So I think it's okay. For DOM nodes created by script on the fly, they are not mounted by react_ujs. While jquery_ujs uses $.live so it can handle dynamically created nodes. However, monitoring DOM changes is expensive. It's probably fine to let the user manually call React.renderComponent on dynamically created nodes.

[zpao]

My mistake. I assumed turbolinks let you operate on a more precise scale. I haven't actually upgraded anything to use it yet...

[zpao]

@johnthethird @JakubMal, would you be willing to do a once-over here? My brain is very much outside the rails world these days and would appreciate some help :)

[zpao]

We can actually default to null for the props (that's actually what we do in the JSX transform to avoid extra object allocations).

[johnthethird]

This looks good to me. Nice work @quark-zju.

When you pull this in I will toss over a PR for a server-rendering component which uses an object pool to allow multiple threads access to a pool of JS VMs with the React code preloaded. Seems to be working well in mri/jruby with puma/torquebox/trinidad (multi-threaded Rails app servers).

https://gist.github.com/johnthethird/8773084

[jtmalinowski]

@quark-zju @zpao Seems fine, but I'll take another look tomorrow. I just did a dumb take ours + take theirs rebase of this against my PR, and all tests passed. 👍

[zpao]

👍