-
Notifications
You must be signed in to change notification settings - Fork 435
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
Allow Turbo Streams with GET requests #621
Conversation
Previously, Turbo Streams could only be used with non-GET form submissions. However there are also cases where it's useful to have a Turbo Stream response to a GET action. With this change, all Turbo-initiated requests will include the Turbo Stream type in the `Accept` header, so that Turbo Streams can be used with any of them.
As the author of #52, I'd like to chime in here. The scenario outlined in #52 involves a hypothetical <%# app/views/messages/new.html.erb %>
<h1>New message</h1>
<%= render partial: "form", locals: { message: @message } %> that submits to a # app/controllers/messages_controller.rb
class MessagesController < ApplicationController
def create
@message = Message.new(params.require(:message).permit(:body))
if @message.save
redirect_to @message
else
render :new, status: :unprocessable_entity
end
end
end There is more than a decade's worth of documentation and search results for Rails Controller samples that resemble this block of code. To editorialize a bit, most It's sturdy, and serves as a foundation for client-side code to progressively enhance. Globally including the Turbo Stream For example, suppose a teammate introduces the <%# app/views/messages/new.turbo_stream.erb %>
<%= turbo_stream.replace dom_id(@message, :form) do %>
<%= render partial: "form", locals: { message: @message }, formats: :html %>
<% end %> Invalid In response to invalid
However, if the Similarly, suppose a teammate were to introduce the <%= turbo_stream.replace dom_id(@message) do %>
<%= render partial: "message", locals: { message: @message }, formats: :html %>
<% end %> The The presence of the Personally, I don't have a lot of experience with transmitting With that being said, I believe that requiring applications to opt-in will lead to fewer surprises than including the header by default and requiring applications to opt-out.
My gut feeling is that we should improve the documentation around this and expand upon what's explained in hotwired/turbo-site#40. |
For example, it might not be obvious that a
In your use-case, could the singularly navigated <a href="/my-pages/1" data-turbo-frame="my-frame">Drive the frame</a>
<turbo-frame id="my-frame">
<h1><%= @page.name %></h1>
<turbo-stream action="replace" target="navigation">
<template><%= render "navigation", current_page: @page %></template>
</turbo-stream>
</turbo-frame> |
I see this conversation a lot online; of people needing/wanting to target multiple dom changes in response to GET requests. I would encourage gathering detailed use cases for this (hopefully with diagrams or even videos) and understanding why there's such a need. I think users of StimulusReflex and Cable Ready would have great insights since those tools encourage an RPC style that does not have these constraints. They tend to make very complex dom operations in response to user "GET" interactions Edit: I've shared this in the StimulusReflex server to hopefully get some insights |
@seanpdoyle thanks for chiming in! You make a good point about that existing pattern being widely used. This would be a breaking change for that example. I think there are some clear ways to solve that - like having the create action render it’s own stream response - and I think it’s conceptually clearer to separate stream responses from the request type, which is why I wanted to bring this option up for discussion. But Turbo has been working this way for quite a while so any breaking changes around this would be disruptive.
I agree, that’s definitely the safer way, and possibly the only way to do it that doesn’t involve some breaking change. What do you think about moving ahead with #612? That would make it easy to opt in without changing any default behaviour. From browsing past discussions, it seems like that specific behaviour has been requested quite a few times. |
That's definitely a possible workaround, although I think it pushes some extra complexity and inconsistency into app code, for something that seems like it should be simple to do. It requires adding a If this is an intended way to use frames & streams together then I agree that the documentation should be updated to make it clear that this can be relied on. Currently the documentation says:
..which isn't what's happening that that case. In my case, I added a little |
I'm going to close this so we can focus on solutions that don't introduce any breaking changes. Probably better to have the discussion on #612 instead, for that reason. |
Note: this is an alternative to #612
This PR makes Turbo Streams work with all Turbo HTTP requests, including
GET
requests.Motivation
Currently, Turbo restricts the HTTP use of Turbo Streams to non-
GET
form submissions. This makes sense when those stream responses correspond to server-side state changes. However there are also cases where it's useful to send targeted DOM updates when not updating server state.We already allow targeted DOM updates with any request method when using Turbo Frames. And in cases where a request affects a single DOM element, Turbo Frames will often be the most straightforward option. But when you have a change that doesn't fit within a frame, we should make it easy for people to reach for Turbo Streams instead.
Some situations where you might want a stream response to a
GET
include:<turbo-frame>
element would disrupt layout.Alternatives
Allow Turbo Streams with GET via
data-turbo-stream
#612 provides a way to opt-in to this behaviour, via a newdata-turbo-stream
attribute. That has the advantage of not changing default behaviour. The downside is that needing to use an attribute to enable streams overGET
seems non-obvious (it's not needed for Turbo Frames, for example), so we would rely on documentation to help users find it.It's possible to achieve this behaviour in application code by intercepting
turbo:before-fetch-request
to modify theAccept
header. However, putting the onus on the user to do this goes against Turbo's spirit of reducing the need for client-side JavaScript, and again, it's not obvious why it should be needed.