hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.76k stars 431 forks source link

Turbo 7.1 - Turbo stream not handled when using form is using URL dot notation turbo_stream format #491

Open acetinick opened 2 years ago

acetinick commented 2 years ago

I have a form that does some live filtering using turbo streams in a form that does a requestSubmit() as you type. Since upgrading to Turbo 7.1 this approach seems to break and the browser renders the as text to the browser, instead of handling the turbo stream.

Downgrading to Turbo 7.0 it works fine.

Even when replacing with a submit button, the result is the same.

My UI: image

After typing:

image

The code I using to request the turbo stream:

          <%= form_tag vacancies_recruitment_applicant_path(@applicant, format: :turbo_stream),
                       method: :get,
                       data: {controller: "form", turbo_frame: "vacancy-search-result"} do |form| %>
            <input type="search" name="query"
                   placeholder="<%= t("common.placeholders.search") %>"
                   autocomplete="off"
                   data-action="debounced:input->form#submit"
                   class="form-control form-control-lg"/>
          <% end %>
        </div>

        <%= turbo_frame_tag "vacancy-search-result" do %>
          <%= render partial: "vacancies", locals: { vacancies: vacancies } %>
        <% end %>

I also noticed that the Turbo Frame header is not inside the request headers in my console, whereas Turbo 7.0 it was sent.

image

seanpdoyle commented 2 years ago

The code sample you've shared looks reasonable: the <form> element is rendered to declare [data-turbo-frame="vacancy-search-result"], which targets the <turbo-frame id="vacancy-search-result">.

However, the screenshot you've shared displays /recruitment/applicants/17968/vacancies.turbo_stream?query=a in the browser's URL bar.

How is that URL being populated?

If it's from the <form>, then you might have omitted a [data-turbo-action="advance"] declaration from somewhere in your code sample.

If you aren't using data-turbo-action anywhere in your sample, then it seems that the <form> submissions is being handled outside of the <turbo-frame> you're mentioning. Regardless of the cause, navigating the browser to a page with a .turbo_stream extension in the URL is likely the cause of the browser treating it as plain text.

Before we dig deeper into whether or not the change in behavior is due to the 7.0 to 7.1 upgrade, could you clarify what you think it causing the page's URL to change despite the <form> targeting a <turbo-frame> on the page?

acetinick commented 2 years ago

Hi @seanpdoyle thanks for replying.

So firstly to answer your question about what submits the form, I have a very simply Stimulus controller (form_controller.js) that will perform a .requestSubmit() when user performs input of a field. This isn't the cause of issue, because if I put a submit button in the form, it does the same thing.

image

It looks like that adding .turbo_stream format to the end of the URL is the root cause of the problem. Whereas in Turbo 7.0 with the same code it worked fine.

Here are the two scenarios.

Turbo 7.1 Below produces browser to do redirect, and shows the result as text in browser, as per screenshot. Also the Turbo Frame header is not present in the request.

<form id="vacancy_search" data-controller="form" data-turbo-frame="vacancy-search-result" action="/recruitment/applicants/17968/vacancies.turbo_stream" accept-charset="UTF-8" method="get">
            <input type="search" form="vacancy_search" name="query" placeholder="Search..." autocomplete="off" data-action="debounced:input->form#submit"class="form-control form-control-lg">
</form>

image

image

I remove the .turbo_stream in the format URL, the request sends the Turbo Frame header in the request, but it returns 500 from my server, because I my controller action expecting a turbo_stream format to respond to.

<form id="vacancy_search" data-controller="form" data-turbo-frame="vacancy-search-result" action="/recruitment/applicants/17968/vacancies" accept-charset="UTF-8" method="get">
            <input type="search" form="vacancy_search" name="query" placeholder="Search..." autocomplete="off" data-action="debounced:input->form#submit" class="form-control form-control-lg">
</form>

image

Turbo 7.0

With same HTML code as first example above with the .turbo_stream format

image

image

image

seanpdoyle commented 2 years ago

@acetinick thank you for sharing additional information.

I think I've answered my own questions:

However, the screenshot you've shared displays /recruitment/applicants/17968/vacancies.turbo_stream?query=a in the browser's URL bar.

How is that URL being populated?

If it's from the <form>, then you might have omitted a [data-turbo-action="advance"] declaration from somewhere in your code sample.

If you aren't using data-turbo-action anywhere in your sample, then it seems that the <form> submissions is being handled outside of the <turbo-frame> you're mentioning.

Since hotwired/turbo#437, Turbo guards <form> submissions against "external" submissions by calling locationIsVisitable, which calls isHTML. Failing that check results in a page-wide navigation, which explains how the browser's URL bar is being populated by the navigation in spite of the <form> targeting a <turbo-frame>.

This is a somewhat known issue (related to hotwired/turbo#480 and hotwired/turbo#385).

With that being said, one immediate way to remedy the issue is to encode the request format as a query parameter instead of a URL parameter. For <a> elements and <form method="post"> elements, that would mean encoding [href="/vacancies?format=turbo_stream"] instead of [href="/vacancies.turbo_stream"] (or [action="..."] in the case of <form> elements).

For <form method="get">, encode the parameter as an <input type="hidden" name="format" value="turbo_stream">.

That would handle the quirk of Turbo treating /vacancies.turbo_stream as an external, non-HTML resource.

That should handle it for Rails applications, since the framework treats ?format=... the same as /path.format. If that's not the case for your server-side framework, you can append the "text/vnd.turbo-stream.html" Content Type to the form submission's Accept: header:

addEventListener("turbo:before-fetch-request", (event) => {
  if (event.target instanceof HTMLFormElement) {
    const { fetchOptions: { method, headers } } = event.detail

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

Could you try one of those solutions out and report back with the results?

A (potentially user-facing) fix for this mechanism would also resolve https://github.com/hotwired/turbo/issues/185, and is worth exploring more deeply.

acetinick commented 2 years ago

Hi @seanpdoyle your solution to add a hidden field as format parameters works fine, and fixed the issue! thanks so much as we have release soon and was blocking us.

So this HTML now works for submitting the form image

Yes, we are using Rails as our backend and the dot notation format syntax is standard, so longer term would be nice to support. It's a workaround that works and can use ongoing.

i've updated issue title to reflect more the issue

tleish commented 2 years ago

@acetinick

Before turbo decides to handle a link or form request, it must determine if the link is an html page.

function isHTML(url) {
  return !!getExtension(url).match(/^(?:|\.(?:htm|html|xhtml))$/)
}

Dot notations inside directories is fine, so another solution is to end URLs with dot notations in a "/"

- /vacancies.turbo_stream?query=something
+ /vacancies.turbo_stream/?query=something

Do you have a suggestion on how javascript can determine in advance if the response form a URL in a form or link will respond with html or non-html media type (e.g. .json, .png, .anything)?

n-studio commented 2 years ago
addEventListener("turbo:before-fetch-request", (event) => {
  if (event.target instanceof HTMLFormElement) {
    const { fetchOptions: { method, headers } } = event.detail

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

Is there any reason why this behavior is not the default? Or at least for the forms with the data-turbo-streaming attribute.

Something like this should be the default behavior imo

addEventListener("turbo:before-fetch-request", (event) => {
  if (event.target instanceof HTMLFormElement) {
    const { fetchOptions: { method, headers } } = event.detail

    if (method.match(/get/i) && e.target.dataset.turboStreaming === 'true') {
      headers.Accept = [ "text/vnd.turbo-stream.html", headers.Accept ].join(", ")
    }
  }
})
tleish commented 2 years ago

@n-studio - This issue is about url formats, not the data-turbo-stream attribute. I recommend reading the thread in the original MR for an explanation as to why it's not default.

n-studio commented 2 years ago

@tleish My comment was not about data-turbo-stream being set by default.

My comment was about the Accept header being set to text/vnd.turbo-stream.html by default for all form with a get method, or at least the ones with a data-turbo-stream attribute.

tmaier commented 1 year ago

I have the same issue.

I have the following code:

    link_to t('.show_more'),
            latest_path(page: publications.next_page, format: :turbo_stream),
            data: { turbo_stream: 'true' }

This will lead to the following request:

image

When I remove the format from the code above, it will lead to the expected request.

    link_to t('.show_more'),
            latest_path(page: publications.next_page),
            data: { turbo_stream: 'true' }
image

Possible workarounds:

import { Controller } from '@hotwired/stimulus'
import { get, post } from '@rails/request.js'

// This controller is necessary to work-around an issue with Turbo Streams
// See https://github.com/hotwired/turbo/issues/491
export default class extends Controller {
  getTurboSteam(event) {
    event.preventDefault()
    get(event.target.href, {
      contentType: 'text/vnd.turbo-stream.html',
      responseKind: 'turbo-stream',
    })
  }
}
    link_to t('.show_more'),
            latest_path(page: publications.next_page),
            data: {
              controller: "myController",
              action: "click->myController#getTurboSteam",
            }