workbox icon indicating copy to clipboard operation
workbox copied to clipboard

workbox-window's getSW() should resolve with shift-reload

Open scytacki opened this issue 5 years ago • 2 comments

Library Affected: workbox-window

Browser & Platform: "all browsers"

Issue or Feature Request Description: When there is an active service worker and a user does a shift reload. the getSW() promise never resolves. I would expect it to return the active (but not controlling) service worker.

The docs say:

If, at registration time, there's already an active or waiting service worker with a matching script URL, it will be used...

In the case of a shift reload with an existing active service worker, there is an active service worker at registration time with a matching script URL.
More specifically, in Workbox#register, _registration.active will be set to the worker. At the same time navigator.serviceWorker.controller will be falsey. This combination is not handled by Workbox#register.

I'm trying to use Workbox#getSW() so I don't need to look at the installing, waiting, and active fields of the registration just to find the service worker. In order to handle this shift reload case, I need still need to check these fields.

scytacki avatar Mar 21 '21 05:03 scytacki

A related issue happens when the page is reloaded while a first time service worker is installing. In the reloaded page navigator.serviceWorker.controller will be falsey, and _registration.installing will be set. Workbox#getSW() does not resolve for basically the same reason as above.

Additionally the registration updatefound event won't fire because the service worker was in the installing state before the page was reloaded. So the _onUpdateFound handler also won't have a chance to resolve getSW().

scytacki avatar Mar 22 '21 01:03 scytacki

Regarding the first comment, yes, that makes sense, and sounds like a bug in our current implementation. I believe that we should resolve both getSW() and the active promise when there's already an active service worker associated with the current registration that isn't in control of the current page, i.e. because someone shift-reloaded. That matches the documented behavior, so I would categorize this as a bug fix, rather than a breaking change.

Regarding the second comment, this is a bit trickier, because it falls into the category of what workbox-window treats as an "externally triggered" update:

https://github.com/GoogleChrome/workbox/blob/2ddef47bbd8a44d3e1ce5e8b13598e022b0b99c7/packages/workbox-window/src/Workbox.ts#L377-L380

We've moved away for much of the special treatment around "external" service workers in Workbox v6, but this is still something that I think we'd need to put more thought into before changing, and if it did change, would be considered a breaking change.

jeffposnick avatar Apr 23 '21 20:04 jeffposnick