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

Check if there is a worker that has been previously installed #8

Closed
wants to merge 1 commit into from

Conversation

imyelo
Copy link
Sponsor

@imyelo imyelo commented Jul 26, 2018

In a recent practice, I found that if developers use a updated hook to guide users to a new version, they tend to fall into a trap.
Let me explain it with the following example steps:

  1. First, the user visits our site for the first time. The service worker caches all the files for the current version A.
  2. After a while, users visited our site for a second time. At this point, the browser directly displays the contents of version A to the user because of the Service worker interception.
  3. At the same time, the browser requests sw.js from our server and is delighted to find that we had released version B, so the browser starts getting all the files for version B in the background with the new sw.js.
  4. When the browser gets all the files for version B, our library register-service-worker emits the updated hook, so we pop up a prompt on the page asking the user if they want to use the new version B. If the user chooses "yes", then we will activate the worker of version B by calling skipWaiting, and then automatically refresh the page with window.location.reload. In most cases, up to this point, everything is going the way we expect it to.
  5. Unfortunately, there are times when users choose not to update immediately, or they simply close our tabs. So this time we assume that the user chose to ignore our prompt.
  6. Next, the user refreshes the page.
  7. At this point, we found that our site no longer asked users if they want to use new version B, even if the worker for version B was in the waiting state.

So, what's the problem?

First, when version B is in the waiting state for the first time, simply refreshing the page does not activate B's worker. The reason for this is explained in this article:

Even if you only have one tab open to the demo, refreshing the page isn't enough to let the new version take over. This is due to how browser navigations work. When you navigate, the current page doesn't go away until the response headers have been received, and even then the current page may stay if the response has a Content-Disposition header. Because of this overlap, the current service worker is always controlling a client during a refresh.

Second, we shouldn't call skipWaiting before the user confirms, which is also explained in the above article:

Caution: skipWaiting() means that your new service worker is likely controlling pages that were loaded with an older version. This means some of your page's fetches will have been handled by your old service worker, but your new service worker will be handling subsequent fetches. If this might break things, don't use skipWaiting().

Thus, we find that in this case, when the new worker has entered the waiting (to be activated) state, we are supposed to go into the process of guiding the user to update. However, we can not achieve it simply by listening to a specified hook.

IMO, is that this case should trigger the updated event, and that's what updated is supposed to do.

In addition, this example shows what developers need to do when updated event does not cover the above case:
imyelo/pokequest-wiki@023bb81

What do you think? @yyx990803

@yyx990803
Copy link
Owner

Will this fire the hook twice if:

  • There's a previous version waiting
  • And there is a new version found

@imyelo
Copy link
Sponsor Author

imyelo commented Jul 26, 2018

Yes, if there is a newer version (e.g. version C).

@imyelo
Copy link
Sponsor Author

imyelo commented Jul 26, 2018

More precisely, this special situation occurs only before the user closes all clients (pages) in the same scope. The new worker will be automatically activated when the page is reopened in a new tab.

@yyx990803
Copy link
Owner

@imyelo in that case I think it's better to ensure the hook is fired at most once for each session.

@imyelo
Copy link
Sponsor Author

imyelo commented Jul 26, 2018

Yeah, I understand your concern. But the above situation is really difficult for the users to find out when they only read the examples in the readme.

Or maybe we can

  • drop the updated event of version C to keep the hook only triggered once?
  • or define a new special hook other than updated for this?

@ulivz
Copy link
Collaborator

ulivz commented Jul 27, 2018

I also encountered similar requirements a week ago and found this solution.

And I still cannot find enough arguments for me to support this workaround.

But there is one thing should be OK, when the synchronous value waiting exists, we shouldn't bind updatefound event.

@yyx990803
Copy link
Owner

@ulivz I just merged #7 - does that somewhat address the need?

@ulivz
Copy link
Collaborator

ulivz commented Aug 8, 2018

@yyx990803

I just tested for that, when I omitted the first updatefound event and didn't skipWaiting the newest SW, when I refreshed the browser, the debugger still kept following status:

image

But onupdatefound wasn't be triggered again. so #7 didn't address our need.

@yyx990803
Copy link
Owner

Closed in favor of #9, thanks for the contribution!

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

Successfully merging this pull request may close these issues.

3 participants