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

No Updates if Page Begins Life Without A Service Worker #256

Closed
daniel-nagy opened this issue Apr 4, 2022 · 11 comments
Closed

No Updates if Page Begins Life Without A Service Worker #256

daniel-nagy opened this issue Apr 4, 2022 · 11 comments

Comments

@daniel-nagy
Copy link

If a page begins life without a service worker then updates to the service worker will not trigger an update notification (will not set needRefresh).

I see offlineReady which looks like it is intended for pages that begin life without a service worker. However, it would be valuable to know that the service worker was updated before the old service worker becomes active.

@userquin
Copy link
Member

userquin commented Apr 4, 2022

@daniel-nagy

offline ready is true when the sw is installed and active when there was not a sw installed, should be only be true once. You can use it to hint the user the app can now work offline.

On the other hand, need refresh will be only true when we have a service worker installed and active and there is a New version available and ready to be installed, only when register type option is prompt, if using autoUpdate on register type option, need refresh will be never true.

@daniel-nagy
Copy link
Author

need refresh will be only true when we have a service worker installed and active and there is a New version available and ready to be installed

It appears the service worker must also be controlling the page for this to be true, so if a page begins life without a service worker you must call clients.claim() in the service worker context. Otherwise if the service worker updates it will not set needRefresh.

This can be problematic for native like experiences were you expect your app to be running for a long time without the app reloading. If the user never restarts the app they won't get an update notification. I don't know that calling clients.claim() is necessarily a solution to this, since you don't necessarily want the service worker to start controlling the page, you just want to know if it has been updated.

@userquin
Copy link
Member

userquin commented Apr 5, 2022

Check periodic sw updates on docs or just force reload the page on native app activation.

Anyway, why are you using a pwa inside a native app? The pwa should replace that native app.

@daniel-nagy
Copy link
Author

Anyway, why are you using a pwa inside a native app?

We have existing native apps and we are considering using Web technologies to add new features. Also, wrapping a PWA in a native shell to get access to platform features and to distribute it in app stores is also common.

@userquin
Copy link
Member

userquin commented Apr 6, 2022

@daniel-nagy ok TWA.

About the problem checking for sw update you should check the docs for the native web view, maybe the feature not yet supported (it works on browsers, maybe a native porting bug). When I have time I'll check it also on android.

@daniel-nagy
Copy link
Author

@userquin Regarding pages that begin life without a service worker, I see the same behavior in both the browser and the Webview. That is, a service worker needs to be controlling the page before needRefresh is true on SW updates.

This is potentially something I can deal with on my end. I might be nice if this plugin had an API for this case though.

@userquin
Copy link
Member

userquin commented Apr 6, 2022

@daniel-nagy this plugin uses workbox-window on the client to handle sw events, as I mention it should be a problem with the interface between the native browser impl and the webview (for example, on android we have WebView and ChromeWebView with Chrome Embedded Framework (via JNI) on the bridge between native and android web view).

If the sw is not controlling the page, then needRefresh should never be true, in that case offlineReady will be true once the sw is controlling the page.

EDIT: maybe there is a race condition when installing first time and checking for update while installing, try increasing the timeout interval for example to 1 minute.

@daniel-nagy
Copy link
Author

daniel-nagy commented Apr 10, 2022

Alright, I've had more time to dig into this.

If a service worker is activated, and there is not currently a service worker controlling the page, then Workbox will emit the activated event but event.isUpdate will be falsy. This will end up calling onOfflineReady. Effectively this makes onOfflineReady an alias for activated.

So I can essentially use the onOfflineReady callback as a SW activated callback until a service worker is controlling the page. Using this knowledge I can call setNeedRefresh(true) in onOfflineReady if offlineReady is already true. E.g.

const context = useRef({ offlineReady: false });

const {
  needRefresh: [needRefresh, setNeedRefresh],
  updateServiceWorker
} = useRegisterSW({
  onOfflineReady: () => {
    context.current.offlineReady && setNeedRefresh(true);
    context.current.offlineReady = true;
  },
});

There is an issue with the useRegisterSW hook capturing scope and not reacting to option changes which is why I need to use useRef to box offlineReady so I can properly reference it in my callback.

With this I am now able to show an update notification for subsequent SW updates before a single service worker is controlling the page. However, there is another issue. When I call updateServiceWorker nothing will happen because the service worker is already active (not waiting). So I need to reload the page myself in this case. E.g.

needRefresh && (
  <UpdateNotification
    dismiss={() => setNeedRefresh(false)}
    update={() =>
      ctx.current.offlineReady
        ? location.reload()
        : updateServiceWorker(true)
    }
  />
)

For me I think having access to the SW lifecycle would be more useful. I would suggest adding callbacks for installed and activated to allow developers to implement their own behavior if they want to. E.g.

useRegisterSW({
  onInstalled(event: { skipWaiting: () => void }) {
    // A service worker was installed and is waiting to be activated, call `skipWaiting` to activate.
  },
  onActivated(event: { takeControl: () => void }) {
    // A service worker was activated but is not yet controlling the page, call `takeControl` to start controlling the page.
  }
});

@userquin
Copy link
Member

For me I think having access to the SW lifecycle would be more useful. I would suggest adding callbacks for installed and activated to allow developers to implement their own behavior if they want to

PR welcome

@userquin
Copy link
Member

There is an issue with the useRegisterSW hook capturing scope and not reacting to option changes which is why I need to use useRef to box offlineReady so I can properly reference it in my callback.

About this, file it on a separate issue, thx

@piotr-cz
Copy link
Contributor

BTW - nice explanation why clientsClaim() should be used in this particular example is given here: GoogleChrome/workbox#2070

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

No branches or pull requests

3 participants