-
Notifications
You must be signed in to change notification settings - Fork 436
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
Breaking change in 7.2, when frame response is 4xx, 5xx OR if frame is missing #841
Comments
@kevinmcconnell is working on a new feature for 7.3 where we refine this such that only paths from an allowlist, like redirects to /sign_in, will break out of the frame, and other 4xx and 5xx responses will instead stay in the frame, but fill it with an error message. That'll address the original concern where session timeouts would just fail, and the problem with 500s just causing a white screen and no user feedback. |
I should have a PR ready for that in the next couple of days. |
Awesome! Appreciate your work on this! If I can assist with testing it out or anything, please let me know |
This is a great plan! 🎉 Will it be possible to get the details of what went wrong in the console? |
Just for breadcrumbs' sake, the allow-list concept ended up getting scrapped in #864 to instead create a flag (of sorts) to put on the incoming page which forces Turbo to full-visit the page, even if the request originated from within a frame |
And with 7.3 I think this issue is closable 😁 |
Thanks for the heads-up, @jon-sully! |
On updating from Turbo 7.1.x to to 7.2.x, via turbo-rails, we were quite surprised to see frames suddenly doing full-page redirects on failures, ie. for a 4xx or 5xx response.
I'm aware of #445, and the reasoning behind the
turbo:frame-missing
event, quite useful!However, the changes introduced in #693, where Turbo now transforms 4xx and 5xx responses to full-page
Visit
s, feel like a breaking change. The minor version bump from 7.1 to 7.2 certainly broke our app 😅I've read up on the discussion in #670, and noticed this sentiment by @seanpdoyle :
I believe the changes introduced in 7.2 specifically break that “promise”: a navigation within a frame will stay within a frame
Are apps now required to add additional/custom JS by default, to handle this scenario?
Or am I simply misunderstanding something? (quite possible!)
Related:
The text was updated successfully, but these errors were encountered: