In the following sample code (live version), I ended up making a wrong assumption that might catch other developers by surprise:
Excerpt from index.html:
navigator.serviceWorker.register('sw.js');
navigator.serviceWorker.ready.then(() => {
// I thought the page would be controlled at this point, thanks to clients.claim()
console.log('.ready resolved, and navigator.serviceWorker.controller is', navigator.serviceWorker.controller);
navigator.serviceWorker.addEventListener('controllerchange', () => {
console.log('Okay, now things are under control. navigator.serviceWorker.controller is', navigator.serviceWorker.controller);
});
});
sw.js:
self.addEventListener('install', event => event.waitUntil(self.skipWaiting()));
self.addEventListener('activate', event => event.waitUntil(self.clients.claim()));
The assumption I had was that, by virtue of waiting on clients.claim() inside the SW's activate handler, the page will end up being controlled once .ready resolves. However, that's apparently a bad assumption, because the spec for clients.claim() states that the promise doesn't wait before resolving.
Is there any appetite for changing the behavior of clients.claim() so that the promise it returned waits until all the controller changes have taken place before it resolves? That would allow event.waitUntil(self.clients.claim()) within an activate handler to have the effect that I assumed it had, and it would by extension make the .ready promise more useful.
Well, I think .ready() resolves as soon as the active worker is set. This is before the activate event is even dispatched. The SW running the activate event is already set as registration.active. So I don't think we can conceivably wait for the activate event's waitUntil() to resolve to get the effect you were expecting.
Also FWIW, the clients.claim() spec seems buggy at the moment. For example, it says Handle Service Worker Client Unload should be run for each client in parallel. This algorithm, however, does things like:
Oh, the navigation fetch should not complete until activate waitUntil resolves, though. So I think your test case should work.
We explicitly do this waiting in firefox, but I think @matto might have told me chrome does not wait yet.
The live version above does seem to work as you expect in firefox nightly.
Sorry, I was confused. From a fresh uninstalled state I do see the behavior you describe.
After thinking about this more, I realized that what I initially proposed, with the assumptions I was making, would lead to a deadlock.
Let me rephrase this feature request: it would be useful if there were a promise exposed on navigator.serviceWorker that only resolved once the page was controlled by (any) service worker. I had incorrectly assumed that navigator.serviceWorker.ready was such a promise, and I had written a bunch of (flakey!) unit tests based on that assumption, but that's wrong, so I guess I'm asking for a new promise in addition to .ready.
Listening for controllerchange on navigator.serviceWorker is a workaround, I suppose.
FWIW, lacking formal support for such a promise in the platform, I'm adopting https://github.com/PolymerElements/platinum-sw/blob/fix-flaky-test/test/controlled-promise.js in some unit tests that need to wait until the page is controlled before they execute.
@wanderview are there technical impediments for instead of resolving .ready when the sw gets to activating, make it resolve when getting to activated?
If .ready were designed to resolve when the client got a controller, we couldn't use it for the initial client loading triggered w/o a registration and the shift + reload case (as that client will live w/o a controller for its lifetime). controllerchange is exactly the event to catch the client's controller (active worker) change.
I think changing the behavior of .ready is not a good idea. Let's discuss whether we really want to add such an API.
/cc @jakearchibald @slightlyoff
Agree with @jungkees. Reg .ready means you can use APIs that depend on an active worker, such as push & BG sync.
@jakearchibald wouldn't it make more sense to resolve .ready on activated, though? I believe we delay functional events fired at the SW until the activate event completes.
@jakearchibald , I agree but you can not use them because there is a small windows between activating and activated where you don't receive functional events (like @jeffposnick discovered). It only can when it is fully activated. So, as @wanderview suggest we should resolve .ready when activated. The other option is to stall functional events until activation completion but this could be harmful if someone is extending the event for a long time.
.ready resolution could be left as-is, and there could be a new promise, perhaps named .controlled, that resolves when there's a controller for the current page.
I see this as being useful primarily for writing unit tests, when the "progressive enhancement/I don't care if there's a controller or not" aspects of service worker isn't relevant, and you really need to delay execution until there's a controller. If you're writing tests that already use promises (because they're using fetch, for instance) then having to mix in controllerchanged event listeners as well ends up feeling messy.
The other option is to stall functional events until activation completion but this could be harmful if someone is extending the event for a long time.
To repeat, this is in the spec today. And we implement it.
Roger but please, consider the other option (resolving .ready when activated, which is more aligned with what we want to communicate) and be aware about the potential problems of stall until activation which could lead to noticeable delays in responses due to perfectly valid onactivate tasks.
@wanderview just a clarification, are all functional events stalled until activation or only fetch events?
@wanderview just a clarification, are all functional events stalled until activation or only fetch events?
All functional events. I recently got step 4 added here:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#handle-functional-event-algorithm
The fetch event waiting for activate event to complete was previously in the spec.
We delay for all functional events in gecko. I don't know what chrome does at the moment.
@delapuente
I agree but you can not use them because there is a small windows between activating and activated where you don't receive functional events
I'm not sure what you mean by "can not use". Functional events are queued. The APIs are fully functional. Can you show me a failure case?
I'm not against making .ready resolve on activation, but we need a better argument.
@jeffposnick
there could be a new promise, perhaps named .controlled
This could be implemented as:
const p = new Promise(r => {
if (navigator.serviceWorker.controller) return r();
navigator.serviceWorker.addEventListener('controllerchange', e => r());
});
I think we need more evidence that this is a common enough pattern to add to the platform.
@delapuente
be aware about the potential problems of stall until activation which could lead to noticeable delays in responses due to perfectly valid onactivate tasks
Can you show me some code that would be delayed in this way?
The "gotcha"/failure came up in the context of writing a unit test, where the code to execute _really_ had to wait until the page was controlled, or it wouldn't end up testing the right thing. I had originally written the test to wait on .ready, and then got confused when the test ended up being flaky.
I now understand why the test was flaky, and am using a synthetic .controlled promise instead.
So that's the main use case I have in mind, and I think that it's a common enough pattern for anyone writing tests. Whether we ask everyone writing tests that depend on the page being controlled to roll their own controllerchange listener, or whether it gets added natively to the spec is obviously a matter for debate.
All functional events. I recently got step 4 added here:
Thank you @wanderview , that step is what I was missing!
I'm not sure what you mean by "can not use". Functional events are queued. The APIs are fully functional. Can you show me a failure case?
@jakearchibald , my comment was added before knowing about the fact that functional events are delayed until activation. So, no problem here.
@jeffposnick can you point to the specific test? If the spec says that events are delayed and it's well implemente, I'm not able to imagine the specific scenario where activated is truly needed.
Personally I think some of the issues here stem from the name "ready" and sticking it on the ServiceWorkerContainer.
The current .ready promise is really when is the oldest ServiceWorker ready to begin receiving (maybe queueing) functional events.
It seems reasonable, though, for a page to think navigator.serviceWorker.ready is more about "when is my page's controlling service worker ready". Being on the ServiceWorkerContainer implies some relationship with the current page.
@delapuente: The test I'm referring to that was flaky when waiting on .ready: https://github.com/PolymerElements/platinum-sw/blob/4cfa76aae4aa9926ace3041fecfafb25c4da2265/test/platinum-sw-fetch/index.html#L37
It's no longer flaky since switching over to explicitly wait on the page being controlled: https://github.com/PolymerElements/platinum-sw/blob/fb5a32dd77a7ee9e4240a05e0d00a570b176fb25/test/platinum-sw-fetch/index.html#L38
It's testing behavior that relies on the service worker's fetch handler being triggered, and that will only happen if the page is controlled when it makes the network request. To reiterate my confusion, I thought that .ready's resolution implied that the page was controlled, but it doesn't actually imply that.
@delapuente: The test I'm referring to that was flaky when waiting on .ready: https://github.com/PolymerElements/platinum-sw/blob/4cfa76aae4aa9926ace3041fecfafb25c4da2265/test/platinum-sw-fetch/index.html#L37
@jeffposnick , was the problem that your test was timing out? How long was the time out? Because, per step 4 of https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#handle-functional-event-algorithm that test should pass as your fetch should be delayed until the SW is activated.
So perhaps your test was revealing a bug in Chrome.
@delapuente That page self registers, so the initial page is not controlled. If the test fetch happens before the clients.claim() finishes then there is no fetch event to be delayed. It really does need to wait until being controlled before starting the test.
So that fetch is not controlled because it's dispatched between steps 10 and 13 of https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#activation-algorithm so, after ready is resolved (step 10) but before claim finishes (step 13). Isn't it, @wanderview ?
That page self registers, so the initial page is not controlled.
The page will have its controller only after clients.claim() step 3.4.2 is executed, which is after the Activate algorithm completed. If the test fetch has already happened before that, it falls back to the network in Handle Fetch algorithm before dispatching the fetch event.
Given how easy it is to do this already (https://github.com/slightlyoff/ServiceWorker/issues/799#issuecomment-165499718), I don't think we need a specific API for it.
For people who're reading this now (through Google search), the workbox-window package now includes active and controlling promises which can be used.
Usage:
// somewhere
import {Workbox} from 'workbox-window';
const workbox = new Workbox('/service-worker.js');
// somewhere else
await workbox.active
await workbox.controlling
Excerpt from the source code at time of writing:
```
/**
/**
clients.claim() is called in theAlthough it is a very small letdown that there's no workbox.ready alias for navigator.serviceWorker.ready, to keep it all nicely tucked into one place 馃槆
Most helpful comment
@jeffposnick
This could be implemented as:
I think we need more evidence that this is a common enough pattern to add to the platform.