Skip to content
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

layout: false not working #38

Closed
dedman opened this issue Nov 16, 2016 · 7 comments
Closed

layout: false not working #38

dedman opened this issue Nov 16, 2016 · 7 comments

Comments

@dedman
Copy link

dedman commented Nov 16, 2016

Hey guys,

I'm having an issue where

caches_action :index, layout: false

doesn't actually render the layout correctly. It just renders the html resulting from the action, and doesn't add layout file around it.

I'm using rails 5, and I'm using 442930e on master of this repo.

I spent some time debugging, and it appears to be this commit that is causing the problem 9dd3a71

I followed the issue down to https://github.com/rails/rails/blob/v5.0.0.1/actionview/lib/action_view/renderer/template_renderer.rb#L96 where it is trying to resolve the layout, however the formats are only [:text], and it needs format [:html] to find the default layout/application.html.slim layout. When I change this line https://github.com/rails/actionpack-action_caching/blob/master/lib/action_controller/caching/actions.rb#L188 to use controller.render_to_string(text: body, layout: true)

it works as expected.

As you can probably tell from my description, I'm a little out of my depth here, so I can't really suggest a solution. Can anyone confirm that this is actually a bug?

Thanks for your time.

@dedman
Copy link
Author

dedman commented Nov 16, 2016

For anyone just looking for a quick fix, here is the monkey patch that works for me.

module ActionController
  module Caching
    module Actions
      class ActionCacheFilter
        def render_to_string(controller, body)
          controller.render_to_string(text: body, layout: true)
        end
      end
    end
  end
end

@pixeltrix
Copy link
Contributor

@dedman does the problem go away if you use ERB templates instead of Slim ?

@dedman
Copy link
Author

dedman commented Nov 16, 2016

Hey @pixeltrix no, I can still see the issue when using an ERB templates for both the layout, and view.

@pixeltrix
Copy link
Contributor

@dedman I assume that you get a deprecation warning about using :text in render_to_string?

@dedman
Copy link
Author

dedman commented Nov 16, 2016

Hey @pixeltrix yes, I'm getting the deprecation warning when using the monkeypatch above. I've tried raw, html options too and neither work as expected.

@pixeltrix
Copy link
Contributor

@dedman right, after banging my head against the wall for a couple of hours of failing to reproduce the bug in the test suite I realised that the layout was called talk_from_action.erb - i.e. no format in the name. 🤕

So that's a better workaround for the moment - just remove the format from the layout and it should find it. A simple fix would just be to change render_to_string from plain to html but that wouldn't fix the issue for other formats, e.g. XML (not sure how many apps do action caching with non-HTML templates).

@steveeq1
Copy link

I had the same problem. Downgrading to 1.1.1 fixed the problem for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants