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

Missing Turbo-Frame header when submitting a form #86

Closed
Intrepidd opened this issue Jan 7, 2021 · 16 comments · Fixed by #166
Closed

Missing Turbo-Frame header when submitting a form #86

Intrepidd opened this issue Jan 7, 2021 · 16 comments · Fixed by #166

Comments

@Intrepidd
Copy link
Contributor

When clicking links while inside a frame, the Turbo-Frame header will be set.

This helps the backend to know this is a frame request and for instance to not render layout.

When submitting a form, this header is not set, I think it should be.

Example : To respect progressive enhancement, we should be able to return a frame when the request comes from turbo, but perform a redirection when not.

@kaspth
Copy link

kaspth commented Jan 7, 2021

Adding the Turbo-Frame header on non-frame requests would blur the concept of frames.

For invalid forms, we recommend responding with the 422 HTTP status and the form contents will be replaced. See #39. You can also respond with turbo streams.

@Intrepidd
Copy link
Contributor Author

Sorry if it wasn't clear, I was advocating to set the Turbo-Frame header only when a form within a frame is being submitted

@kaspth
Copy link

kaspth commented Jan 7, 2021

Ah! Yeah, that should be set, and if not, it's a bug.

@weaverryan
Copy link

Hi!

If this is a bug, then I can also confirm it :). Inside the a frame, when I click a link, I can see the Turbo-Frame header. When I submit a form right below that link (inside the same frame), the Turbo-Frame header appears to be missing.

Cheers!

@tleish
Copy link
Contributor

tleish commented Jan 14, 2021

In the following example, only the first GET button will include a request header with Turbo-Frame, the other 2 do not include it in the header.

<%= turbo_frame_tag 'user_frame' do %>
  <%= button_to('Edit User', '/users/1', method: "get") %>    <!-- ADDS header['Turbo-Frame'] --> 
  <%= button_to('Save',      '/users/1', method: "post") %>   <!-- NO header['Turbo-Frame'] --> 
  <%= button_to('Delete',    '/users/1', method: "delete") %> <!-- NO header['Turbo-Frame'] --> 
<% end %>

Is this intended or a bug?

using: @hotwired/turbo 7.0.0-beta.3

@danjac
Copy link

danjac commented Jan 22, 2021

I can confirm this issue (using Django):

  1. Request inside turbo frame using a link: Turbo-Frame header present.
  2. Request inside turbo frame using a form submission: Turbo-Frame header not present.

@tleish
Copy link
Contributor

tleish commented Jan 30, 2021

If you prefer an alternative approach than PR #110 I'm happy to work on it.

@oleksandr
Copy link

Run into the same issue today (using hotwire with Golang backend). Looks like the suggested fix is still hanging :-\

@seanpdoyle
Copy link
Contributor

As part of #142, we changed how the headers are constructed from a merge operation to a direct mutation. Combined with #127, all form submissions (GET or POST, inside a frame or referencing one from the outside) should result in a requests with the Turbo-Frame header.

That behavior was shipped as part of https://github.com/hotwired/turbo/releases/tag/v7.0.0-beta.4.

@Intrepidd @danjac @oleksandr are you still experiencing this issue?

@jmfederico
Copy link

I am still experiencing the issue in beta-4.

GET (links) send the header.
POST (form submissions) do not.

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Feb 7, 2021
Closes hotwired#86

Recursively walk the tree of dependent FetchRequestDelegates so that
they all have an opportunity to modify the fetch request headers.

Testing adds listeners for `turbo:before-fetch-request` and
`turbo:before-fetch-response` so that the event logs can drain.

Co-authored-by: tleish <tleish@users.noreply.github.com>
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Feb 7, 2021
Closes hotwired#86

Recursively walk the tree of dependent FetchRequestDelegates so that
they all have an opportunity to modify the fetch request headers.

Testing adds listeners for `turbo:before-fetch-request` and
`turbo:before-fetch-response` so that the event logs can drain.

Co-authored-by: tleish <tleish@users.noreply.github.com>
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Feb 7, 2021
Closes hotwired#86
Closes hotwired#110

Recursively walk the tree of dependent FetchRequestDelegates so that
they all have an opportunity to modify the fetch request headers.

Testing adds listeners for `turbo:before-fetch-request` and
`turbo:before-fetch-response` so that the event logs can drain.

Co-authored-by: tleish <tleish@users.noreply.github.com>
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Feb 7, 2021
Closes hotwired#86
Closes hotwired#110

