Add option to enable service worker#2062
Conversation
|
auspice.us review app works:
|
f86ffe5 to
b28ad85
Compare
0417918 to
f169a23
Compare
1fa0c40 to
8edb28c
Compare
7bf5e3e to
5a56fc0
Compare
|
Demo using auspice.us (0:00-0:25) vs. review app (0:25-0:53): service.worker.demo.mov |
jameshadfield
left a comment
There was a problem hiding this comment.
I reviewed this with claude as I'm not an expert on service workers. It seems to work well (localhost & the auspice.us review app. I've read stories which make me wary of them due to the difficulty in undoing mistakes that have gone out, but I do see the advantage for auspice.us. Some general comments on the PR
- We need more dev docs - especially around what requests will be intercepted (bundles defined in index.html and all navigation requests in the domain) and which won't (e.g. map tiles, charon), how to test them, how to remove them etc.
- If a user has an auspice.us tab open (with the service worker running), then refreshing the tab won't get an updated auspice.us version as far as I can tell. Something to be aware of, but probably not a deal breaker for auspice.us
- Is there any anticipated use-case beyond auspice.us? The worker won't handle the charon API, so at best it's giving an auspice error page. Should we use
navigateFallbackAllowlist: [/^\/(\?.*)?$/],so that it only works for the root URL?
| navigator.serviceWorker.register('/service-worker.js') | ||
| // Check for updates on each network-connected page load. | ||
| .then((registration) => registration.update()) |
There was a problem hiding this comment.
If a user has an auspice.us tab open (with the service worker running), then refreshing the tab won't get an updated auspice.us version as far as I can tell.
It should! This is controlled by the precache, which contains all JS bundles and assets. You can see the full list in the compiled service-worker.js file which looks like:
function(e) {
C().precache(e)
}([{
url: "/dist/09172b1910dff418491f.woff2",
revision: null
}, {
// ...
url: "/dist/auspice.chunk.163.bundle.f2dc78be857ccb8ba92e.js",
revision: null
}, {
// ...
url: "/dist/index.html",
revision: "4cb1bfc61bc2a453d2d5"
}]),and is checked on each network-connected page load:
Lines 38 to 40 in 5a56fc0
There was a problem hiding this comment.
My understanding is that while registration.update() runs on a page refresh, if it updates the service worker (from A to B) then B becomes ready in a "waiting" state but the tab continues to use A. I believe index.html will also stay version A, because service worker A just does cache.match() without any revalidation. I don't have enough experience with service workers to give concrete advice here. Maybe this is all just fine?
There was a problem hiding this comment.
I've added skipWaiting: true so that B will immediately take control in all existing tabs.
Lines 131 to 133 in 56e3f03
There was a problem hiding this comment.
Nice. For completeness, this is what can go wrong with skipWaiting: true but I don't think it applies to us as requests will fallthrough to the network if not handled by the service worker
If version-A HTML lazy-loads a hashed chunk after SW B has activated — say a dynamic import, or an image the A page requests on interaction — that request goes to SW B, which no longer has version A's assets in precache. If your precache-only strategy returns nothing for an unknown URL (and the network copy is also gone because you shipped B), that fetch fails. This is the classic "
skipWaitingbreaks in-flight pages" hazard, and it's specifically a problem for cache-first/precache setups with content-hashed assets.
There was a problem hiding this comment.
Actually, this does apply to us:
the network copy is also gone because you shipped B
though it's very specifically for when someone has an old tab open, opens a new tab, and tries to use the old tab again without refreshing*.
I guess this is why clientsClaim and skipWaiting default to false. There is a flip side though, because the browser can only have one active service worker per origin: for the user who has an old tab open, they'll be stuck with the old version on the new tab too, until all tabs are closed to end the waiting phase.
In our case, I think it's better that new tabs use the latest version while breaking old tabs, than to have all tabs stuck on the old version. I'll keep both options set to true.
* could be mitigated by auto-refresh on ChunkLoadError?
5a56fc0 to
56e3f03
Compare
joverlee521
left a comment
There was a problem hiding this comment.
Will use my own time to understand service workers, but the user facing changes, both the offline behavior and the envvar to enable service workers, look good to me.
|
I'll hold off merging until at least after dev chat tomorrow. Pushed some tests for the functionality: 8f58dd8 |
Use Workbox's webpack plugin to install a simple service worker that enables offline use of the index page. This is added primarily for auspice.us, but should work for any Auspice server where assets are served at the root path. Gated behind a build-time env var setting to prevent unintended service worker installation (e.g. on nextstrain.org).
Simplifies configuration of workbox-webpack-plugin (see removed TODO).
Of all the breaking changes¹, only the script loading behavior should
apply to Auspice. Preserving existing behavior with
scriptLoading='blocking'. We may want to consider switching to the new
default ('defer') in the future.
¹ https://github.com/jantimon/html-webpack-plugin/blob/v5.0.0/CHANGELOG.md#-breaking-changes
8f58dd8 to
949ee54
Compare
Use Workbox's webpack plugin to install a simple service worker that enables offline use of the index page.
This is added primarily for auspice.us, but should work for any Auspice server where assets are served at the root path. Gated behind a build-time env var setting to prevent unintended service worker installation (e.g. on nextstrain.org).
Review threads
Checklist
AUSPICE_ENABLE_SERVICE_WORKER=trueset in Heroku installs a service worker and works offline