Updated offer page reload recipe seems to contain broken code

[daffinm]

Library Affected:
https://developers.google.com/web/tools/workbox/guides/advanced-recipes#offer_a_page_reload_for_users

Browser & Platform:
N/A

Issue or Feature Request Description:
Updated recipe for offering a page reload to users when an update is found seems to contain broken code. The working code is included here with FIXME comments. Please verify.

if ('serviceWorker' in navigator) {
    const wb = new Workbox('/sw.js');

    const showSkipWaitingPrompt = (event) => {
        // `event.wasWaitingBeforeRegister` will be false if this is
        // the first time the updated service worker is waiting.
        // When `event.wasWaitingBeforeRegister` is true, a previously
        // updated service worker is still waiting.
        // You may want to customize the UI prompt accordingly.

        // Assumes your app has some sort of prompt UI element
        // that a user can either accept or reject.
        const prompt = createUIPrompt({
            onAccept: async () => {
                // Assuming the user accepted the update, set up a listener
                // that will reload the page as soon as the previously waiting
                // service worker has taken control.
                wb.addEventListener('controlling', (event) => {
                    window.location.reload();
                });

                // Send a message to the waiting service worker instructing
                // it to skip waiting, which will trigger the `controlling`
                // event listener above.
                // Note: for this to work, you have to add a message
                // listener in your service worker. See below.
                // FIXME: there is no sw field on the originalEvent.
                // messageSW(event.originalEvent.sw, {type: 'SKIP_WAITING'});
                messageSW(event.originalEvent.currentTarget, {type: 'SKIP_WAITING'});
            },

            onReject: () => {
                prompt.dismiss();
            }
        });// FIXME: method call was not closed out.
    };// FIXME: this bracket was missing so scope of showSkipWaitingPrompt included the following lines.

    // Add an event listener to detect when the registered
    // service worker has installed but is waiting to activate.
    wb.addEventListener('waiting', showSkipWaitingPrompt);
    wb.addEventListener('externalwaiting', showSkipWaitingPrompt);

    wb.register();
}

I think it would also be handy if this recipe showed how to trigger update discovery (unless you do this somewhere else, in which case please link to that from here).

Here's my code for doing this, which I call when the user clicks a button, and from an update poller that fires once an hour:

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.')
        }
    });
}

Note: in order to tell if the call to wb.update() resulted in anything (so I can tell the user something) I have to look at a hidden field called p -- the associated registration object. Not a great solution (I suspect it may break at any time). Better would be a result of some kind (boolean would do) but the wb.update() promise resolves to undefined at the moment.

[daffinm]

Please also note that I am seeing other problems with the code in this recipe. Just got the following error: TypeError: Cannot read property 'currentTarget' of undefined. The event has no originalEvent field:

What I need at this point is a reference to the new service worker so I can tell it to skip waiting. The simplest thing seems to be to do as follows, but this means bypassing the public API again, which is not great.

let reg = event.target.p;
let serviceWorker = (reg.installing || reg.waiting);
messageSW(serviceWorker, {type: 'SKIP_WAITING'});

[darktears]

I can confirm I have the issue here as well. Sometimes originalEvent doesn't exist. Also Safari does behave slightly differently than Chrome.

[jeffposnick]

#2489 should be what we eventually switch it to use, but that's going to land in v6.

In the meantime, apologies that this is still an issue. Here's my attempt at provided something fool-proof:

https://github.com/google/WebFundamentals/compare/wb-skipWaiting-update-again?expand=1

(You basically save a reference to the ServiceWorkerRegistration from the initial call to wb.register(), and then use that registration.waiting as the target to postMessage() to. That avoids having to use any private symbols.)

Can you give that a try and if you confirm it behaves as you expect, I'll merge it into the docs?

[darktears]

@jeffposnick your suggestion worked fine for me.