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

Add a turbo:reload event that returns the reason why turbo needed to do a hard reload. #556

Merged
merged 8 commits into from Apr 30, 2022

Conversation

manuelpuyol
Copy link
Contributor

I feel like we are missing an event fired when the page is going to be hard reloaded. With turbo:reload users will be able to listen to why their Turbo navigations are causing reloads instead of soft-navigation. That will help users track the data and improve their apps.

@dhh
Copy link
Member

dhh commented Mar 30, 2022

I like this. @seanpdoyle thoughts on implementation?

@manuelpuyol
Copy link
Contributor Author

hey @dhh @seanpdoyle 👋
Anything I need to add to get this merged?

@seanpdoyle
Copy link
Contributor

Could you share a use-case that depends on the contents of the event.detail.reason?

I can understand the utility of the turbo:reload event, but I'm curious what types of things more information and context would enable. It's worth mentioning that whatever values we introduce here would be treated as a public API, and we'd be beholden to maintaining those values, regardless of any internal implementation changes or architectural restructuring.

@keithamus
Copy link
Contributor

@seanpdoyle that's a great question. We compute hashes for JS versions, and CSP headers, which we deliver in <meta> tags with data-turbo-track="reload" on. We want to observe when these get triggered so we can measure & potentially react to them. We'd like to differentiate those from from <meta name="turbo-visit-control"> which is far more intentional and would go in a different reporting bucket than changes to CSP headers which invalidate a Turbo request.

@manuelpuyol
Copy link
Contributor Author

I think that the granularity helps the user understand what is going on with their app. Giving that information to the user will be important so they can react accordingly.

@manuelpuyol
Copy link
Contributor Author

@seanpdoyle those cases make sense for you?

@dhh dhh merged commit 41119f9 into hotwired:main Apr 30, 2022
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.

None yet

4 participants