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

Provide a one-line way to listen for a waiting Service Worker #1222

Open
dfabulich opened this issue Nov 13, 2017 · 24 comments
Open

Provide a one-line way to listen for a waiting Service Worker #1222

dfabulich opened this issue Nov 13, 2017 · 24 comments

Comments

@dfabulich
Copy link

dfabulich commented Nov 13, 2017

It's a common use case to pop up a notification to end users when a new Service Worker is waiting, but it's inconvenient to listen for that event.

ServiceWorkerRegistration offers an onupdatefound event, but that fires when the new Service Worker is detected and installation has started, not when the new Service Worker is fully installed and waiting to take control. In other words, it notifies us when registration.installing changes, but not when registration.waiting changes.

It's still possible to await a change in registration.waiting by attaching a listener to the onstatechange event of the .installing Service Worker, and to wait for an .installing Service Worker by waiting for the onupdatefound event of the registration itself, like this:

function listenForWaitingServiceWorker(reg, callback) {
  function awaitStateChange() {
    reg.installing.addEventListener('statechange', function() {
      if (this.state === 'installed') callback(reg);
    });
  }
  if (reg.waiting) return callback(reg);
  if (reg.installing) awaitStateChange();
  reg.addEventListener('updatefound', awaitStateChange);
}

This function gets its own special quiz in the Udacity "Offline Web Applications" course; it seems like this cow path could be paved.

I propose adding a .waiting Promise on the ServiceWorkerContainer, and/or an onwaiting event to ServiceWorkerRegistration. I'd love a one-liner like this:

navigator.serviceWorker.waiting.then(alertUser);

Combined with #1016, you could write code like this:

navigator.serviceWorker.addEventListener('controllerchange', () => window.location.reload());
navigator.serviceWorker.waiting.then(reg => {
  if (confirm('refresh now?')) reg.waiting.skipWaiting();
});

(Uh, don't actually use confirm() in production; it's just convenient for this example.)

@dfabulich
Copy link
Author

I suggest this "prolyfill" (a forward-looking polyfill for behavior to be eventually standardized)

if (!('waiting' in navigator.serviceWorker)) {
  navigator.serviceWorker.waiting = new Promise(function(resolve) {
    navigator.serviceWorker.ready.then(function(reg) {
      function awaitStateChange() {
        reg.installing.addEventListener('statechange', function() {
          if (this.state === 'installed') resolve(reg);
        });
      }
      if (reg.waiting) resolve(reg);
      if (reg.installing) awaitStateChange();
      reg.addEventListener('updatefound', awaitStateChange);
    })
  });
}

@gauntface
Copy link

Two cents. I'm not sold that navigator.serviceWorker.waiting is the right naming here.

I would actually prefer some way of listener for changes for the service worker that updating along the lines of:

reg.addEventListener('updatestatechange', (event) => {
  switch(event.state) {
    case 'installing':
      break;
    case 'waiting':
      break;
    case 'activating':
      break;
    case 'activated':
      break;
  }
});

@dfabulich
Copy link
Author

  1. I suggested .waiting to parallel .ready
  2. If the new API is an event, instead of a promise, I'd prefer the event to fire from navigator.serviceWorker rather than the registration. ServiceWorkerRegistration's updatefound event tells you about the installing SW, and navigator.serviceWorker's controllerchange event tells you about the activated SW. This API feels inconsistent; it's slightly more convenient to listen to navigator.serviceWorker than it is to get the registration and listen to that.
  3. I don't think a full-blown updatestatechange event would be very useful. I can't think of any use case for knowing when there's a Service Worker activating but not yet activated. (What would client-side code do in response to that?) But I can't even think of a use case for knowing when the SW is installing, except so as to listen for its state changes to wait for it to start waiting. If I'm right that the only interesting states on the client side are waiting and activated, and we already have a solid event for activated, then it's just the waiting state that would benefit from sugar.
  4. Promises are just nicer than event listeners.

It's hard to beat this one-liner: navigator.serviceWorker.waiting.then(alertUser)

This is definitely uglier.

