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

Response fails to render (client-side) if view files don't contain .html in the name #22

Closed
cmartyn opened this issue Dec 23, 2020 · 2 comments

Comments

@cmartyn
Copy link

cmartyn commented Dec 23, 2020

We have a relatively large Rails codebase that uses the slim templating language along with Turbolinks (including native Turbolinks wrappers for iOS and Android). In general, it has worked great for us for years! (Thanks!)

But when I upgraded to Turbo, all our links appeared to stop working: The URL would change, and the client would make the request to the server (per browser console), and the server would render a response (per the Rails logs), but the DOM would never be updated with the content rendered by the server.

After a lot of trial and error, I finally figured out why: Our view files are generally named file_name.slim rather than file_name.html.slim. There was one link I could find that did work, and it happened to be one of the few files that included the .html extension in its name.

Using this naming convention for our view files hasn't caused us issues before, so it's technically a regression from Turbolinks 5. I'm not sure if this is a common pattern that could affect lots of people, or if we're the only app doing it and we've been doing it wrong for years.

I figured it was worth posting here to both clarify our upgrade path (should we bite the bullet and do a massive file rename, or will this be treated as a bug to be fixed?) and to save others time if they encounter the same issue.

@dhh
Copy link
Member

dhh commented Dec 25, 2020

That's a good catch. It's caused by the need to switch rendering formats when streams are rendered, see

rendering.delete(:content) || (rendering.any? ? render_format(:html, **rendering) : nil)
. Can only force that switch if the mime type is present in the template.

Not sure the best way to catch/alert about this. Maybe there's a way to catch the exception in

def render_format(format, **rendering)
ApplicationController.render(formats: [ format ], **rendering)
end
and rethrow with better error messaging? Please do investigate!

@dhh dhh closed this as completed Dec 25, 2020
@dvisockas
Copy link

@cmartyn, after a weekend of digging found a rather hacky fix: remove 'turbo-stream;' from Content-Type headers:

if response.headers['Content-Type'].include? 'text/html'
  response.headers['Content-Type'].remove!('turbo-stream;')
end

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