Skip to content

turbo:frame-missing: Re-use FetchResponse HTML#672

Merged
dhh merged 1 commit intohotwired:mainfrom
seanpdoyle:frame-missing-repurpose-response
Aug 11, 2022
Merged

turbo:frame-missing: Re-use FetchResponse HTML#672
dhh merged 1 commit intohotwired:mainfrom
seanpdoyle:frame-missing-repurpose-response

Conversation

@seanpdoyle
Copy link
Copy Markdown
Contributor

After re-considering a comment on hotwired/turbo#445, this commit
re-purposes the response HTML from the FetchResponse instance, instead
of issuing a follow-up HTTP request.

After re-considering a comment on [hotwired#445][], this commit
re-purposes the response HTML from the `FetchResponse` instance, instead
of issuing a follow-up HTTP request.

[hotwired#445]: hotwired#445 (comment)
@dhh
Copy link
Copy Markdown
Member

dhh commented Aug 11, 2022

Nice. This will save that extra request 👍

@willcosgrove
Copy link
Copy Markdown
Contributor

Doesn't the response not include a layout? I know there was talk of always including the layout for frame requests, but it sounds like that was dismissed (hotwired/turbo-rails#232 (comment)).

I'm experiencing an issue where I have a form contained within a Turbo Frame, and on submit I want to redirect to a different page that does not contain a matching frame. What happens is:

  1. The frame handles the form submit
  2. It gets a redirect response
  3. It follows the redirect (still including the Turbo-Frame header)
  4. The document fetched as a result of the redirect has no matching frame, and no layout
  5. This response document replaces the body because it is being promoted to a full page visit. But because it has no layout, it breaks the page.

It seems like the two options to fix this are either to 1. remove the layout: false optimization for frame requests, or 2. issue two requests in this specific scenario. The first one follows the redirect with the Turbo-Frame header, sees it has no matching frame, and then performs a visit to the same URL but this time with no Turbo-Frame header so that the full layout is included.

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.

3 participants