navigator.serviceWorker.ready.then(function(reg) {
  reg.addEventListener('updatestatechange', function(event) {
    if (event.state === 'waiting') alertUser(event.registration);
  });
});

If updatestatechange were on navigator.serviceWorker, it would be acceptable.

navigator.serviceWorker.addEventListener('updatestatechange', function(event) {
    if (event.state === 'waiting') alertUser(event.registration);
  });
});

But at that point, it would be nicer if it were an updatewaiting event.

navigator.serviceWorker.addEventListener('updatewaiting', function(event) {
    alertUser(event.registration);
  });
});

But then, what would I use the event for? It wouldn't be extendable or anything. Might as well just provide a promise and eliminate the event altogether.

navigator.serviceWorker.waiting.then(alertUser)

@jakearchibald
Copy link
Contributor

jakearchibald commented Dec 14, 2017

Since this is related to the registration, this promise should really go on the registration. That way it can be used on all the origin's registrations.

I feel that there may be a more general way to solve this. For example, what if statechange bubbled to the registration? Combined with whatwg/dom#544 you could do:

async function waitingWorker(reg) {
  if (reg.waiting) return;
  return reg.on('statechange').filter(e => e.target.state === 'installed').first();
}

@dfabulich
Copy link
Author

A few points.

Since this is related to the registration, this promise should really go on the registration. That way it can be used on all the origin's registrations.

I worry that this solves non-problems without adequately addressing the real use case: refresh banners.

I claim that the only use case for this is a refresh banner. I'm not aware of any use case where someone would want to track state changes on all of the origin's registrations; the only registration that matters for refresh banners is the registration for the current page's controller.

My goal is to listen for a refresh banner in one easy line. A successful implementation should make us want to delete an entire quiz from the Udacity course on service workers. There's already an "easy" solution in nine lines of boilerplate; anything that makes the use case just slightly more convenient is probably not worth the trouble, IMO.

Under Jake's proposal, the whole code snippet would look like this. (Note that it has to be in ES5, since it's going to run in the client-side page.

navigator.serviceWorker.ready.then(function(reg) {
  if (reg.waiting) return alertUser(reg);
  return reg.on('statechange').filter(function(e) {
    return e.target.state === 'installed';
  }).first().then(function(e) {
    alertUser(reg);
  });
})

IMO, the observable didn't even help; it's actually a bit easier to just use addEventListener.

navigator.serviceWorker.ready.then(function(reg) {
  if (reg.waiting) return alertUser(reg);
  return reg.addEventListener('statechange', function(e) {
    if (e.target.state === 'installed') alertUser(reg);
  });
})

This is an improvement over the status quo, but the cow path is only half paved. It has to repeat alertUser(reg), which means that alertUser has to be a separate, named function. A .waiting promise would be simpler and DRYer.

navigator.serviceWorker.waiting.then(alertUser);

Being DRY allows users to define the alertUser function inline:

navigator.serviceWorker.waiting.then(function(reg) {
  var button = document.createElement('button');
  button.textContent = 'This site has updated. Please click here to see changes.');
  button.style.position = 'fixed';
  button.style.bottom = '24px';
  button.style.left = '24px';
 
  button.addEventListener('click', function() {
    if (reg.waiting) {
      button.disabled = true;
      reg.waiting.postMessage('skipWaiting');
    }
  });
  document.body.appendChild(button);
});

@jakearchibald
Copy link
Contributor

jakearchibald commented Dec 14, 2017

@dfabulich

I claim that the only use case for this is a refresh banner. A successful implementation should make us want to delete an entire quiz from the Udacity course on service workers.

It could also be said that creating justDoEverythingInTheUdacityCourseForMe() would remove the need for the whole course 😄 .

We have to be very careful with single use-case APIs. Big assumptions about what everyone wants, and single use-case high-level APIs can often end up being wasted effort. This is what happened with appcache.

I realise that a single function to do the thing you want would help you, but I'd rather dig into it, and find something that will help with lots of cases.

@dfabulich
Copy link
Author

dfabulich commented Dec 14, 2017

When you say "dig into it," I guess you mean you'd want to investigate how the relatively low-level ServiceWorkerRegistration.onupdatefound and ServiceWorker.onstatechange events are used today? If nobody's using them for anything except refresh banners, that would be the right kind of evidence that a single use-case API is the right thing in this case, right?

