-
Notifications
You must be signed in to change notification settings - Fork 828
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
Remove "external" flavor of workbox-window events #2031
Comments
Hmmm, yes, please try to create a repro, as I wouldn't expect this to be the case. We have an |
See also: #2130 |
The problem seems to be, that the behaviour in case of ServiceWorkerRegistration.update() is not deterministic. Sometimes it works as expected (resulting in a 'waiting' event) while in other cases an 'externalwaiting' event is triggered instead. Couldn't reproduce that consistently (so it's pretty hard to test), but it primarily happens, when the page was offline for a while (by devtools or server shutdown) and became online again. |
Browser and platform
I am also being bitten by this issue when using The test steps with my app are always the same and yet sometimes the external versions of the lifecycle events get triggered.
Most of the time the But sometimes, for no apparent reason, the externalwaiting event is triggered instead. When this happens the Happy to provide example code if this will help. |
Further update iteration testing in Chrome 78.0.3904.70 (Official Build) (64-bit). Conclusion: The logic for distinguishing between and internal and external service worker update seems 'temperamental'. Test setup
Test 1 Press the 'Check for Updates' button in B.
Test 2 Press the 'Check for Updates' button in B.
Test 3 Press the 'Check for Updates' button in B.
Two observations
|
@novaknole recently brought this up in #2165 as well. CC: @philipwalton for follow-up. |
In retrospect, I wonder if the Perhaps moving forward, coalescing both into a single (presumably new) event would make sense? The separate events could still be maintained for backwards compatibility. Alternatively, maybe we need to update our recipes to add in listeners for both |
Looks like After some debugging, I have found something as @daffinm found out and more. Code i call when my NOT real navigation happens: Vue.prototype.$checkForUpdate = () => {
if(firstTime) {
firstTime = false;
return;
}
navigator.serviceWorker.ready.then((reg) => {
reg.update();
// onNewServiceWorker(reg);
})
} Assumption After yesterday, I tried to implement workbox-window plugin myself. pure SW implementation. Looks like new service worker would activate immediatelly when calling So, I don't know how exactly in a nutshell Then, I switched back to workbox-window plugin. I also added clientsClaim in SW. and workbox also didn't activate new service worker immediatelly which is good. but now, as @daffinm said, sometimes it calls What doesn't work: A) I opened the first tab. it installed SW. all good. Then I changed service worker code, deployed it. I try to open another tab and go to my website. the first tab is still open. After opening my site in second tab, new service worker gets installed. and it goes to
B) If i only have 1 tab and if i listen the route change in my SPA and call |
@jeffposnick @philipwalton hey guys, when do you think you might give me some heads up or look through what I commented? |
Glad to see that this is getting some attention now.
This seems like the simplest idea. It feels like the waiting/externalwaiting distinction is a bit of an over-refinement.
This is what I tried first -- calling the same method from handlers for both types of waiting event. The problem is that the waiting service worker cannot be contacted from the externalwaiting handler code. The skipwaiting message is received by the older, active service worker. If this was fixed then you could indeed just update your docs, which might be less work. |
Hey all, just catching up here now (sorry, for some reason I wasn't seeing updates to this issue)! From reading all the responses here, it sounds like the main actual pain point is that The thinking there was that, when there are two service workers in play, There is a solution, though. The import {Workbox, messageSW} from 'workbox-window';
async function main() {
const wb = new Workbox('/sw.js');
wb.addEventListener('externalwaiting', (event) => {
// Sends the message to the external service worker.
messageSW(event.sw, {type: 'SKIP_WAITING'});
});
await wb.register();
}
main(); Does this address the issues brought up here? On the question of whether or not we should have I ask because most cases where the The only way But perhaps the vast majority of people using the |
Hi @philipwalton. I don't know what the vast majority are doing. I only know what I am doing. And (as usual) I am doing my best to keep things simple, and so I may have an overly simplistic take on all this. I am not sure I understand the distinction between
When I tested all this originally (see message above for summary) the listeners for the In either case (single or multiple tabs) I would still like to be able to inform the user via a choice dialog of some kind: "An update is available for this app." [Use it now?] [Later...] If 'use it now' is clicked then a I don't see why it matters if the service worker update occurred because of an action occurring in another tab, or after a long delay in one tab, or because of a periodic check for an update issued by the browser in the background. As for your solution using existing code, I will test it and get back to you here. Thanks for explaining it. |
Hello all, I will join this conversation that I am happy to come across now as I actually have been struggling with this issue since I started implementing registration.update() a couple of hours ago. My goal is to also display our 'update app' button (which when clicked sends the SKIP_WAITING message to the sw using @philipwalton your solution above using As for when and how |
Your understanding is correct about the general service worker update cycle. With regards to the difference between An "external" version of each lifecycle event will be triggered if any of the following are true:
That last point is a bit of a hack, but currently the service worker API does not provide any signal to the developer as to whether or not an invocation of Does that help explain why you're sometimes seeing
In your specific case, I don't think it does matter. If all you're planning to do is prompt the user to refresh any time there's an update, then there's no meaningful difference between the events. The difference is intended for cases where a developer only wants to prompt a user for an update in the case of an "external" (a.k.a. unexpected) service worker being installed. That being said, I discussed with @jeffposnick last week and we decided that in v6 we'll likely remove the "external" versions of each event and instead put an What do you think? |
@goyavo
I'd have to see a repo to know for sure, but in my experience, the most common reason a service worker would fail to activate (and thus take control) is that the currently active service worker has an extend lifetime promise that hasn't yet resolved. For example, if you have the following fetch event listener in your active service worker, then calling addEventListener('fetch', (event) => {
// Respond by fetching the request.
event.respondWith(fetch(event.request));
// Call `waitUntil()` with a promise that will never resolve.
event.waitUntil(new Promise(() => {});
}); Could that be the case in your code? |
@philipwalton. It sort of explains. (But it also tells me that I have not got my head fully wrapped around this thing yet...) I think that sounds like the ideal solution. What's the ETA for V6? |
it will be same as messageSW(registration.waiting, {type: 'SKIP_WAITING'}) right? |
That's the logic that I'd recommend just giving the v6 prerelease a try. |
@jeffposnick I would like to give a try for v6 pre-release, but afraid to spend extra time on figuring out what its not working - is it SW in general, our implementation or just library. I already spent time fighting with some issues here and there and everything is kinda working right now (I hope). Is v6 consistent about navigator.serviceWorker.addEventListener('controllerchange' to make sure it works as supposed. task is simple - doesnt matter how many tabs were opened (one tab can be first one that installed SW and claimed control), any update to SW should inform user about updates and on confirmation all tabs should be reloaded. In our case it's dead simple logic - SW is updated each app rebuild due to precache, so we should not have SW older/newer than app. And with |
Library Affected:
workbox-window
Browser & Platform:
Chrome 72
Issue or Feature Request Description:
If the service worker update is triggered from within the app using ServiceWorkerRegistration.update() instead of automatically at page load then the
workbox-window
events don't work as they normally do, breaking the update mechanism which otherwise works as expected when used with normal app re-launch.This would be useful for long-running devices where the trigger for a version check can be pushed out (e.g. by the device subscribing to firebase, having a notification sent etc...)
Will try to create a simplified reproduction.
The text was updated successfully, but these errors were encountered: