Don't break out of frames when frame missing

[kevinmcconnell]

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.

[dhh]

We need the allowlist concept to go along with this, so you can deal with "session expired" cases without having to write a custom turbo:frame-missing handler.

[kevinmcconnell]

@dhh yep, just decided to split that piece into its own PR. We've been chatting about a couple of different ways to specify that allowlist, so I'm opening a 2nd PR for that part now.

[dhh]

Cools cools 👌

[jonathanloos]

Hey there,

Love the idea behind the change, seems like a great way to enforce the correct patterns when implementing frames! However, as bullet number two:

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.

This change rendered most of our application essentially unusable as our (seemingly) external links now just return the explained Content missing error blocking our users from navigating throughout the site. Ultimately this is a fault on our end for improper implementation but IMO a very breaking change to introduce in a minor version.

Do you by chance have any strategies for identifying which frames/links will now be broken by this? We just finished rewriting our entire UI with Turbo and it's looking like the only way to make sure is by checking every single one...

Any help is appreciated! As mentioned, we love the change but were not expecting such an impact to our application.

@VGHOCIOT @krsyoung

[kevinmcconnell]

Hi @jonathanloos, yeah, I totally understand! Thanks for bringing this up.

It's hard to make this change without some disruption because the behaviour we want is a bit stricter than what we had in 7.2. (Prior to 7.2 it was stricter too). In 7.2. we have been emitting console warnings to try to highlight where the requests were missing their frames. But it's very easy to miss those, so it's totally understandable that you could end up with links missing frames/targets without noticing. (This situation was a big part of the motivation for the change.)

One idea that might help would be to use a global handler to catch any turbo:frame-missing events, and perform a visit in place of the default handling. That would give you effectively the same behaviour as was in 7.2. If you use an error monitoring service (like Sentry or similar) you could report these occurrences as they happen.

That should keep your app working as it did before, but now you can also see what's coming in to the error monitoring to find any links/frames that need to be updated, and you can make those changes gradually instead of all at once.

Something along these lines should work for that:

document.addEventListener("turbo:frame-missing", function(event) {
  // TODO: Report this event somewhere...

  // Visit the response
  event.preventDefault()
  event.detail.visit(event.detail.response)
})

Hope that helps!

[jonathanloos]

@kevinmcconnell we'll be taking a look into that! We appreciate the quick response as well 🙏 Definitely need to pay closer attention to the console warnings..

[jonathanloos]

Hey @kevinmcconnell !

Once again thank you for the idea behind mitigating the issues. Our team has successfully implemented the patch and are tracking the offending DOM elements using Ahoy. That way we can gather info on which turbo frames are offending and use it as a todo list in the future.

For reference and anyone also looking to track, here's the basis of our implementation (very similar to the code above!).

import { Controller } from '@hotwired/stimulus'
import ahoy from 'ahoy.js'

export default class extends Controller {
  connect () {
    document.addEventListener('turbo:frame-missing', this.handleMissingFrame)
  }

  handleMissingFrame (event) {
    this.trackAhoy(event)

    // Visit the response
    event.preventDefault()
    event.detail.visit(event.detail.response)
  }

  trackAhoy (event) {
    ahoy.track('turbo_frame_missing', event.target.id)
  }
}

Note we had to do some solid debouncing and event listener disconnecting in a stimulus controller to make sure we didn't get duplicates all over the place.

Hope that helps! Thanks again.

[kevinmcconnell]

@jonathanloos great! I'm glad to hear this worked out. Thanks for letting me know! 🙏

[jon-sully]

Howdy! Just spent a couple hours integrating this (and its companion) (and/with the Turbo-Rails companion, hotwired/turbo-rails#428) into an app with a partner of mine and we had one particularly subtle hiccup I wanted to note for others / consideration.

In 7.2 (or at least in RC3, which we were running previously), if a frame requests something and the response didn't contain the matching frame, that same response ended up being painted. It wasn't requested again.

I believe the idea with the new API is, "use turbo_page_requires_reload / the raw meta tag equivalent in the <head>" but turbo-visit-control="reload" is made to force a full page load AFAIK — so in that case it's another request but it's actually a heavier request (all assets and resources in the head will be reloaded).

There's this 'escape hatch':

document.addEventListener("turbo:frame-missing", function(event) {
  event.preventDefault()
  event.detail.visit(event.detail.response)
})

Which, based on the way visit is written under the hood, should actually paint that same response's content (nice API btw, this is clean), but a full page load does end up happening.

As we found after a couple hours of digging and experimenting is that this .visit call does do what it was written to: paint that response as-is and not re-request the page... but the problem is that that response was constructed with a minimal layout — one with a <head> that will not match the <head> of the current page and thus will cause Turbo Drive to do its own full-page-reload if you have any turbo-track="reload"s in your view.

In short, the solution we found for a frame 'breaking out' to a full page load, but using the singular response it already received as the content for that new page load (e.g. not requesting again) is to use the escape hatch above plus forcing the layout to be our application layout (which will have a matching head block):

class ApplicationController < ActionController::Base
  # ...
  layout -> { "application" if turbo_frame_request? }
  # ...
end

which should override the custom layout introduced and used by default for frame responses via hotwired/turbo-rails#428.

This is sort of a half Turbo and half Turbo-Rails concern so hopefully this is an okay place to post it, just wanted to put forth this use case for consideration and thoughts as it seemed like the event.detail.visit API was wired up carefully to re-use the same Response object (e.g. not request again) but the Turbo-rails 'minimal-layout' setup is a recipe for having a forced reload no matter what.