Decouple renderer

[rmosolgo]

This is my attempt to decouple the server renderer as described in #143 (comment)

• React::ServerRendering handles pooling & provides an interface to the rest of the gem
• It takes a renderer class who must implement #initialize(options) and #render(component_name, props)
• React::ServerRendering::SprocketsRenderer is an implementation which uses files from the asset pipeline to prerender components.

Whaddya think? Does this provide enough flexibility for issues like the #143?

Also I added guard & Guardfile, I think it's a nice addition to the development flow, does anyone mind?

[rmosolgo]

This should always be JSON because ReactRailsUJS will try to parse it as JSON!

[willcosgrove]

This looks great! I'll have to check it out tomorrow and see if I can use this API to build out something from my original example in #143.

[rmosolgo]

Somehow I broke a lot of tests 😞 gotta check that out.

[johnthethird]

+1. I would like to see a way to render with renderToStaticMarkup as well, would that be a totally different class like SprocketsStaticRenderer or an options flag options[:static_markup] = true? I am leaning towards the option, as there are 2 ways to render on the server, and you should be able to switch between them.

[vegetabill]

As an abstraction layer, I think this looks good.

The direction I took my implementation, basically to support seeding Flux stores from Rails data, reuses much of the impl of Renderer. It would be nice if there were some extension points to allow that.

For example:

   js_code = <<-JS
      (function () {
        #{preparation_js}
        var result = React.renderToString(React.createElement(#{component_name}, #{props}));
        #{post_render_js}
        return result;
      })()
   JS

    ...

  def preparation_js
  end

  def post_render_js
    @replay_console ? CONSOLE_REPLAY : ""
  end

That way I could just subclass the Sprockets render and tweak a few things.

But maybe someone more clever than me can come up with a better way to handle the whole Flux thing anyway.

[rmosolgo]

@johnthethird I made it so you can do prerender: :static and use renderToStaticString

@RearAdmiral I can jive with that, I'll do it tomorrow

[rmosolgo]

I'm stumped on this failing test -- the asset pipeline serves the right file, but the route doesn't

If i start up the dummy app, it works as expected:

And it passes on master, I don't know what I changed between here and there that would change asset delivery 😩

Tried:

• Master does pass locally
• Clearing tmp/cache more, still failed
• Removing guard, still failed

[rmosolgo]

Ugh, it's test order: if I remove the tests I added, everything runs ok.

[rmosolgo]

Asking for help rails/sprockets#51

[rmosolgo]

It ain't pretty but it's working.

My call to the asset pipeline in SprocketsRenderer (in an earlier test) caused react.js to be cached. In an Sprockets version, it's not expired correctly, so I put a workaround in the test. Asset#force? doesn't exist in later Sprockets versions, so it's never called.

I'm bummed that we have a test-order issue :S But, it's a very odd case, it only fails on old Sprockets, and the same actions work in the dummy/ app if I boot it locally, so I'm not really concerned.

[rmosolgo]

Double-checked this on a local project. Works & reloads on changes :)

[vegetabill]

I'm happy to see this merged. This is the key foundational step for a future flux-compatible plugin gem.

@rmosolgo did you ever have a chance to work on extension points for the base class? See my comment above If you haven't spent any time on it, I can try to write one and see what everyone thinks

Thanks again for pushing this through.

[rmosolgo]

Shoot, I thought I did that but obviously not :S I'm definitely open to it
though!

On Fri, May 15, 2015 at 1:15 PM, Bill DePhillips notifications@github.com
wrote:

I'm happy to see this merged. This is the key foundational step for a
future flux-compatible plugin gem.

@rmosolgo https://github.com/rmosolgo did you ever have a chance to
work on extension points for the base class? See my comment above
<#14d5937911c00187_issuecomment-97610816> If you haven't spent any time
on it, I can try to write one and see what everyone thinks

Thanks again for pushing this through.

—
Reply to this email directly or view it on GitHub
#253 (comment).

[mchristen]

This seems to have introduced a regression when trying to use prerender:true and already stringified properties being passed into react_component.

For example, if you use jbuilder to build your JSON you could in the past(without this pull request) do something like

react_component("MyComponent", render(file: "path/to/some_file.json.jbuilder"), prerender: true)

It seems that the changes in view_helper.rb are the culprit. view_helper.rb#L12 assumes that the props aren't already stringified.

[rmosolgo]

@mchristen thanks for reporting and finding the problem! I added some tests & worked up a fix here: #268

[vipulnsward]

guard throws a circular loading in progress warning