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

Registration API doesn't expose primitives #534

Closed
sicking opened this issue Oct 24, 2014 · 8 comments
Closed

Registration API doesn't expose primitives #534

sicking opened this issue Oct 24, 2014 · 8 comments

Comments

@sicking
Copy link

sicking commented Oct 24, 2014

As I was writing up the proposals in issue #445 I noticed some mainly unrelated oddities in the current registration API.

Right now there's a few operations that aren't exposed through the API. These operations are instead only available

  1. Waiting for a given registration to become available. The serviceWorker.ready property lets you do this, but only for the SW whose scope covers the current page. If you want to do it for any other SW you have to create an <iframe> to a page within that scope and call serviceWorker.ready there.
  2. You can't force just an update. You can do this by calling register(), but that forces you to pass in the same registration parameters as when the registration was originally. It also creates a new "service worker registration", which I'm not sure if that has the side effect that it causes getRegistration(s) to return a different ServiceWorkerRegistration instance (or does getRegistration(s) always return new instances?)
  3. You can't do an atomic "check if a SW registration exists, if so hand it back, otherwise create a registration". Again, register() does this, but it also forces an update check if the registration already exists.

Per extensible web we should expose these lower level operations rather than just have them be part of larger APIs.

We could solve 1. by firing events on navigator.serviceWorker whenever registrations are added/removed/updated. Or we could add something like registrationReady(url) which is like getRegistration(url) except rather than resolving with undefined if no registration is found, it waits to resolve until such a registration is created.

Adding a Promise<boolean> ServiceWorkerRegistration.update() function would solve 2.

I don't have a great solution for 3. One option would be to have register() not do an update check if a registration exists. That way register() would provide this primitive. Alternatively we could add a getOrRegister() function which is like register(), except it doesn't do the update check.

@sicking sicking mentioned this issue Oct 24, 2014
@jakearchibald
Copy link
Contributor

.register() doesn't force an update check, so 3 isn't an issue.

.getRegistration(s) should be available within the worker scope, self.registration should give you sync access to the current registration from within a worker, then we can remove self.scope because you can get it from the registration.

Agree registration.update() should be a thing that resolves with a new installing worker if one is found, null if no update is found, or rejects on network failure or invalid worker (need to figure the details out here).

registration.ready would solve 1 (you can already achieve this with listeners, but it's a handy shortcut). Is navigator.serviceWorker.ready worth keeping as a shortcut to navigator.serviceWorker.getRegistration().then(r=>r.ready)? Or do we need something that waits for the registration to appear?

@sicking
Copy link
Author

sicking commented Oct 24, 2014

In section "8.1 Register" step 7.5 says "Return the result of running the Update algorithm...". This is done unconditionally, independent of if the scriptURL changed or not. As far as I can tell the Update algorithm will hit the network unless an update happened within the last 24h.

A registration.ready API seems like an interesting idea. But it wouldn't give the ability to wait until a registration appears which navigator.serviceWorker.ready currently provides. I'm not sure how important it is, but if it's important for the current document URL, I would imagine it's important for other registrations as well.

@jakearchibald
Copy link
Contributor

In section "8.1 Register" step 7.5 says "Return the result of running the Update algorithm...". This is done unconditionally, independent of if the scriptURL changed or not.

If the url hasn't changed, you hit 7.2.2.1.2 which returns. It probably should have an "abort these steps" afterwards.

A registration.ready API seems like an interesting idea. But it wouldn't give the ability to wait until a registration appears which navigator.serviceWorker.ready currently provides

Hmm yeah. Will think on this.

@sicking
Copy link
Author

sicking commented Oct 27, 2014

If the url hasn't changed, you hit 7.2.2.1.2 which returns. It probably should have an "abort these steps" afterwards.

Ah. I didn't realize the intent was to abort the steps. I don't think that's always the case when an algorithm returns a promise.

Though what happens if we're in the process of installing a new worker, either due to a previous register() call for the same scriptURL, or due to a normal update? Then it doesn't seem like we'll get all the way to 7.2.2.1.2? So there's still a risk of hitting update.

Seems like some tweaks are needed if the intent is that update should not be performed if the scriptURL is the same as last time register() was called.

@domenic
Copy link
Contributor

domenic commented Oct 27, 2014

Small thing, but we should somewhere agree on whether or not every time an algorithm says "return" it also needs to say "and stop running these steps". From a programming POV requiring the second sentence is silly, but maybe existing specs overrule such concerns.

@sicking
Copy link
Author

sicking commented Oct 27, 2014

Yup, agreed.

@jungkees
Copy link
Collaborator

Seems like some tweaks are needed if the intent is that update should not be performed if the scriptURL is the same as last time register() was called.

Since we don't know whether the new register request's scriptURL is byte-for-byte match to what has been installed, it just runs through to Update algorithm 4.3.10 where it downloads and checks it and resolve with the existing registration if the script has not been updated.

Small thing, but we should somewhere agree on whether or not every time an algorithm says "return" it also needs to say "and stop running these steps".

+1 to not explicitly specify "stop running these steps" for "return". For the majority of the cases "return" would mean stopping the steps from their. It seems it'd be better to articulate that in the opposite case. e.g., the snapshot version of the XHR's send() method stated explicitly, "Return the send() method call, but continue running the steps in this algorithm.", which is a rarer case.

@jakearchibald
Copy link
Contributor

We added reg.update, and .register doesn't trigger a soft update.

reg.ready is still a good idea. Broken out into #770

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

4 participants