fix: refresh and ttl edgecases #841
Conversation
src/index.tsx
Outdated
| return | ||
| } | ||
| // user is requesting content addressed data and has the config already, render the UI | ||
| await renderUi() |
There was a problem hiding this comment.
we should likely never need to do this.. I think we can remove it. if they have the config and it's a request for contentaddresseddata (pathRequest|subdomainRequest), then we should be calling ensureSw and loadServiceWorker and reloading..? if this main page JS is running, and we are requesting content, and the service worker is registered, it should have captured the request.
we need to address this
| @@ -1,10 +1,7 @@ | |||
| import type { FrameLocator, Locator, Page } from '@playwright/test' | |||
| if (!isServiceWorkerRegistrationTTLValid()) { | ||
| log.trace('Service worker registration TTL expired, unregistering service worker') | ||
| event.waitUntil(self.registration.unregister()) | ||
| return false | ||
| } else if (isUnregisterRequest(event.request.url)) { |
There was a problem hiding this comment.
moved to after the request to prevent weird network failure issues because sw unregisters in the middle of a response
| const hasActiveWorker = registration?.active != null | ||
| const hasControllingWorker = navigator.serviceWorker.controller != null | ||
|
|
||
| if (hasActiveWorker && !hasControllingWorker) { |
There was a problem hiding this comment.
in chrome & firefox, hasActiveWorker will be true and hasControllingWorker will be false after a hard refresh.
in playwright-firefox, this is not true. it may be an out of date browser runtime?
src/index.tsx
Outdated
| window.location.reload() | ||
| return |
There was a problem hiding this comment.
💭 not sure how real risk is, but to avoid infinite reload loop we may want to do something like
// Prevent reload loops
const reloadKey = 'sw-hard-refresh-reload'
if (sessionStorage.getItem(reloadKey)) {
sessionStorage.removeItem(reloadKey)
// Already reloaded once, don't loop
return
}
sessionStorage.setItem(reloadKey, '1')
window.location.reload()
returnThere was a problem hiding this comment.
I don't think there is a risk because one flag is whether or not the service worker is registered. isHardRefresh is only true if there is an active worker, and the controller for that worker is null.. a reload of the page means the service worker (that we already confirmed exists) will capture it
Title
fix: refresh and ttl edgecases
Description
Fixes #822
Notes & open questions
I still think we need to work towards removing the registrationTTL, and work on implementing badbits. I threw some more thoughts down at #840.
Change checklist