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

Add a script version property to the ServiceWorker object #1387

Open
philipwalton opened this issue Jan 31, 2019 · 13 comments
Open

Add a script version property to the ServiceWorker object #1387

philipwalton opened this issue Jan 31, 2019 · 13 comments

Comments

@philipwalton
Copy link
Member

When a page with a controlling service worker calls navigator.serviceWorker.register(), there's no easy way for that page to know whether or not the script being registered will trigger an update (assuming the developer is following best practices and not changing the script URL).

This is problematic if the page wants to communicate with the service worker via postMessage because whether or not the service worker will be able to respond to those messages depends on what version of the service worker is running. But from the page's perspective, there's no easy way to determine that.

Proposal:

A relatively simple solution to this would be to add an optional scriptVersion property to the options object you can pass to register(scriptURL, options):

navigator.serviceWorker.register('/sw.js', {
  scriptVersion: '1.0.0',
});

This scriptVersion would then be exposed on the ServiceWorker object, so you could reason about whether it's safe to communicate with the service worker from the page.

if (navigator.serviceWorker.controller &&
    navigator.serviceWorker.controller.scriptVersion === '1.0.0') {
  // Do something now that you know it's safe.
}

You could also use the version info to determine whether or not an update is "significant" enough to display a notification to the user (using whatever versioning convention you use):

const controller = navigator.serviceWorker.controller;
const registration = await navigator.serviceWorker.getRegistration();
const getMajorVersion = (v) => v.split('.')[0];

