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

PWA regularly check for service worker updates and automaticially reload the page #1188

Merged
merged 16 commits into from
Nov 6, 2023

Conversation

jinzo
Copy link
Contributor

@jinzo jinzo commented Nov 5, 2023

Hi,

I was working on the #1169 issue and when deploying to staging I noticed that my PWA (on phone or open in the browser) does not actually update automatically like I would presume with the registerType: 'autoUpdate' config option set.
After a bit of digging I actually found two things missing:

  1. The ServiceWorker itself did not check for an update when running. So I checked the documentation of VitePWA and adapted the code found there: https://vite-pwa-org.netlify.app/guide/periodic-sw-updates
  2. The actual webpage reload (after the ServiceWorker autoUpdates with step 1.) was not applied, therefore I set the immediate parameter to true as seen in the warning here: https://vite-pwa-org.netlify.app/guide/auto-update.html#automatic-reload

I tested this and now it behaves like I would expect it to behave. The Traccar PWA now checks every hour, if there is a new version and if found updates the ServiceWorker and reloads the webpage automaticially.

On a somewhat related note, just to be sure, so you do prefer automatic reload instead of the update-popup-and-user-clicks-for-reload?

@tananaev
Copy link
Member

tananaev commented Nov 5, 2023

I thought they have a standard way to detect it automatically? And have a prompt option. How is registerType: 'autoUpdate' supposed to work?

@jinzo
Copy link
Contributor Author

jinzo commented Nov 5, 2023

I was under that impression too, but apparently not - it did update itself (ninja edit, here I mean the ServiceWorker itself, not the PWA)/check (with autoUpdate option) on a new page load (either open a new tab or reload the current page). And that's how I understand this option alone works (it silently updates the ServiceWorker, not the webapp) on new page load (as this is actually not guaranteed with ServiceWorkers, there could be old ServiceWorkers open and somewhere in another tab and they would not get updated if a new tab loads a new version of a ServiceWorker). More info on updating ServiceWorkers: https://web.dev/articles/service-worker-lifecycle#updates
Yes there is a prompt option, if you change registerType to 'prompt' instead of 'autoUpdate' (or omit it, 'prompt' is default) then it won't autoUpdate the ServiceWorker, but it will do it once the user clicks on the button (so it will update the ServiceWorker AND the pwa/web).
But you still have to check every X ms, if you don't want to wait for (re)load of the webpage (which I'm not even sure how the customer can do, if it 'installs' the PWA on their phone for example).
I personally would prefer the prompt option, so the PWA would just check every X ms if there is an update, and use the VitePWA prompt to let the user update if there is one.

EDIT: Sorry I really need to double check my comments before I send them, I was not very clear.

So, 'autoUpdate' option mostly tells the VitePWA to update the ServiceWorker (which is shared between all tabs from the same address) when it detects that there is a new version (as this ServiceWorker is shared between multiple tabs, auto updating is not automatic in the sense of how we always get the latest version of the webpage itself - bar caching). Nothing much about updating the Webpage/PWA. I guess that could be handled smartly too, but a reload is the most foolproof method there is - so we need to turn that on with the registerSW 'immediate' option, which reloads the webpage/PWA immediately when the ServiceWorker gets updated (automaticially with 'autoUpdate').

And if we don't want to wait for a webpage reload (or any of the other update triggers here: https://web.dev/articles/service-worker-lifecycle#updates) we need to check for ServiceWorker updates ourselves - and they have some edge case handling in the provided sample. Not sure why they are not doing that automatically TBH. I was expecting that too. But apparently not, as it's not even mentioned in the linked VitePWA docs.

@tananaev
Copy link
Member

tananaev commented Nov 5, 2023

It seems like the browser should automatically check for updates:

https://web.dev/articles/service-worker-lifecycle#updates

Maybe I misunderstand something?

@jinzo
Copy link
Contributor Author

jinzo commented Nov 5, 2023

The browser is checking for an update in this cases:

  • A navigation to an in-scope page.
  • A functional events such as push and sync, unless there's been an update check within the previous 24 hours.
  • Calling .register() only if the service worker URL has changed. However, you should avoid changing the worker URL.

