search

vite-pwa / vite-plugin-pwa

Zero-config PWA for Vite
https://vite-pwa-org.netlify.app/
MIT License
3.23k stars 210 forks source link

Bug: Prompt for update doesn't reload page on first update #789

Open piotr-cz opened 6 days ago

piotr-cz commented 6 days ago

Steps to reproduce in v0.21.0:

  1. git clone https://github.com/vite-pwa/vite-plugin-pwa.git
  2. Run examples as in docs
  3. Answer questions:
    • Select a framework: preact (bug is not related to preact)
    • Select a strategy: injectManifest
    • Select a behavior: Prompt for update
    • Enable periodic SW updates? yes
    • Unregister SW? no
  4. Open https://localhost in Chrome in incognito mode
  5. In prompt App ready to work offline: click Close
  6. Wait for 1 minute as suggested in docs
  7. Add console.log('foobar') toexamples/preact-router/src/main.tsx`
  8. Repeat step 2 and 3.
  9. In prompt New content available, click on reload button to update. click Reload
  10. Problem: Page does not reload

This happens only for first service worker update. For any subsequent updates, clicking on Reload reloads the page. Reproduction should be run in incognito mode to simulate first update

Seems that the controlling event doesn't fire in this case:

https://github.com/vite-pwa/vite-plugin-pwa/blob/95142ebc588dea3d9376a2eb4affe086f19a7395/src/client/build/register.ts#L85-L88

When I add 

// examples/preact-router/src/prompt-sw.ts
import { clientsClaim } from 'workbox-core'

//...

clientsClaim()

Then the controlling event fires, however with incorrect event.isUpdate = false and in effect reload still doesn't happen

BTW: I've noticed that in Workbox Docs > Use cases and recipes > Handling service worker updates with immediacy there is no check for the event.isUpdate

Possibly related: #256

piotr-cz commented 6 days ago

The issue is related to checks for event.isUpdate, which is false for for first service worker update.

The event.isUpdate property doesn't indicate that service worker has been updated, but the fact that it's an update to service worker that was available when workbox-window was registered.

It's set at workbox-window registration, and not updated later on:

// Set this flag to true if any service worker was controlling the page
// at registration time.
this._isUpdate = Boolean(navigator.serviceWorker.controller);

Source: https://github.com/GoogleChrome/workbox/blob/c77dceb54d4af1749db95316710d6430e82b0c48/packages/workbox-window/src/Workbox.ts#L117-L119

For the controlling event it's description is:

True if a service worker was already controlling when this service worker was registered.

It doesn't mean that service worker has been updated from previous version

Reference: https://github.com/GoogleChrome/workbox/blob/c77dceb54d4af1749db95316710d6430e82b0c48/packages/workbox-window/src/Workbox.ts#L698-L699

I think it's safe to remove all if (event.isUpdate) checks.

piotr-cz commented 3 days ago

Related issue in workbox repo: https://github.com/GoogleChrome/workbox/issues/3377