-
Notifications
You must be signed in to change notification settings - Fork 159
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
Server-side template rendering with Handlebars & HandlebarsAssets. #103
base: master
Are you sure you want to change the base?
Conversation
using ExecJS and the handlebars javascript source the handlebars templates is rendered on the server objective is to keep templates DRY. same template on the client and server
* enables the use of `respond_with(@object)` to render Handlebars templates on the server side. * opt-in by require/include. It is not loaded by default into handlebars_assets.
…in order to be less susceptible to implementation changes.
Sprockets renders show.html.hbs to show.html, and then refuses to include the file because it’s text/html. So we work around it, and require that the file is named {action}.js.hbs or {action}.jst.hbs
Adds responder
All server components are in the HandlebarsAssets::Server namespace and are not loaded by default. `require ‘handlebars_assets/server` will activate the server-side rendering components.
Get server-side rendering working with the latest version
variables = variable_names.inject({}) { |acc,name| acc[name.sub(/^@/, "")] = controller.instance_variable_get(name); acc } | ||
variables.merge!(local_assigns) | ||
|
||
HandlebarsAssets::Handlebars.render(#{template.source.inspect}, variables).html_safe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing I didn't really like about this is that the JS context has already compiled this template (especially if precompile is on); I tried hooking this into the sprockets side but it really is a hack as well... I am not sure of the runtime speed of a view like this (which is the thing I was trying to make better).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree about your speed concerns. For my purposes, in any case, the HTML is a fallback for generating pages to be consumed by clients that don't support Javascript (GoogleBot, I'm looking at you.)
The controller I've implemented it within also serves JSON that the web app uses with Handlebars to render on the client side, which is much better overall. I'll be caching the hell out of 'static' HTML generated, and expect the usage of the HTML-only versions to be rather low.
Given the numbers I'm seeing from generating the HTML server side on my development box (200-500ms in views), there's definitely a lot of room to improve before I'd recommend using it for any large scale system without an effective caching strategy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only other concern which I thought of last night (but had already been on the way to bed) was that the context might not be thread safe.
I did just update my tilt_action_view repo; so I might replace the code with that after (as it is just a setup of a module that I could put in the same server file). Runtime performance is a good idea to verify as well (I have not bothered checking); there is clearly some improvements that are possible to do (e.g. check if we have already compiled that source before -- remember about local dev though for changes!) The other kicker I would want to do is add ActionView helpers to the regular templates (which I assume is a problem you will run into soon). |
At least in development, precompile:assets generates a file in app/assets/templates/**/{action}.jst. Knowing the rules of naming and the location of the compiled file, we could probably do a simple File.exist? on the name we assume it will have, and compile using that if it exists. I'd want to expose a configuration option whether or not to use cached compiled files in development, however, and default it to false. I don't know if the individual files are generated in other environments. That would be a blocker. Another possible option is to cache the compiled template ourselves. It still results in duplication of effort between the two pipelines, but would avoid template recompilation on subsequent requests. My preference is to use the Rails cache to store it, rather than the filesystem. |
This is built on the work of @deepak, @zohararad, @AlexRiedler and @variousauthors in #46. Only the controller/responder code is my own.
I'm putting this up mainly for feedback and improvement, and perhaps paving the way for inclusion in the main gem or development as an enhancement gem.
As far as I am aware it only supports get requests on a single resource (i.e. 'show') at the moment; I have tested that.
It adds support for the use of
respond_with(@object)
style rendering when paired withrespond_to :hbs
(or :handlebars), eliminating the need for either of the following cases as implemented in #46:It does require a
:to_hbs
or:to_handlebars
method on your resource, as it is used by ActionController::Responder to convert the resource.It supports responding to multiple mime types (
respond_to :hbs, :json
) within the same controller and action, as well.I don't know if it works with hamlbars or slimbars yet, but I'm interested in implementing that support, as I will likely need it in the near future.
No tests, but I'll see what I can do about adding tests for the code as soon as possible.
Finally, I tried to keep the server-side code as separate as possible. It is not loaded by default when the gem is included in a project, and needs to be required using
require 'handlebars_assets/server'
and included into a controller before it fully activates.