GoogleChrome / workbox

📦 Workbox: JavaScript libraries for Progressive Web Apps
https://developers.google.com/web/tools/workbox/
MIT License
12.36k stars 818 forks source link

workbox-window.update() should resolve to a result that tells you if an update was found or not #2431

Closed daffinm closed 6 months ago

daffinm commented 4 years ago

Library Affected: workbox-window 5.1.2

Browser & Platform: N/A

Issue or Feature Request Description: As reported at the end of #2430 (and previously at the end of #2130), there is currently no way of telling if a call to wb.update() resulted in anything. This is not great from a UX point of view if the call to wb.update() happens when the user performs some deliberate act intended to check for updates.

At the moment you have to do this:

function checkForUpdates(wb) {
    wb.update().then(function () {
        let reg = wb.p;
        if (reg.installing || reg.waiting) {
            sendMessageToUser('Update found!');
        }
        else {
            sendMessageToUser('You are already on the latest version of this app.')
        }
    });
}

But this is fragile. The p field is not part of the public interface. It might be better if the promise resolved to a boolean result:

function checkForUpdates(wb) {
    wb.update().then(function (updateFound) {
        if (updateFound) {
            sendMessageToUser('Update found!');
        }
        else {
            sendMessageToUser('You are already on the latest version of this app.')
        }
    });
}

Further tests reveal that, if a user cancels an update (see recipe referred to in #2430), listeners for waiting and externalwaiting (added using wb.addEventListener()) no longer fire if the 'check for updates' button is pressed again with a waiting service worker already in the background. To handle this you really need a result to be returned from wb.update() based on the state of the registration.

jeffposnick commented 4 years ago

CC: @philipwalton, who I believe had some breaking changes intended for v6 intended to smooth over this flow.

As of right now, following #2130, workbox-window's update() is basically equivalent to calling update() on a ServiceWorkerRegistration object.

fridaystreet commented 4 years ago

Would be nice to have it available in workbox, but if you want something a bit more robust in meantime, you can subscibe to the updatefound event.

We put it inside a useEffect hook in react to periodically check for updates every 10 minutes. This causes a MUI alert bar to be displayed at the top of the page notifying the user there is an update with a 'refresh now' button. works pretty well

  useEffect(() => {

    if (!'serviceWorker' in navigator) return;
    let timeout;
    let serviceWorker;

  //adding listen to newWork activated state, prevents setUpdateAvailable being called when user clicks refresh
   const processUpdate = event => {
      const newWorker = serviceWorker.installing;
      if (newWorker) {
        newWorker.addEventListener('statechange', () => {
          if (newWorker.state === 'activated') {
            setUpdateAvailable(true);
          }
        });
      }
      return;        
    };

    const checkForUpdates = (serviceWorker) => {
      if (!serviceWorker) return;
      serviceWorker.update();
      timeout = setTimeout(() => checkForUpdates(serviceWorker), 600000);
    }

    navigator.serviceWorker.getRegistration().then(function(sw) {
      serviceWorker = sw;
      serviceWorker.addEventListener('updatefound', processUpdate);
      timeout = setTimeout(() => checkForUpdates(serviceWorker), 0);
    })

    return () => {
      clearTimeout(timeout);
      if (serviceWorker) serviceWorker.removeEventListener('updatefound', processUpdate);
    }
  }, []);
TheParad0X commented 3 years ago

@fridaystreet how do you prevent the updatefound event from triggering when the service worker is installed for the first time ?

fridaystreet commented 3 years ago

@TheParad0X never noticed any issue on initial install. Are you sure it's called on initial install? I wouldn't have thought it would be triggered until after the first sw is installed as there would be nothing for it to detect an update against?

I think from memory it's doing a chksum check against sw, so if it runs. update() against the same sw that called it, then it's just ignored (ie updatefound isn't triggered)

I could be wrong, but that was my understanding of how it works.

jeffposnick commented 3 years ago

It's true that as per the Service Worker specification, updatefound is fired whenever the service worker registration's installing service worker changes. That includes the very first installation, in which the installing worker would change from null to the new service worker.

There are a few things you could check for if you need to distinguish a "true" update from an initial installation. I think the easiest is probably just checking navigator.serviceWorker.controller. If it's null, then the update is (most likely) due to an initial installation. If it's not null, then the update is definitely due to "true" update.

TheParad0X commented 3 years ago

@fridaystreet @jeffposnick Thanks for your replies. I am struggling to create an update prompt when a new Version is available. The "official way" (https://developers.google.com/web/tools/workbox/guides/advanced-recipes#offer_a_page_reload_for_users) states that I have to add an EventListener for the SkipWaiting message in my Service Worker and then show a yes/no prompt as soon as there is a new SW in the "waiting" state. Clicking on Yes would send the "skip waiting" message to the SW so it can update, and then I would reload the page.

The problem is that as soon as I call ServiceWorkerRegistration. update(), the new SW is installed and activated, without waiting. How can I check for updates without calling ServiceWorkerRegistration. update()? In other words, how do I get the new SW to be installed and waiting ? I wanted to check every 15 min or so... I need to cover the (very probable) scenario where a user has left my PWA running on his tablet and never reloads it.

PS: I gave up on Angular's SW: https://stackoverflow.com/questions/65790749/angulars-service-workers-checkforupdate-does-not-resolve-what-to-do. I found the API to be not very user friendly, I opened an issue and apparently I am not the only one: https://github.com/angular/angular/issues/40467

Then I came across Workbox, the API seems to be much more flexible, although as you can see I am still struggling 😜

jeffposnick commented 3 years ago

It sounds like you're unconditionally calling skipWaiting() inside of your service worker. You need to follow the second part of that recipe in the documentation, "Code in your service worker", that gives an example of conditionally calling skipWaiting() in response to a message posted from your web app.

TheParad0X commented 3 years ago

All right, you were right. However, the "controlling" state does not consistently fire. I figured tried adding clientsClaim right after skipWaiting, but to no avail (well it fired once, but in the following 3 updates it did not fire).

This is my SW template (typescript)

declare const self: ServiceWorkerGlobalScope;
const assetsToCache = self.__WB_MANIFEST;

addEventListener('message', (event) => {
  if (event.data && event.data.type === 'SKIP_WAITING') {
    skipWaiting();
    clientsClaim();
  }
});

cleanupOutdatedCaches();
precacheAndRoute(assetsToCache);
// This assumes /index.html has been precached.
const handler = createHandlerBoundToURL('/index.html');
const navigationRoute = new NavigationRoute(handler);
registerRoute(navigationRoute);

Am I missing something here ? I need a reliable hook based on which I will call window.reload().

TheParad0X commented 3 years ago

I described the problem here: https://stackoverflow.com/questions/66030244/service-worker-sometimes-installes-without-waiting Any ideas?

Edit: Problem solved.

tomayac commented 6 months ago

Hi there,

Workbox is moving to a new engineering team within Google. As part of this move, we're declaring a partial bug bankruptcy to allow the new team to start fresh. We realize this isn't optimal, but realistically, this is the only way we see it working. For transparency, here're the criteria we applied:

Thanks, and we hope for your understanding! The Workbox team