The last two are not applicable, as we are not pushing or syncing anything to the ServiceWorker and we are not chaging the ServiceWorker URL.
Which means that only the first check is applicable, that happens when a user opens the webpage (either a new load, or an reload of the existing page). So in Traccars React App this means, that if a user has just one tab with it open, and uses it normally (and doesen't close the tab itself) he won't get the updated serviceworker/code until he opens a new one (or reloads the webpage). As a React App doesen't really navigate to a new page and therefore trigger a update. And in the case of a "Installed" PWA on a phone, this probably means force closing the "app" and opening it again.

@tananaev
Copy link
Member

tananaev commented Nov 5, 2023

Why do we need the immediate: true? Would having prompt, as we discussed, fix it?

@jinzo
Copy link
Contributor Author

jinzo commented Nov 5, 2023

Because of the warning here: https://vite-pwa-org.netlify.app/guide/auto-update.html#automatic-reload
Quote from the yellow WARNING "Automatic reload is not automatic page reload, you will need to use the following code in your application entry point if you want automatic page reload:" My note: Automatic reload == 'autoUpdate'

Yes, having a prompt removes the need for immediate: true

@tananaev
Copy link
Member

tananaev commented Nov 5, 2023

  • OK, let's change to prompt
  • Let's make auto-update configurable
  • Let's keep this copied code in a separate file and provide a link to the original

@jinzo
Copy link
Contributor Author

jinzo commented Nov 5, 2023

Ok, just to be clear, so user configurable as a server setting? And do we make it just on/off or can the update check interval be configured too (eg. then I would still go with one setting, but if it's 0 then don't check, if it's more than 0 then use that as an update check interval)?

@tananaev
Copy link
Member

tananaev commented Nov 5, 2023

We can make the interval configurable, if that's easy to implement. And zero means no updates. I think we should include it in the server attributes.

@tananaev
Copy link
Member

tananaev commented Nov 5, 2023

I think if it works well, let's make it as a default, so some reasonable default value.

@jinzo
Copy link
Contributor Author

jinzo commented Nov 5, 2023

So this is just a work in progress for now - but have to finish for today and comited to get some preliminary feedback.

I moved everything to a React component and adapted it to use plain Snackbar like other similar functionality in Traccar. While tweaking this I tested it plenty, and it works very well. What remains is figuring out why "useAttributePreference" is erroring out on me and do basic cleanup around that. I'm thinking 1h as the default value (as this is also the 'default' in Vite PWA documentation).

P.S: There is a big ol red warning that switching between 'autoUpdate' and 'prompt' while in production can be a pain, but from what I can see 'autoUpdate' did not make it into 5.9 so this won't be a problem: https://vite-pwa-org.netlify.app/guide/auto-update.html

@jinzo
Copy link
Contributor Author

jinzo commented Nov 5, 2023

After stepping back from the monitor for a few minutes it dawned on me, that as the system is not initialized, I cannot access the user attributes. Moved it to a better location and it is ready for a review.

modern/src/ReloadPrompt.tsx Outdated Show resolved Hide resolved
modern/src/ReloadPrompt.tsx Outdated Show resolved Hide resolved
modern/src/ReloadPrompt.tsx Outdated Show resolved Hide resolved
modern/src/ReloadPrompt.tsx Outdated Show resolved Hide resolved
modern/src/ReloadPrompt.tsx Outdated Show resolved Hide resolved
modern/src/ReloadPrompt.tsx Outdated Show resolved Hide resolved
modern/src/resources/l10n/en.json Outdated Show resolved Hide resolved
modern/vite-env.d.ts Outdated Show resolved Hide resolved
Comment on lines 49 to 51
<Button onClick={() => updateServiceWorker(true)}>
{t('settingsReload')}
</Button>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just use an icon button? Like this one:

https://mui.com/material-ui/material-icons/?query=ref&selected=Refresh

It looks better and also we can remove one of the strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, did not even think about this.
image

modern/src/UpdateCheckPrompt.tsx Outdated Show resolved Hide resolved
@tananaev tananaev merged commit 4a6e35c into traccar:master Nov 6, 2023
1 check passed
@tananaev
Copy link
Member

tananaev commented Nov 6, 2023

Merged, thank you.

@jinzo
Copy link
Contributor Author

jinzo commented Nov 6, 2023

Thank you very much for your patience.

@jinzo jinzo deleted the pwa-sw-update-check branch November 6, 2023 15:46
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.

2 participants