Recursively walk the tree of dependent FetchRequestDelegates so that
they all have an opportunity to modify the fetch request headers.

Testing adds listeners for `turbo:before-fetch-request` and
`turbo:before-fetch-response` so that the event logs can drain.

Co-authored-by: tleish <tleish@users.noreply.github.com>
@tleish
Copy link
Contributor

tleish commented Feb 7, 2021

@seanpdoyle — created this JSFiddle to show the headers being sent. Click any of the buttons and see the headers sent through fetch below the buttons. Turbo only adds turbo-frame to header on a GET.

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Feb 7, 2021
Closes hotwired#86
Closes hotwired#110

When submitting a Form that is within a `<turbo-frame>` or targets a
`<turbo-frame>`, ensure that the `Turbo-Frame` header is present.

Testing adds listeners for `turbo:before-fetch-request` and
`turbo:before-fetch-response` so that the event logs can drain.

Co-authored-by: tleish <tleish@users.noreply.github.com>
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Feb 7, 2021
Closes hotwired#86
Closes hotwired#110

When submitting a Form that is within a `<turbo-frame>` or targets a
`<turbo-frame>`, ensure that the `Turbo-Frame` header is present.

Since the constructive-style
`FetchRequestDelegate.additionalHeadersForRequest()` was replaced by the
mutative style `FetchRequestDelegate.prepareHeadersForRequest()`, this
commit changes the `FetchRequest.headers` property to be `readonly` and
created at constructor-time so that its values can be mutated prior to
the request's submission.

Testing adds listeners for `turbo:before-fetch-request` and
`turbo:before-fetch-response` so that the event logs can drain.

Co-authored-by: tleish <tleish@users.noreply.github.com>
@tleish
Copy link
Contributor

tleish commented Feb 7, 2021

One other item to note, should the GET request also include text/vnd.turbo-stream.html in the accept header (which it currently does not)?

@seanpdoyle
Copy link
Contributor

seanpdoyle commented Feb 7, 2021

@tleish that's intentional: see #52, #127, and hotwired/turbo-site#40 (comment).

@tleish
Copy link
Contributor

tleish commented Feb 7, 2021

@tleish that's intentional: see #52 and #127.

Ah. I did not realize this was intentional. I also didn't realize I could not use turbo-stream with a GET method.

@danjac
Copy link

danjac commented Feb 7, 2021

@seanpdoyle using beta 4. Turbo header is not sent with POST request, only GET.

@Intrepidd
Copy link
Contributor Author

I confirm as well, header is still not sent for my POST use-case

seanpdoyle added a commit that referenced this issue Feb 12, 2021
Closes #86
Closes #110

When submitting a Form that is within a `<turbo-frame>` or targets a
`<turbo-frame>`, ensure that the `Turbo-Frame` header is present.

Since the constructive-style
`FetchRequestDelegate.additionalHeadersForRequest()` was replaced by the
mutative style `FetchRequestDelegate.prepareHeadersForRequest()`, this
commit changes the `FetchRequest.headers` property to be `readonly` and
created at constructor-time so that its values can be mutated prior to
the request's submission.

Testing adds listeners for `turbo:before-fetch-request` and
`turbo:before-fetch-response` so that the event logs can drain.

Co-authored-by: tleish <tleish@users.noreply.github.com>
seanpdoyle added a commit that referenced this issue Apr 9, 2021
Closes #86
Closes #110

When submitting a Form that is within a `<turbo-frame>` or targets a
`<turbo-frame>`, ensure that the `Turbo-Frame` header is present.

Since the constructive-style
`FetchRequestDelegate.additionalHeadersForRequest()` was replaced by the
mutative style `FetchRequestDelegate.prepareHeadersForRequest()`, this
commit introduces a readonly `FetchRequestHeaders` property created at
constructor-time so that its values can be mutated prior to the
request's submission.

Co-authored-by: tleish <tleish@users.noreply.github.com>
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Apr 9, 2021
Closes hotwired#86
Closes hotwired#110

When submitting a Form that is within a `<turbo-frame>` or targets a
`<turbo-frame>`, ensure that the `Turbo-Frame` header is present.

Since the constructive-style
`FetchRequestDelegate.additionalHeadersForRequest()` was replaced by the
mutative style `FetchRequestDelegate.prepareHeadersForRequest()`, this
commit introduces a readonly `FetchRequestHeaders` property created at
constructor-time so that its values can be mutated prior to the
request's submission.

Co-authored-by: tleish <tleish@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
8 participants