registration.addEventListener('updatefound', () => {
  if (getMajorVersion(registration.installing) > getMajorVersion(controller) {
    // Do something because now we know this is a "major" update.
  }
});

And this would be completely optional. If the user doesn't set a version, then it's simply not present on the ServiceWorker instance (or it's present but null).

If the user set a version that's already been registered, it would be an error. And if the user called register and passed a new scriptVersion but the script didn't update, it would also be an error (or perhaps an no-op).

An alternative to a user-submitted version would be a version automatically generated by the UA, but I'd argue that's less useful since then you'd have to store that version in localStorage or IDB in order to use it across page loads. If you wanted to have something unique to the instance, you could have a separate id value that's auto-generated or an initial registration timeStamp property that would also be unique.

@asutherland
Copy link

It sounds like this ends up being a variation of #1331. In particular, if a string length limit/grammar isn't imposed on scriptVersion, it seems likely it would end up being used for arbitrary JSON storage, etc.

As I understand your link's suggested best practices, the idea is that by changing the underlying script the automatic update checks will upgrade your script for you when the byte-comparison check indicates the script/its dependencies have changed. It doesn't seem like your proposal addresses how the scriptVersion would be updated in this case where the triggering event was not a call to register.

@wanderview
Copy link
Member

It seems tracking and exposing worker install time (#842) might be another way to satisfy this use case. In theory you should know when you published the script with the postMessage handler, so you could treat any earlier installation as not having the handler?

@philipwalton
Copy link
Member Author

It sounds like this ends up being a variation of #1331. In particular, if a string length limit/grammar isn't imposed on scriptVersion, it seems likely it would end up being used for arbitrary JSON storage, etc.

@asutherland, thanks for pointing me to that issue. I do think a more generic store would help solve my use cases, but actually your comment about URLSearchParams made me realize that I could also potentially solve it by recommending our users add a version to the script URL they register (for context, this is regarding a new feature we're adding to Workbox).

It seems tracking and exposing worker install time (#842) might be another way to satisfy this use case.

@wanderview yes, I do think an install/registration time property would also be helpful in solving this use case, but I think passing version data in the script URL is probably more helpful.

There's also still the issue I reported in #1379 (when a page calls .register() and then it receives an updatefound event, there's no good way for the page to know whether its call to register triggered that update); though, if you put script version data in the script URL you register, then not knowing that for sure is much less of a problem.

@philipwalton
Copy link
Member Author

So after playing around with this a bit, I'm realizing my original proposal is not going to work. At least not as a way to set a property on a ServiceWorker object from the window.

The problem comes if the page is serving content cache-first, so it's running an outdated version of the JavaScript, but a newer version of the service worker is available. Here's how that could play out (note: this example puts the script version in the URL, but the same problem would exist if it were set via a scriptVersion configuration option):

  • A navigation request returns cached HTML/JS, which registers sw.js?v=1
  • When the browser requests sw.js?v=1, it finds an updated version (v2) but its scriptURL property on the registration will still say v1
  • The updatefound event listener prompts the user to reload to get the latest version of the page.
  • When the user reloads the new HTML/JS will register sw.js?v=2, which hasn't changed on the server since it was just downloaded, but because the URLs are different, it'll trigger a new install.

I think this means if we add any kind of versioning to the ServiceWorker object, it'll have to be set either from within the service worker file itself, or automatically assigned by the UA. It can't be set from the window.

@asakusuma
Copy link

@philipwalton FWIW, LinkedIn does the former. We embed a version in the service worker file itself. The page can ask the service worker for its version via postMessage, and the service worker will respond with a generic error if it receives a postMessage that it does not recognize.

@philipwalton
Copy link
Member Author

@asakusuma, yeah that's exactly what we were originally doing in Workbox, but it's really tricky from a library perspective because if the developer doesn't also implement the message listener properly in the service worker, it will break things.

We were setting a timeout on the postMessage() call to hopefully minimize they breakage, but ultimately I decided to remove the entire versioning system altogether and let advanced users who need that functionality implement it themselves.

I still do think it's a really important feature though, and as service worker usage gets more complex in the future, I think we'll need a platform solution.

@asakusuma
Copy link

if the developer doesn't also implement the message listener properly in the service worker, it will break things.

@philipwalton Isn't that problem orthogonal to whether or not the version is implemented by the platform or injected in the script by the developer?

@philipwalton
Copy link
Member Author

@asakusuma not if there's a platform API for scriptVersion that's exposed to both the window and the service worker.

If that were the case, both would get access to the version at the same time, even in cases where there's a scriptVersion mismatch.

@madmoizo
Copy link

madmoizo commented Feb 5, 2019

@philipwalton if you don't want to use postMessage, your service worker lib can use indexeddb to store the installed/installing version and your window lib can query these versions on service worker update and let the user do whatever he wants with it

@asakusuma
Copy link

@philipwalton so you want a platform version available everywhere so that backwards incompatible changes can be made in the service worker message handling code? Which would require the window code to send different messages depending on what version of the service worker is running?

@frlinw IndexedDB is not very reliable. We tried that exact setup with storing the version in IndexedDB. See #1331 (comment)

@jakearchibald
Copy link
Contributor

Pre TPAC thoughts:

  • I'd love a way to simplify getting data from a worker & calling methods. Eg, if a worker had export const hello = 'world', I'd love to be able to do this from a page: (await worker.exports).hello.
  • We should talk about the data store thing again, but it still feels like a generic "faster IDB" is the answer.

@asutherland
Copy link

The await worker.exports sounds like it would need to be a middle-ware right now because of the mismatch between the capabilities of what exports can expose and what structured serialization can encode over the wire. That said, it sounds like Typed Objects https://github.com/tschneidereit/proposal-typed-objects/blob/master/explainer.md are going to happen because of WASM needs. I don't think the explainer goes into it, but my understanding was that these typed objects could enable concurrent multi-threaded access as well as be transferrable via structured serialization (and cheaply!). So they could form the basis of something easy to spec if the spec is "only typed objects can be exported by a worker top-level script" and then the fact that all typed objects can be cloned/serialized means there's no problems with things that don't serialize.

@jakearchibald
Copy link
Contributor

I was thinking it would be structured cloned, but if there's something better, great!

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

6 participants