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

Allow opt-in to breaking out of missing frames #864

Closed
wants to merge 7 commits into from

Conversation

kevinmcconnell
Copy link
Collaborator

@kevinmcconnell kevinmcconnell commented Feb 2, 2023

This builds on #863.

When a frame response is missing its expected turbo-frame element, we consider it an error. Normally this is what we want.

In certain specific cases, though, it's better if a response without a matching frame is treated as a full page instead. The classic example of this is a login page being rendered in response to an expired session.

To allow specific paths to be opted in to full reloads when they are returned in response to a missing frame, this adds support for an allowlist specified in a meta tag in the document:

<meta name="turbo-frame-escape-paths" content="/sessions/new /sign_up">

When a response is missing its frame, we check if its path matches anything in that list. If so, we perform a full page visit.

This configuration is app-wide, and would typically be included in an application layout.

This changes the default behaviour when a frame response is missing its
expected `<turbo-frame>` element.

Previously, when the response was missing its frame, we would trigger a
`turbo:frame-missing` event, and then (provided that event wasn't
cancelled) perform a full page visit to the requested URL.

However there are cases where the full reload makes things worse:

- If the frame contents were non-critical, reloading the page can turn a
  minor bug into a major one.
- It can mask some bugs where frames were intend to explicitly navigate
  out of the frame (`target="_top"`), by incurring a second request that
  loads the page that makes it seem as if it's working corrects.
- It leaves the user at a URL that may never be capable of rendering a
  valid response (since that URL was only intended to serve a particular
  frame). That means refreshing the page is no help in getting back to a
  working state.
- It can lose other temporary state on a page, like form values.

With this change, we no longer perform the full page visit. Instead, we
handle a missing frame by doing two things:

- Write a short error message into the frame, so that the problem is
  visible on the page.
- Throw an exception, which should make the problem quite obvious in
  development, and which allows it to be easily gathered by exception
  monitoring tools in production.

We keep the `turbo:frame-missing` event exactly as before, so
applications can still hook in to perform alternative behaviour if they
want.
Also remove `invalidate` that has effectively been replaced.
When a frame response is missing its expected `turbo-frame` element, we
consider it an error. Normally this is what we want.

In certain cases, though, it's better if a response without a matching
frame is treated as a full page instead. The classic example of this is
a login page being rendered in response to an expired session.

To allow this, we provide a way for a page to mark itself as
"breakoutable" by including a specific meta tag in its head:

    <meta name="turbo-frame-missing" content="visit">

When a response is missing its expected frame, but includes that meta
tag, we'll perform a full-page visit to it, rather than throw an error.
Rather than use a meta tag in the response, we can put a central list of
"breakoutable" paths in a single meta tag, and include that on
requesting pages.

Assuming that an application will have a small number of these paths, it
might be simpler to centralise them in one place and include that in a
shared layout.
Base automatically changed from frames/no-breakout to main February 7, 2023 12:06
@kevinmcconnell
Copy link
Collaborator Author

We decided to take a different approach to this: #867 lets us solve the problem without introducing new configuration options.

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.

2 participants