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

fix: prompt for update logic #428

Merged
merged 2 commits into from Jan 3, 2023
Merged

Conversation

userquin
Copy link
Member

@userquin userquin commented Dec 17, 2022

With current version multiple clients get auto updated once the service worker receives the first update: with logic included on PR #394

With current changes, it is working properly, also tested on Firefox.

There are a few combinations using isUpdate and isExternal: these 2 flags from workbox-window are defined as boolean | undefined, when installed|activated events fired, undefined values have special meaning.

As an example, with only one client:

  • on first visit we receive isExternal: false with no isUpdate
  • on update, we receive isExternal: false and isUpdate: true and showSkipWaitingPrompt being called from waiting event.

Problems comes when we have a few clients, for example, 2 tabs in first browser instance and another browser instance:

On first visit we receive isExternal: false with no isUpdate only in the first one client visiting the app, other clients will not receive the event

On update it is more complex, since we can have a few combinations, here the tests I've made with previous setup (building between tests):

Test 1:
Reloading app in tab1:

  • tab 1 receives isExternal: false and isUpdate: true
  • tab 2 receives isExternal: false with no isUpdate
  • second instance receives isExternal: false with no isUpdate

Click prompt reload button from tab1, all clients being updated, then build again.

Test 2:
Reloading browser instance 2:

  • tab 1 receives isExternal: false and isUpdate: true
  • tab 2 receives isExternal: false and isUpdate: true
  • second instance receives isExternal: false and isUpdate: true

Click prompt reload button from browser instance 2, all clients being updated, then build again.

Test 3:
Reloading browser instance 2:

  • tab 1 receives isExternal: false with no isUpdate
  • tab 2 receives isExternal: false with no isUpdate
  • second instance receives isExternal: false and isUpdate: true

Click prompt reload button from tab2 in browser instance 1, all clients being updated.

It seems on next update isExternal depends on the client that fires previous update.

@netlify
Copy link

netlify bot commented Dec 17, 2022

Deploy Preview for vite-plugin-pwa-legacy ready!

Name Link
🔨 Latest commit 74bdc8e
🔍 Latest deploy log https://app.netlify.com/sites/vite-plugin-pwa-legacy/deploys/639df4979b4dc2000806f342
😎 Deploy Preview https://deploy-preview-428--vite-plugin-pwa-legacy.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@userquin userquin marked this pull request as ready for review December 17, 2022 16:56
@userquin userquin requested a review from antfu December 17, 2022 16:56
@userquin
Copy link
Member Author

userquin commented Dec 17, 2022

don't merge yet, I'm checking if we can switch event listener from installed to activated.

EDIT: I'll keep installed event (the PR ready)

@crunchwrap89
Copy link

Master @antfu can you please review this PR? <3

@userquin userquin merged commit 78f8672 into main Jan 3, 2023
@userquin userquin deleted the userquin/fix-prompt-for-update branch January 3, 2023 12:42
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

Successfully merging this pull request may close these issues.

None yet

3 participants