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

Provide callbacks in registerServiceWorker (1.x) #3375

Closed
wants to merge 3 commits into from

Conversation

piotr-cz
Copy link
Contributor

@piotr-cz piotr-cz commented Oct 31, 2017

Adding two callbacks in registerServiceWorker file:

  • onUpdate - executed after console logs New content is available; please refresh.
  • onSuccess - executed after console logs Content is cached for offline use.

This is a more lightweight change, compared to other related PR: #2426 as in my opinion problem should be separated into two parts:

  • Providing callbacks in registerServiceWorker.js file
  • Adding an callback notification (Toast) that may be customized by developer

How callbacks are provided or named is up to discussion, the point is to provide minimal working technique to detect update.

When this or similar PR is landed on master branch we can discuss how to notify user about the update.

To use it inside application to show notification (toast/ icon etc) register service worker inside App.js file:

import registerServiceWorker from './registerServiceWorker.js'

export default class App {
  constructor(props) {
    // Register service worker and add onUpdate callback
    registerServiceWorker({
      onUpdate: this.handleServiceWorkerUpdate
    })
  }

  // Simplest notification implementation example.
  handleServiceWorkerUpdate(registration) {
    if (window.confirm('New update has been installed, click to restart')) {
      window.location.reload()
    }
  }
}

@James2516
Copy link

James2516 commented Jan 11, 2018

Just to add on from #3665
Is it possible to add another callback: onRefreshSuccess: executed after a refresh, so that developers can show a message like

Page was automatically refreshed to display updated content

So that developers can opt for auto refresh instead of a toast asking for manual refresh

# Conflicts:
#	packages/react-scripts/template/src/registerServiceWorker.js
@piotr-cz
Copy link
Contributor Author

@James2516 Refresh/ Restart is triggered either by application or browser restart - not by service worker and to my knowledge so it's not possible to add a callback there (it doesn't trigger any events, see ServiceWorker.state).

You'd have to persist the state of info about new installed version across reloads somehow (for example reload location with added query param or save information in IndexedDB by service worker itself).

It's far more easier to indicate that new version has been installed in the background than that there was an update to application which just has started.

@James2516
Copy link

You'd have to persist the state of info about new installed version across reloads somehow (for example reload location with added query param or save information in IndexedDB by service worker itself).

So the service worker can store the last refresh timestamp in local storage and use that to infer and call the onRefreshSuccess callback accordingly?

Otherwise the application can do that as well by making use of local storage or URL redirects.

@piotr-cz
Copy link
Contributor Author

It doesn't have to be a timestamp, just simple flag saved in IndexedDB should be enough (service worker doesn't have access to local storage).

You could also use notification if your app has permission to show these.

As service worker is generated by sw-precache-webpack-plugin to add listener to installed event check suggestions in this issue: #2253

@gaearon
Copy link
Contributor

gaearon commented Jan 19, 2018

Would you please rebase against the next branch?
I'm not opposed to this.

We just disabled them by default in #3817, but it makes sense to add some conventional helpers for users who want to opt in.

@piotr-cz
Copy link
Contributor Author

I'll prepare PR for next branch but still keep this one hoping that it may appear in v1.x release

@piotr-cz piotr-cz changed the title Provide callbacks in registerServiceWorker file Provide callbacks in registerServiceWorker (1.x) Jan 19, 2018
@gaearon
Copy link
Contributor

gaearon commented Jan 20, 2018

Thanks, but I don't think we'll be doing another 1.x release except for some critical fixes.

@gaearon gaearon closed this Jan 20, 2018
@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants