make SprocketsRenderer more extendable

[vegetabill]

• extract asset resolution logic into a overridable method: load_asset_files
• allow prerender_options to come in as a hash already from a subclass and merge in the render_function option

The background for this is that I'm using a custom server renderer subclass that uses different asset fetching logic and also uses a hash as prerender options to hold custom options to hold Flux store initialization javascript. Without this change, I had to subclass ExecJsRenderer and duplicate the code already present in SprocketsRenderer, e.g. console polyfiling and the props.to_json. With this change I can just override the asset loading and rely on this base class for everything else.

[rmosolgo]

I have a similar change coming in #430 , which provides an "Asset container" interface, something that receives filepaths and returns JS code: https://github.com/reactjs/react-rails/pull/430/files#diff-1cd07a77af52dfe22d04099826f272a9R59

(Its in order to support loading JS from a Sprockets::Environment vs Sprockets::Manifest.)

I wonder, would providing an asset container fit the bill for your custom JS?

[vegetabill]

Ah, oops I should've search for open PRs.

At this point, I can't say for sure. Long term we might move all of our assets to a gulp pipeline and so I'm not sure if we'll be using sprockets at all, so I guess I'd still prefer to have this flexibility unless you think it adds unnecessarily complexity to this object.

[vegetabill]

What was your thinking for this change? If you think it's okay to add to provide flexibility, I can fix up the merge conflicts.

[rmosolgo]

I guess I hesitate about adding another "extend-and-override" API. Sometimes it's the only way, but in general I worry about some maintainability issues:

• How do you keep people from clashing with gem internals? If you "invite" them into the class, they might add some other methods for their own behavior, and they might conflict with the gem's implementation (perhaps not immediately, but in a later version).
• The resulting class is not coherent. The behavior is spread between application code and gem internals, which makes it hard for a new reader to understand what it does and how it does it.
• It restricts gem development. Since application code is coupled to gem code, we have fewer options to refactor the gem.

That's why I've been looking for more dependency-injection-ish approaches lately. It solve a few of those problems:

• There's no clashing with gem internals
• The resulting class implements a discrete behavior
• The only requirement of the gem is to accept and use the injected behavior

All that to say, if we can cover this use case with the AssetContainer API, I'd like to do that! If not, we can add a hook (but it would also need unit tests so it's not broken in the future!).

Can you tell me a little bit about how you're loading assets now, or perhaps share some code for it?

[rmosolgo]

Another good solution might to expose some different things from the gem, for example the CONSOLE_POLYFILL or other SprocketsRenderer behaviors, so that, if you're making your own server renderer, you don't need to duplicate as much code!

[vegetabill]

Thanks for the design rationale for avoiding inheritance. I think that makes good sense.

The Asset Container idea should be a good fit for what I would need to vary so I'm going to close this for now. If, after using it, I see advantages to reusing CONSOLE_POLYFILL etc I'll open a separate PR for that.