I'd be happy to help with such an investigation. I know of no way to access data like that, but I think I've seen Googlers exhibit data like that in the past...?

@jakearchibald
Copy link
Contributor

If nobody's using them for anything except refresh banners, that would be the right kind of evidence that a single use-case API is the right thing in this case, right?

That would be excellent data, but I already know folks also track workers that enter the "redundant" state for error tracking.

@dfabulich
Copy link
Author

Seems like you've got all the cards here… are there other use cases for updatefound/statechange you'd like to share? 😀

@dfabulich
Copy link
Author

Actually, let me pose a more specific question.

There are only five states: installing, installed (waiting), activating, activated, and redundant.

installing can be tracked with ServiceWorkerRegistration.onupdatefound; activated can be traced with navigator.serviceWorker.oncontrollerchange. That leaves three states that are annoying to listen for: installed, activating, and redundant.

This issue is about tracking the installed state. You just mentioned that some people care about the redundant state, which comes as a surprise to me.

Are you aware of anybody who pays attention to the activating state for anything? (I guess someone might, uh, log it for performance tracking? I really can't think of any user-facing action to take.)

@gauntface
Copy link

Minor note: I think oncontrollerchange is actually for activating and not activated. I added a 10 second timeout to the activate callback in the service worker and the controllerchange event triggered as the activate event was dispatched.

@wanderview
Copy link
Member

wanderview commented Dec 14, 2017

Correct. Controller is set when activate event is dispatched. FetchEvents, etc won't be dispatched until the activate event waitUntil is resolved, though.

@dfabulich
Copy link
Author

This API is full of surprises! I think I got confused because the the new service worker is the reg.active service worker; the name "active" made me wrongly think that it had already activated, but, sure enough, MDN clearly says that the active property refers to the "activating or activated" SW.

So, uh, installing can be tracked with ServiceWorkerRegistration.onupdatefound; activating can be tracked with navigator.serviceWorker.oncontrollerchange, and it is therefore annoying to listen to installed, activated, and redundant. Is that right?

Am I now right in understanding that the only known reasons to listen for the activated and redundant states are for logging purposes? (Either error logging or performance logging?)

@gauntface
Copy link

That's my understanding.

Waiting for activated would actually be better before refreshing the page - otherwise, the page refreshes the browser just acts as if it's loading the page while it's waiting for the service worker to activate.

Other use case I'll throw in the ring is testing. May be useful to know what a service worker is in any of these use cases.

Redundant may also be useful in certain scenarios such as libraries managing service workers on behalf of the user (i.e. push SDKs)

@jeffposnick
Copy link
Contributor

Here'a a late suggestion, given that this looks to be on the agenda for tomorrow's F2F: would exposing the skip waiting flag as a readonly attribute on the Web IDL for the ServiceWorker interface solve some of the use cases?

Code that I've used in the past to in the window context to detect service worker state changes would benefit from knowing whether or not the skip waiting flag is set, as the message you'd end up presenting to users ("close all tabs to update" vs. "reload to update") may end up being different for each of those scenarios.

This doesn't provide what initial requestor was asking for, in that you still need to listen for event changes. But it at least would expose one extra bit of useful information that would open the door to using generic window client code without hardcoded messages tailored to whether or not skipWaiting() was called during the updated service worker's installation.

@dfabulich
Copy link
Author

I don't see that exposing the skip-waiting flag solves any problem at all. Either the SW skips waiting on install, in which case you probably don't want any window code at all (the user can just click the browser's Refresh button to refresh), or the SW doesn't, in which case the window just has to await controllerchange.

To be clear, the problem as I see it is that refreshing the page on upgrade requires a lot of confusing boilerplate, even for users of workbox.

Workbox can automate adding arbitrarily complicated code to the SW, but it can't automagically drop code in the window. So instead they have an "advanced recipe" in the documentation ("Offer a page reload to users") explaining how to allow users to refresh the page when the SW updates. https://developers.google.com/web/tools/workbox/guides/advanced-recipes

The "advanced recipe" links to a blog article that I wrote on the topic. https://redfin.engineering/how-to-fix-the-refresh-button-when-using-service-workers-a8e27af6df68 The article is 2,000 words long! Understanding this code requires understanding the registration API, the difference between the statechange event, the updatefound event, and the controllerchange event, and when to use which. That's why it's a whole lesson plan + quiz in the Udacity course.

Maybe this would be OK if refreshing the page in response to SW updates was an advanced use case that most sites don't need, but IMO refreshing on upgrade is a perfectly ordinary use case that basically everybody needs.

The general feedback I see in this thread is "ah, but refreshing is just one use case." I've said that this seems off-base, because what I'm proposing is sugar, and there are no other important use cases to desugar.

If it helps this proposal move forward, I think @gauntface's updatestatechange is at least better than nothing, even if it isn't as helpful as a simple waiting promise.

@philipwalton
Copy link
Member

Workbox can automate adding arbitrarily complicated code to the SW, but it can't automagically drop code in the window.

@dfabulich we're currently working on a workbox-window library, and one of its goals is to ease this pain with some helper methods as well as explanatory logging in dev mode.

A skip waiting flag would at least let us know when not to show such a message if we knew skip waiting had already been applied after the install phase. It's not a complete solution, but it does help with part of this.

@dfabulich
Copy link
Author

workbox-window should be a prollyfill!

@dfabulich
Copy link
Author

In a comment on #1247 that I think was directed at this proposal, @jakearchibald wrote:

A promise doesn't really work in this case, as many service workers can pass through the waiting phase during the life of a page.

If a promise is entirely off the table, then that entails that the best hope for an improvement here is for statechange events to bubble up to the registration; that's issue #1247.

navigator.serviceWorker.ready.then(function(reg) {
  if (reg.waiting) return alertUser(reg);
  return reg.addEventListener('statechange', function(e) {
    if (e.target.state === 'installed') alertUser(reg);
  });
});

As I said above in this thread, this isn't what I'd prefer, but at this point, I'm going to give up on my favorite thing in favor of something (anything) getting done here.

@philipwalton
Copy link
Member

@dfabulich in case you're interested, we've included this feature in the workbox-window library, which should be released very soon (it's currently version 4.0.0-rc.1).

With workbox-window you can listen for a waiting service worker as follows:

import {Workbox} from 'workbox-window';

const wb = new Workbox('/sw.js');

wb.addEventListener('waiting', () => {
  // Do something when the SW is stuck in the waiting phase.
});

wb.register();

While obviously this is not as good as a native solution, hopefully it'll make it easier for developers to handle these important cases.

@dfabulich
Copy link
Author

Does that event fire immediately if a service worker is already waiting?

That's the main confusing thing about using an event listener for this; event listeners prepare for future events, but require an alternate approach for when the event already happened before the listener attached.

@philipwalton
Copy link
Member

It fires if a SW was waiting at registration time, or if an update was found and installed but then that SW didn't move to the activating state.

event listeners prepare for future events, but require an alternate approach for when the event already happened before the listener attached.

Yeah, it's possible to miss it. If you await registration, you'd missed the case you just described. We may consider dispatching our events in the next microtask to help avoid this.

@daffinm
Copy link

daffinm commented Apr 2, 2020

Another year has turned...

Having just spent too many hours (far too many hours) trying to get a simple update workflow working flawlessly (and I am still not there), @dfabulich's one-liner looks like heaven to me.

navigator.serviceWorker.waiting.then(alertUser);

Why is something that seems so simple so maddeningly complex? If there is ever an update I would like to know so I can tell the user. It might be because they pressed a 'check for updates' button. Or it might be because they launched the app and an update was found. Or some periodic background check might find an update. It doesn't matter. In all these cases I need to say: "An update has been found - would you like to use it now?" If they say yes then I need to skip waiting and reload the page when the new service worker is fully activated and in control.

This has to be the most common use case of all and yet it really seems almost impossible to implement (at the moment...)

@willcalderbank
Copy link

3 years later and this is still not there? @daffinm Is absolutely right, surly this is one of the most common use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants