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

Document Turbo Stream Accept: header changes #40

Merged
merged 3 commits into from
Mar 5, 2021

Conversation

seanpdoyle
Copy link
Contributor

Documents hotwired/turbo#52


Explain that Turbo Stream requests are only made within <form> element
submissions that are not GET requests.

Additionally, provide an example of how to turn other types of requests
into Turbo Stream requests.

@netlify
Copy link

netlify bot commented Jan 29, 2021

Deploy preview for gifted-meitner-072014 ready!

Built with commit 894fed6

https://deploy-preview-40--gifted-meitner-072014.netlify.app

@seanpdoyle seanpdoyle force-pushed the turbo-stream-headers branch from bbb683c to 2697802 Compare January 29, 2021 15:46
@t27duck
Copy link
Contributor

t27duck commented Jan 30, 2021

At least in Chrome for me, trying the example resulted in a Javascript error Cannot set property 'Accept' of undefined when trying to set detail.headers.Accept.

I was able to get it to work by changing it to detail.fetchOptions.headers.Accept = "text/vnd.turbo-stream.html"

@t27duck
Copy link
Contributor

t27duck commented Jan 30, 2021

I also noticed that 01_introduction.md mentions "This happens by intercepting all clicks on <a href> links to the same domain."

If links by default do not invoke a turbo request, then that section probably needs to be updated as well.

@seanpdoyle
Copy link
Contributor Author

@t27duck anchor initiated requests are still Turbo powered, but they're not Stream requests (that is, they're not sent with the Accept header). This is the case for anchors no matter where they are in the HTML: inside or outside of turbo-frames.


