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

Fix restoring a visit without a cached snapshot available #368

Closed
wants to merge 2 commits into from

Conversation

jayohms
Copy link
Collaborator

@jayohms jayohms commented Sep 1, 2021

There may not always be a cached snapshot available when restoring a visit. We need to issue a new request in that situation.

This fixes a regression from #298. Bug report here: hotwired/turbo-android#184

cc @domchristie

@jayohms jayohms added this to the 7.0.0 milestone Sep 1, 2021
@jayohms jayohms added the bug Something isn't working label Sep 1, 2021
@domchristie
Copy link
Contributor

I think this "breaks" the intended behaviour when navigating Back from a same-page anchor with turbo caching disabled 🥴

E.g.

  1. Visit page with <meta name="turbo-cache-control" content="no-cache">
  2. Tap "Skip" to jump the the main content
  3. Tap Back

There won't be a cached snapshot, so it'll make a request, even though it should just jump back :/

@jayohms
Copy link
Collaborator Author

jayohms commented Sep 3, 2021

Hmmm, that is a dilemma 🤔. Let me think about this a bit more. The existing behavior breaks the native apps, so we definitely need to address that aspect.

@domchristie
Copy link
Contributor

I'm not familiar with Android logs, so I'm not sure I understand the problem. Could you describe the flow that is causing the error?

@jayohms
Copy link
Collaborator Author

jayohms commented Sep 7, 2021

@domchristie I've outlined the issue here: hotwired/turbo-android#185

I was actually able to resolve the issue in the Android library and the iOS library isn't affected by this bug, so there shouldn't need to be any additional changes necessary here. Let me know if that doesn't make sense or you have any questions. Thanks!

@jayohms jayohms closed this Sep 7, 2021
@jayohms jayohms deleted the fix-restore-without-snapshot branch September 7, 2021 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

2 participants