turbo: `turbo:frame-missing` event not dispatched when code is not 200
<turbo-frame id="my-frame">
<a href="/non-existing-url">Link to 404 from inside frame</a>
</turbo-frame>
If in the code above you click in the link, turbo:frame-missing is not dispatched.
From my understanding, we are inside of a frame expecting a frame in the response, so the event should be dispatched.
Is this expected from the new turbo:frame-missing event, or am I wrong?
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 18 (9 by maintainers)
Commits related to this issue
- `turbo:frame-missing`: Dispatch for 4xx and 5xx Closes https://github.com/hotwired/turbo/issues/670 Re-use the existing `2xx` and `3xx` behavior for a response to handle error responses. When the fr... — committed to seanpdoyle/turbo by seanpdoyle 2 years ago
- `turbo:frame-missing`: Dispatch for 4xx and 5xx Closes https://github.com/hotwired/turbo/issues/670 Re-use the existing `2xx` and `3xx` behavior for a response to handle error responses. When the fr... — committed to seanpdoyle/turbo by seanpdoyle 2 years ago
- `turbo:frame-missing`: Dispatch for 4xx and 5xx (#693) Closes https://github.com/hotwired/turbo/issues/670 Re-use the existing `2xx` and `3xx` behavior for a response to handle error responses. W... — committed to hotwired/turbo by seanpdoyle 2 years ago
I think the problem is that if you don’t directly setup a turbo:frame-missing handler to deal with the 404, and the frame hits one, you’re leaving the UI in a broken state with no explanation. Maybe the brokenness is contained to the frame, but that hardly makes it better? The user took an action, but got no feedback that it failed.
I think letting all error codes, and therefore all pages without the matching turbo frame, target top is the right default. Better to have the 404 take over the whole page than to fail silently. And then, if you’re a developer who wants to explicitly let 404s be contained, you can catch the event and do your custom work.
EDIT: I’ve since realized the change I describe below has already been made in https://github.com/hotwired/turbo/pull/672 (🎉) and just isn’t released into
beta.3yet! Sweet 😁. Leaving most of the rest of this comment for others who want to runbeta.2but need the event handler code below to avoid a second request.One additional note I wanted to make after playing with this yesterday is that the behavior of “no matching frame was found, just render the response that was given as a whole page navigation” isn’t quite true currently (
beta.2). In https://github.com/hotwired/turbo/pull/445/commits/1043f0def44b182e7e34bb8d631ea38fc7c3277a (one of the latter commits of https://github.com/hotwired/turbo/pull/445) the behavior was changed slightly from “render the exact content that was given” (don’t make another request to the same path) to “request the same path again but as a full visit”. I’d suggest that we may want to revert that change. (EDIT: This happened in https://github.com/hotwired/turbo/pull/672 🎉 )I can understand that re-requesting the same path would be valuable in the case where potentially only a partial was returned from the initial response, but I’ll wager that that’s very, very unlikely. If no matching frame was found I believe it likely that the response was already a full page of markup. E.g. if we’re protecting against the case where the back-end only served just a frame and we don’t want to render a layout-less page (therefore re-request the whole page as a full navigation), I think that problem is already guarded against by the notion that there is no matching frame. The odds that the back-end served back a response with no matching frame… but is still only a frame, just not the right one, seems… unlikely.
The downside of re-requesting the same path, as noted in several other threads over the last year, is that we lose any flash message/content that would’ve been present in the first response. I think that’s a notable cost.
In the meantime (and for others), the
frame-missingevent is obviously exposed and we have control to do this ourselves, so here’s the JS snippet I’m using to use the initial request’s HTML rather than re-request the same path again. This is essentially a clone of the ‘before’ from https://github.com/hotwired/turbo/pull/445/commits/1043f0def44b182e7e34bb8d631ea38fc7c3277a. I’ll dump this oncebeta.3is out and we hop on it! 👍I have been following some issues/discusions about this topic in the repo. I know there were other options on the table, but I think the decision of firing an event was the right one. And the reason is because you don’t break the implicit contract that a navigation within a frame will stay within a frame.
The benefit of taking the decision of firing an event is that you don’t break your design, but you allow developers to take the decision on their own development. So doing this, you don’t accept that responsibility, you give it to the next one in the chain, the developer.
Having said that, I don’t see any problem firing the event in a 404 or 500 errors. If the developers want the user to stay in the site, just breaking that frame, they can do it. If they want to give a error message and do a full reload, they can do it. They can check they response code and decide case by case, and the default would be the same than before these changes, but with a new option for the developers (instead of the previous console message).
In my opinion, I would fire the event always, because I see here an excelent example of what an event should be used for. And just a small section in docs to explain it, with a small example of full reload, alert of error, or ignorr and continue.