addEventListener("turbo:before-fetch-request", ({ detail }) => {
if (specialCaseRequest) {
detail.fetchOptions.headers.Accept = "text/vnd.turbo-stream.html"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we export "text/vnd.turbo-stream.html" as a const so that users dont need to hardcode it here?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 good idea!

@seanpdoyle seanpdoyle force-pushed the turbo-stream-headers branch from acf0f9c to 7e2707f Compare February 2, 2021 02:06
@rainerborene
Copy link

rainerborene commented Feb 2, 2021

@seanpdoyle Another approach that could be helpful is to force turbo_stream response format on the server side. Say we have a turbo-frame that loads an initial page via the src attribute. Until those turbo-frame events aren't implemented yet, you could do that instead:

class ReportsController < ApplicationController
  def show
    render formats: :turbo_stream if turbo_frame_request?
  end
end

```html
<body data-controller="turbo-streams">
<!-- ... -->
<a href="/streams-request" data-action="turbo:click->turbo-streams#injectHeadersIntoNextRequest">Load content via Turbo Streams</a>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly, this solution looks a bit hacky to me 😅 wouldn't it be possible to have something like:

<a href="/streams-request" data-turbo-stream="true">Load content via Turbo Streams</a>

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@santib I'm on the fence about built-in support through an attribute, since GET requests resulting in stream responses are a bit of an anti-pattern, and we've only recently removed support for them in 7.0.0.beta4.

Of course, I defer to @sstephenson and @javan to decide.

Copy link

@santib santib Feb 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for responding Sean. Why do you see it as an anti-pattern? I see a very useful use case that is the ability to have dynamically repeatable fields within a form. In my head the "Add" button would be a link_to (GET request) to a turbo stream endpoint that returns the HTML of the field to be appended to the form. Thoughts?

Copy link
Contributor Author

@seanpdoyle seanpdoyle Feb 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my head the "Add" button would be a link_to (GET request) to a turbo stream endpoint that returns the HTML of the field to be appended to the form.

Could that be achieved by combining two forms: one to make the Turbo Stream requests, and one for the Form Submission?

<form id="more-widget-fields" method="post" action="/widgets/new/fields">
  <input type="hidden" name="field" value="widget[name][]">
</form>

<form action="/widgets" method="post">
  <fieldset id="names">
    <input type="text" name="widget[name][]">
    <input type="text" name="widget[name][]">
  </fieldset>

  <button type="submit" form="more-widget-fields" name="widget[form_specific_data]" value="...">Add more rows</button>

  <button type="submit">Create Widget</button>
</form>

By making one <button> reference the <form id="more-widget-fields"> via the [form] attribute, we can drive Turbo Stream updates through a <button> nested within the <form> that intends to create a resource (a "widget" in this example).

Why do you see it as an anti-pattern?

It would be possible for the behavior to be triggered by an <a> element, but that undercuts the standard HTML semantics that an <a> element references another resource and drives navigation, and that <button> elements drive behavior on the current page.

This is a pseudocode example, and might require some more thought. The name and form_specific_data mentioned by the form are totally arbitrary and made up, but intend to demonstrate how submission data can come from the surrounding elements in the page, and could very well be generated by a form builder or Turbo Streams themselves.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seanpdoyle Turbo could also handle the dynamic form generation. It would be nice to have a built-in feature for "nested forms". By the way, I've implemented a Stimulus controller that does exactly that. https://gist.github.com/rainerborene/644da1be59ecb4f6c4c9637abf2622e0

Copy link

@santib santib Feb 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Sean, I get your point on <a> behavior, and I agree. But what if it's a button_to .. method: :get instead of an <a>? I think that would be the best solution for the nested fields use case. IMHO a POST to load another field doesn't make sense since it's not submitting any data 😅 and I think a GET would be more appropriate (even if it's not an anchor tag) since its purpose is only loading a piece of page

Update
Seems like button_to .. method: :get wouldn't work because it generates a form like

<form class="button_to" method="get" action=".."><input type="submit" value=".."></form>

I still think Turbo should be able to cover this use case in a more natural way than having a hidden form outside of the actual form 😅

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly, this solution looks a bit hacky to me 😅 wouldn't it be possible to have something like:

<a href="/streams-request" data-turbo-stream="true">Load content via Turbo Streams</a>

?

I already tried to implement this (before I found this suggestion), however this is still not a solution because Turbo visit still takes over and updates the page's url. So we'd need a way to stop that visit and just do regular AJAXy fetch.

Copy link

@santib santib Feb 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another example of what wouldn't be possible in Hotwire if we don't allow GET requests to respond with a turbo stream https://htmx.org/examples/infinite-scroll/

Other solution would be to get hotwired/turbo#146 merged. Actually, I like it more than streams. If that's the plan I'm 👍 then

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possible workaround might involve a <turbo-frame> for "operations" that you can drive with an anchor element:

<turbo-frame id="operations"></turbo-frame>

<div id="feed">
  <article id="post_1">...</article>
  <!-- ... -->
  <a href="/operations/posts?page=2" data-turbo-frame="operations">Load more</a>
</div>

Then, make sure that your response renders an HTML content type containing a <turbo-frame id="operations"> element that renders the <turbo-stream> elements to update the page:

<!-- app/views/operations/posts/index.html.erb -->
<turbo-frame id="operations">
  <turbo-stream target="feed" action="append">
    <template><!-- ... --></template>
  </turbo-stream>
</turbo-frame>

The frame will navigate, setting its src="/operations/posts?page=2" and the <turbo-stream> elements in the response will operate and then remove themselves.

You could re-purpose a single frame for most operations on the page.

Other solution would be to get hotwired/turbo#146 merged. Actually, I like it more than streams.

I agree! I think that is going to take a little while to hash out the rest of the design details.

In the interim, I think #59 is likely to ship first, which adds a turbo:before-frame-render event as a hook to modify a frame's contents so that you could roll your own rendering logic for appending and whatnot.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to get this to work using this suggestion with modals after get support didn't exist anymore the way I was using it.

https://gitlab.com/doug.stull/turbo_modal/-/compare/turbo-rails-0.5.8...turbo-rails-0.5.9

@coorasse
Copy link

coorasse commented Mar 5, 2021

I stumbled upon this while trying to implement what I thought was a simple use case:

<turbo-frame id="a">
  Container A
</turbo-frame>

<turbo-frame id="b">
  <a href="/action/that/changes/turbo/frame/a">Refresh the other container</a>
</turbo-frame>

Realising that I could not respond.to turbo_stream was quite a ❓ moment. It took me a while to realise that this was possible only for POST actions.
And I am actually not really sure how to achieve that...

@seanpdoyle
Copy link
Contributor Author

I stumbled upon this while trying to implement what I thought was a simple use case

@coorasse have you tried making the link in #b reference the #a frame through a data-turbo-frame attribute?

<turbo-frame id="a">
  Container A
</turbo-frame>

<turbo-frame id="b">
-  <a href="/action/that/changes/turbo/frame/a">Refresh the other container</a>
+  <a href="/action/that/changes/turbo/frame/a" data-turbo-frame="a">Refresh the other container</a>
</turbo-frame>

Documents hotwired/turbo#52

---

Explain that Turbo Stream requests are only made within `<form>` element
submissions that are not `GET` requests.

Additionally, provide an example of how to turn other types of requests
into Turbo Stream requests.
@seanpdoyle seanpdoyle force-pushed the turbo-stream-headers branch from 7e2707f to 7d5d49f Compare March 5, 2021 13:23
@coorasse
Copy link

coorasse commented Mar 5, 2021

Yes, this has, in fact, an impact in terms that the turbo-frame "b" is not refreshed anymore,
but the request is still a pure HTML and not TURBO_STREAM (Accept header is not changed).
the container "a" does not get refreshed.

@seanpdoyle
Copy link
Contributor Author

the container "a" does not get refreshed.

@coorasse does the response served by /action/that/changes/turbo/frame/a contain a <turbo-frame id="a"> element?

@coorasse
Copy link

coorasse commented Mar 5, 2021

It does. And now it's also working! I started feeling weird so I deleted the node_modules and rebuilt all my packages and restarted the server. Tanks for you help @seanpdoyle , really appreciated!

What if we need to refresh two or more containers?

<turbo-frame id="a">
  Container A
</turbo-frame>

<turbo-frame id="b">
  Container A
</turbo-frame>


<turbo-frame id="c">
  <a href="/action/" data-turbo-frame="a,b">Refresh the other container</a>
</turbo-frame>

of course this example is oversimplified, but we cannot merge "a" and "b" or wrap them.
The data-turbo-frame does not support multiple ids I guess?
The server cannot respond with a custom:

= turbo_stream.append 'a', render(partial: 'a')
= turbo_stream.append 'b', render(partial: 'b')

how do we deal with this case?

@seanpdoyle seanpdoyle force-pushed the turbo-stream-headers branch from 7d5d49f to 894fed6 Compare March 5, 2021 14:21
@coorasse
Copy link

coorasse commented Mar 5, 2021

Maybe is worth mentioning that since the "Accept" header is not set, the layout is rendered as well when generating the response.
In our specific case, this means a much slower response time, compared if we could skip the layout rendering.

We can still do this manually by checking if request.headers['Turbo-Frame'].present?

@seanpdoyle
Copy link
Contributor Author

seanpdoyle commented Mar 5, 2021

To quote #40 (comment):

Why do you see it as an anti-pattern?

It would be possible for the behavior to be triggered by an <a> element, but that undercuts the standard HTML semantics that an <a> element references another resource and drives navigation, and that <button> elements drive behavior on the current page.

In your use case, could you replace #c's <a> element with a <form method="post">?

<turbo-frame id="c">
-  <a href="/action/" data-turbo-frame="a,b">Refresh the other container</a>
+  <form method="post" action="/action"><button type="submit">Refresh the other container</button></form>
</turbo-frame>

Then, your response can be a turbo_frame.erb template that renders the response you shared:

= turbo_stream.append 'a', render(partial: 'a')
= turbo_stream.append 'b', render(partial: 'b')

@seanpdoyle seanpdoyle merged commit 45ef0d1 into main Mar 5, 2021
@seanpdoyle seanpdoyle deleted the turbo-stream-headers branch March 5, 2021 14:36
@coorasse
Copy link

coorasse commented Mar 5, 2021

I could, but performing a POST instead of a GET for this case, feels hacky, since there is nothing really POSTed. We are not actually changing the state of our system. But apart from this, our main issue would be that we cannot right-click on it anymore and treat it as a link 😕 (see the URL and navigate it for example)

Said that, I think you already took the time to discuss this above so I don't want to raise this topic again.
We'll go with a global:

addEventListener("turbo:before-fetch-request", (event) => {
  const { headers } = event.detail.fetchOptions || {}
  if (headers) {
    headers.Accept = ["text/vnd.turbo-stream.html", headers.Accept].join(", ")
  }
});

and solve all our problems. If I have something relevant to add to the discussion, I'll post it here. Thanks again!

@rockwellll
Copy link

So, Im currently using this approach and it works great!. However, i have a route that responds to both turbo_stream and html. When i want the format to be html the tubro_stream is responded with. How can i go about forcing the format on specific pages?

@mtomov
Copy link

mtomov commented Apr 6, 2021

@A7madXatab -> Regarding the example code from @coorasse - maybe just don't do it on a global level as suggested, but wrap it in a stimulus controller called accept_turbo_stream_controller.js and apply it only to the sections that you'd expect will send a get request that you want a turbo_stream back.

@rockwellll
Copy link

I actually had issues with that being global. And yes! your solution sounds great!. Thanks very much

@rockwellll
Copy link

@mtomov i this a good way of going about ur suggestion

import { Controller } from "stimulus"

export default class extends Controller {
    connect() {
        this.element.addEventListener("turbo:before-fetch-request", (event) => {
            const { headers } = event.detail.fetchOptions || {}
            if (headers) {
                headers.Accept = ["text/vnd.turbo-stream.html", headers.Accept].join(", ")
            }
        });
    }
}

@mtomov
Copy link

mtomov commented Apr 8, 2021

Yeah, exactly that. If you want to be diligent, you can also remove the event listener once the element is gone from html.

import { Controller } from "stimulus"

export default class extends Controller {
  initialize() {
    this.addTurboStreamHeaders = this.addTurboStreamHeaders.bind(this)
  }

  connect() {
    this.element.addEventListener(
      "turbo:before-fetch-request",
      this.addTurboStreamHeaders
    )
  }

  disconnect() {
    document.removeEventListener(
      "turbo:before-fetch-request",
      this.addTurboStreamHeaders
    )
  }

  addTurboStreamHeaders(event) {
    const { headers } = event.detail.fetchOptions || {}

    if (headers) {
      headers.Accept = ["text/vnd.turbo-stream.html", headers.Accept].join(", ")
    }
  }
}

@rockwellll
Copy link

rockwellll commented Apr 8, 2021 via email

@rockwellll
Copy link

I should apply the controller on any link/form tags that i want them to be turbo_stream requests, correct? Ahmed Khattab Lead Product Engineer www.hellotext.com i am using it here and it keeps sending out html requests <%= button_to business_inbox_conversation_path(business_id: business, id: conversation, params: { filter: params[:filter] }), method: :get, form: { class: "filter my-1 rounded-lg", controller: "accept-turbo-stream" }, data: { action: 'click->radio#check' } do %>

Is there way to fix it. Or i am doing something wrong here? because i want that form to send turbo stream requests

@rockwellll
Copy link

Thank you for your code. The thing is that, it doesn't change the format of the request @mtomov , it seems like the event turbo:before-fetch-request isn't being fired from what i noticed from the console. Any ideas?

@martinGerez
Copy link

Is it possible to add Accept header in fetch js method?
When I tried it, the TURBO_STREAM was triggered in the server but was not replaced the content in the UI.

@jduff
Copy link

jduff commented May 9, 2021

Just wanted to mention that the Stimulus controller here doesn't quite work because the turbo:before-fetch-request fires on the top level html element so the handler doesn't fire because it is being attached to the stimulus element (this.element). Adding the listener to document works, but then you essentially have a global again that is going to add the header for all the requests.

The event doesn't have any reference to element that is triggering the fetch, but fetchOptions.headers should have a key Turbo-Frame that has the name of the frame the element is targeting. You can compare that to the data-turbo-frame element that you attach the stimulus controller to.

I also tried using turbo:submit-start because I thought I could compare the reference to the FormSubmission, but that doesn't fire when the form is a get

If the turbo:before-fetch-request event had a reference to the element that triggered it that would make handling a case like this much easier.

@dush
Copy link

dush commented Jun 3, 2021

Possible workaround in Rails is to use link_to ..., method: :get. It makes POST request with turbo_stream format.

@michelson
Copy link

michelson commented Jun 26, 2021

Hi, some hack that you can make is read the request header, given a link with data-turbo-frame="x" will send a get fetch with the Turbo-frame header.

Then in controllers:

def show
   if request.headers['Turbo-Frame'].present? # or request.headers['Turbo-Frame'] == 'x'
      turbo_stream.replace(...)
   else
     render 'index' 
   end
end

note that I'm not using respond_to format since the request from is HTML

@phoozle
Copy link

phoozle commented Jul 23, 2021

Possible workaround in Rails is to use link_to ..., method: :get. It makes POST request with turbo_stream format.

Cheers this has worked very well for us! Can now remove all these stupid POST routes I've had to make for show actions.

@feliperaul
Copy link

Possible workaround in Rails is to use link_to ..., method: :get. It makes POST request with turbo_stream format.

@dush Just to confirm, your proposed solution broke on 7.1.0, right? A simple link_to ..., method: :get or even data: { turbo_method: :get } is issuing a regular HTML request for me.

@rockwellll
Copy link

Possible workaround in Rails is to use link_to ..., method: :get. It makes POST request with turbo_stream format.

@dush Just to confirm, your proposed solution broke on 7.1.0, right? A simple link_to ..., method: :get or even data: { turbo_method: :get } is issuing a regular HTML request for me.

I am using it and on tubro-rails: 7.1.0 all works great for me

              <%= link_to t("actions.transfer_ownership"),
                              new_business_transfer_ownership_path(business, to: privilege),
                              method: :get,
                              class: 'dropdown-item' %>

Any idea why your's might not be working?

@raj-patil53
Copy link

Possible workaround in Rails is to use link_to ..., method: :get. It makes POST request with turbo_stream format.

@dush Just to confirm, your proposed solution broke on 7.1.0, right? A simple link_to ..., method: :get or even data: { turbo_method: :get } is issuing a regular HTML request for me.

It is working for me after adding RailsUJS. Install @rails/ujs package and add following to your JS entry point:

import Rails from "@rails/ujs"
Rails.start()

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

Successfully merging this pull request may close these issues.