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

ready promise is kind of weird after you call unregister() #1279

Open
wanderview opened this issue Feb 9, 2018 · 3 comments
Open

ready promise is kind of weird after you call unregister() #1279

wanderview opened this issue Feb 9, 2018 · 3 comments

Comments

@wanderview
Copy link
Member

Consider this:

// These should both evaluate to the same registration...
await navigator.serviceWorker.register('sw.js', { scope: './' });
let swr = await navigator.serviceWorker.ready;

// unregister and drop our ref
await swr.unregister();
swr = null;

// the registration should not be accessible with getRegistration() any more
swr = await navigator.serviceWorker.getRegistration('./');
assert_equal(swr, null);

// the ready promise continues to resolve the old registration, even
// if there are no controlled clients.
swr = await navigator.serviceWorker.ready;
assert_true(swr);

// I think the workers are all gone, though??
assert_equal(swr.installing, null);
assert_equal(swr.waiting, null);
assert_equal(swr.active, null);

// register a new service worker
let swr2 = await navigator.serviceWorker.register('sw.js', { scope: './' });
let swr3 = await navigator.serviceWorker.ready;

// the ready promise should not change since its settled
assert_equal(swr, swr3);

// but the register() call should result in a new registration since the
// old one was not kept alive by any controlled clients
assert_not_equal(swr, swr2);

Does this make sense to developers? I find it really weird. Related to #1278.

@mfalken
Copy link
Member

mfalken commented Feb 10, 2018

I agree this is weird. I think our hands are tied because .ready is a property, so it always needs to return the same promise. There is some historical discussion at #223 and https://groups.google.com/a/chromium.org/d/msg/blink-dev/jjh4KUS0cS0/xA8_F904UeQJ

I'm not too sure why #223 went with a property instead of a method. #223 says "it's just a state transition". I guess we didn't consider that unregister() can transition you back to the "not ready" state.

Of course this is all a bit of an edge case, I don't think it's caused problems in the wild, and sites can detect it by checking if registration.active is null.

@gauntface
Copy link

Adding two cents: the lifecycle API's of SW are either too high-level or too low-level.

High-level APIs like .ready() fail to encapsulate edge cases like this
Low-level APIs become error prone to use (i.e. listening for state changes on a SW instead of a registration and checking for controller change events).

For the most part - this isn't an issue until you want to take advantage of skipWaiting() and clients.claim().

@mfalken
Copy link
Member

mfalken commented Oct 26, 2018

F2F: see #1278

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