Ensure that page refreshes do not trigger a snapshot cache #1196
Conversation
Fixes a bug introduced in #1146 `exemptPageFromPreview()` adds a `<meta>` tag to the `<head>` setting `turbo-cache-control` to `no-preview`. However, since the MorphRenderer now inherits from the PageRenderer, it can update meta tags in the head and remove the `turbo-cache-control` tag. This means that the snapshot cache will be used for the next visit, which is not what we want. Specifying `shouldCacheSnapshot: false` in the `visit` options ensures that the snapshot cache is not used for the refresh visit.
|
Hey, sorry if hijacking, but related to this is a few other issues and PRs (#1186, #1188) that I've been exploring around page refreshes, in particular how they behave in Turbo Native with Turbo 8. Specifically refreshes in Turbo Native will make the native app unresponsive as the native apps will display a spinner and blank the WebView during the refresh action, more or less breaking any native apps that subscribe to refresh broadcasts: MorphingShowingSpinners.mp4I'm wondering what the reason is for exempting refreshes from the snapshot cache? I've explored actually making a snapshot of the page before a refresh in #1188 which significantly improves the Turbo Native experience, as at least the WebViews aren't blanked out while refreshing. This PR seems to re-iterate the wish to exempt refreshes from snapshots, but it's not clear to me why? |
|
@pfeiffer we've found a few issues in which rendering a cache snapshot interferes with the morphing algorithm. In particular we don't want a cached preview rendering on a morph refresh because that can override any changes to the current page, which is what we want to avoid with morphing. For example, we found that rendering a cached snapshot may result in closing a modal that was added dynamically to the page after the cache snapshot was taken. |
Wouldn't that be solved by explicitly making a new snapshot before refreshing as proposed in #1188? In my tests, I've seen the issues you describe if there was snapshot of the page taken way earlier (ie. as part of a By explicitly taking a snapshot in #1188 before refreshes, we ensure that snapshots being shown are up to date with the current state (ie modals open). |
|
@pfeiffer I've given #1188 a try but unfortunately it doesn't solve some the issues we've seen. In our apps we have some code that prepares a page to be cached on a Conceptually, page caches should happen when you leave a page, so we have the most recent snapshot if you ever come back to that location. In a page refresh you don't leave the page and there's no need to take a snapshot. But it's true that the blank page flickering is annoying and we should fix it. My preferred solution for this would be to fix hotwired/turbo-ios#136 in the native adapters, so they don't show a blank screen and a spinner when a visit targets the current page. That should make the solution independent of whether the page is cached or not and make it more robust. //cc @jayohms |
|
It looks like the iOS-specific issue is due to the I'll look at this separately, but we should be able to stop displaying the activity/progress indicator in the iOS library during same-page refreshes/visits. |
|
I've opened hotwired/turbo-ios#177 which addresses the issue; specifically skipping proposing locations if the URLs are the same (as proposed in hotwired/turbo-ios#160 by @afcapel) and on the native side skipping showing screenshots and activity indicators if the visit is a page refresh. |
Fixes a bug introduced in #1146
exemptPageFromPreview()adds a<meta>tag to the<head>settingturbo-cache-controltono-preview. However, since the MorphRenderer now inherits from the PageRenderer, it can update meta tags in the head and remove theturbo-cache-controltag. This means that the snapshot cache will be used for the next visit, which is not what we want.Specifying
shouldCacheSnapshot: falsein thevisitoptions ensures that the snapshot cache is not used for the refresh visit.