-
Notifications
You must be signed in to change notification settings - Fork 100
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
Discontinue skip waiting by default (and automatically reloading) when service worker updates #205
Comments
From the a support forum topic from @ckchaudhary: I know this issue has been brought up in past but I wasn’t able to find any updates on this, hence I am opening a new ticket. |
Hi. Please reply to my questions above when you can. |
I just wanted to bump this thread as this is a major issue that needs to be addressed. Any subscription site, or site that requires a front-end login will face this problem. It's completely unacceptable to have the page reload every time a user logs in. It's also not practical to expect users to press any kind of update button. In our case we don't display the admin bar at all. This issue is serious enough that we had to completely disable the PWA plugin until it's resolved. Please put some focus on this issue so it can be fully resolved. The solution must be completely transparent to the user and not require any page reloads. Lastly, I'd be happy to help test any solution. ~ Michael |
@ILLUMINICE adding this filter should suffice in your case (and most cases): add_filter( 'wp_service_worker_skip_waiting', '__return_false' ); We may make that the default. |
By default only difference between the unauthenticated service worker an the authenticated service worker is cache-busting for the precached offline page and error page: - "revision": "0.5.0-alpha;twentytwenty=1.1;user=1;options=53aa3ec937eff52bd7eba9d52883fd69;nav=8ec58a01006ec862ebe23639c742bcb0;deps=b3424c8adfc670221926a94d4a71af8c;473d76563bc41c392bc9f4e213f0cc35"
+ "revision": "0.5.0-alpha;twentytwenty=1.1;user=0;options=53aa3ec937eff52bd7eba9d52883fd69;nav=8ec58a01006ec862ebe23639c742bcb0;deps=b3424c8adfc670221926a94d4a71af8c;473d76563bc41c392bc9f4e213f0cc35"
- "revision": "0.5.0-alpha;twentytwenty=1.1;user=1;options=53aa3ec937eff52bd7eba9d52883fd69;nav=8ec58a01006ec862ebe23639c742bcb0;deps=b3424c8adfc670221926a94d4a71af8c;473d76563bc41c392bc9f4e213f0cc35"
+ "revision": "0.5.0-alpha;twentytwenty=1.1;user=0;options=53aa3ec937eff52bd7eba9d52883fd69;nav=8ec58a01006ec862ebe23639c742bcb0;deps=b3424c8adfc670221926a94d4a71af8c;473d76563bc41c392bc9f4e213f0cc35" In reality the offline page should not vary between logged-on and logged-out users. So when the service worker precaches the offline page, it really should do so as an unauthenticated user. We'll make that change. For now, just go ahead and disable skip-waiting. |
Thank you for the quick response @westonruter We were thinking of using the filter, but were afraid it would compromise the service worker in some way, as we're not yet familiar with the intricacies of the PWA plugin. So you're saying that there is only one difference at this time (for the authenticated vs. unauthenticated service worker), and it applies to cache-busting. We are planning to implement offline caching in the future, but we're not up to that yet. I guess we'll be facing this issue at that time.
This is curious to me, as we have a subscription based site and functionality that is based on being logged in or not. If the offline pages are not authenticated in some way, then we couldn't have any offline content for logged in users. Are you saying that offline caching can only be for unauthenticated content? In the meantime we'll go ahead and use this filter. My thanks again for weighing in on this! ~ Michael |
The offline page I'm referring to is the one singular fallback template that is served when the requested URL is not available in the cache. It had a generic "you are offline" message. It's this offline fallback page template that is precached and can vary between logged-in and logged-out users, although in practice it shouldn't ever vary. So no, pages that have been cached by previously visiting them are not affected here. |
Ah, that makes sense. I didn't realize there was a default template like that. Please excuse my ignorance, as I haven't yet had a chance to fully explore the plugin's capabilities. Ok, so if we are bypassing the service worker login status change with the filter, will this have any other effect on the plugin beyond this default template? Also, when we eventually do implement an offline caching scheme, will that be affected in any way by this filter? We're easing into the PWA development. Our initial use is to get the A2HS functionality in Android. But we do ultimately want to make full use of PWA capabilities for our web app. |
Btw, I agree with you that having a default template for uncached pages should probably be authentication agnostic, and use an unauthenticated user. |
Here's an example of what an offline error template looks like: https://weston.ruter.net/?wp_error_template=offline You can create your own by adding an The |
Thank you for explaining this in greater detail. It all makes perfect sense now. If I understand correctly, the service worker will update its logged in vs. not logged in status as soon as the page is reloaded. Even with this filter in place, the service worker will still update itself when someone logs in, so long as the page is reloaded, or a new one is loaded. Does that sound correct? To confirm, you're also saying that the entire reload functionality that was causing this issue was only in place to update the default offline template? If everything I said above is accurate, then we can definitely disable that automatic reload. I can see that we're going to need a custom default template eventually, but we can deal with the authentication status issue at that time. I agree with you that this feature should be disabled by default, assuming I properly understood everything. There's no reason to force a reload like this just to support a template that people will want to customize anyway. |
One thing that just occurred to me... We were seeing the page automatically reload whenever someone logged in. We weren't displaying any offline status page. Couldn't this reload be limited so it only occurs when the offline status page is displayed? Also, when someone logs in, this typically involves a page reload. So why wouldn't the service worker be able to update itself on that initial page load after logging in? |
I wanted to let you know that I setup that filter, and it did prevent the site from reloading upon login. However, I noticed a change in the Application tab of Chrome DevTools for the Service Workers. There is now a status of "waiting to activate" with a "skipWaiting" button. If I click that button it reloads the page and then the status says "activated and is running." So now I am left wondering, does this mean the service worker is not running by default? If that's the case this has defeated my entire purpose. I also looked to see if this status would change when reloading the page manually, or going to a different page, and it did not change. The only way to make it say "activated an is running" is by clicking that button in DevTools. Please let me know what you think... ~ Michael |
Hi Weston, I got a similar problem with my site. I get the following behaviour: But when I do visit the site some days later with an already started service worker, this happens: So basically the login-page gets shown twice. Can you help me here a bit? |
@CKranebitter This sounds like a somewhat different issue then what this thread is about. I'd recommend putting the above comment in a new ticket. This way @westonruter can reply to us all more efficiently. =) |
Just wanted to ask if you saw my last message? I'd greatly appreciate if you can fill me in on this. ~ Michael |
I did but I have not had time to respond. I believe I already answered your question but I'll try to find time this week to re-state my answer. |
That's ok, answer when you have the time. I appreciate it! You did answer my original question, but I had further questions based on what happened after implementing the filter. Here's what I wrote for convenience:
~ Michael |
What this means is that the old service worker is still running. This is the initial service worker that was installed which had the unauthenticated offline page being cached. Once you click “skipWaiting” button, then this forces it to reload the page to replace the old service worker with the new one.
I thought this would have the same effect if the there was only one tab open for the site. When there are multiple tabs open for the site these appear as “clients” of the service worker: The service worker can only update on its own when it has no more clients active. This is important as it would be bad if two tabs of the same web app were using different versions of the same service worker, or two pages that are expecting different versions of the same service worker. It could lead to application data being corrupted. This is why the waiting phase exists, as I understand. See more about skipping the waiting phase. I thought that the waiting service worker would activate when there was just a single tab open and a page navigation is being done. But I see now this is not the case, and it makes sense because the an app being controlled by the service worker could be handling navigation requests. So it's only when the page is not being controlled that the new service worker can activate. The way to force it without clicking The issue with the skip-waiting functionality in the PWA plugin is that it is accompanied by reloading the page (when pwa-wp/wp-includes/service-workers.php Lines 203 to 209 in d2713a3
It is this auto-reload logic that is causing the most headaches. I think we actually should continue to enable skipWaiting by default, but just eliminate the reload functionality. I think we also should then do
Ultimately, this issue will be mitigated by first preventing the service worker being served with authenticated state inside of it. This will prevent logging-in from causing a different service worker to be installed in the first place. But in the case when a service worker update does happen, we need to improve the user experience: continue skip-waiting, discontinue reloading, and enable client-claiming. |
Thank you for this detailed response. I'm having our developers look at this as well. I agree with your assessment that the front-end reloading is the biggest issue. If there's a way to get the service worker to update itself on login authentication without the page reloading on the front-end, that would be ideal.
I'm glad you were able to confirm this behavior. This is what I saw as well.
If that is possible, then this would be the ideal solution.
I thought the service worker was first installed using an unauthenticated state?
This sounds good to me. I'm glad you're on top of this. When you make these changes I'll be more than happy to help you test them. Please keep me posted here. Thank you again Weston! ~ Michael |
I've opened a PR to fix this and have a build for testing: #279 (comment). |
@eruditely When will you be able to test? |
I can test anytime, but next week is best for me. Thank you for making this is a priority! |
It's ready to test now, so as soon as you get a chance: #279 (comment). |
Can I grab the updated version directly from the repo? Once I have that I can deploy it to our development environment for testing. I'm pretty involved with something today, but I can get to this shortly (early next week is best). |
I linked you above to a plugin build ZIP. |
Thanks @westonruter I see it now. I downloaded: (0.5.0-alpha-88c1314-20200426T224736Z) I'll make this is a priority to test. Perhaps I can get to this over the weekend. I'll check back with you here after I'm done. |
I have some testing results for you... I installed the alpha release and deployed it to our development environment. The behavior I previously saw on login has now changed. When visiting the site for the first time (not logged in) there is a single service worker with the status "Activated and is running." That service worker remains the same when visiting other pages. When logging in that same service worker now maintains the status of "Activated and is running." The previous behavior I saw on login with the "waiting to activate" and "skipWaiting" button is gone. Basically there is no change at all on login, except for the addition of a second service worker for admin-ajax.php. I don't know the details of the back-end changes, but is this what you expected? Also, do we still need this filter?
I'm happy to run additional tests for you. Just let me know what to do. As currently configured the same service worker appears to keep running both before/after login, and there is no longer any automatic page refresh. This is looking good to me, assuming we can use the service worker to grow our PWA functionality in the future. My thanks for making this a priority! =) |
Correct. There should not be any difference to the initially-installed service worker after logging in. The only difference is that a second service worker is installed for the admin.
No, you don't need that anymore. Since the service worker won't be changing between logged-out and logged-in users, there's not going to be a waiting phase because no change in the SW will happen. Nevertheless, when a change to the SW does happen (e.g. after updating the plugin) you'll want the SW to update. So in that case, it's good to retain the skip-waiting and that filter is not needed. We removed the logic to reload the page when the SW changes, so there is no longer a concern of that happening. |
Thanks for answering all my concerns above, and for being so responsive. It's greatly appreciated! I'll remove that filter right now... All my best! |
Currently when an updated service worker is installed, the current page will reload automatically. This can be disorienting and it can even cause data to be lost, if the user starts entering data while the updated service worker is being installed.
In other words, we should discontinue skipWaiting by default. We could potentially show the service worker update button in the space where the admin bar would be displayed normally, if the admin bar is not already on the page. So we can reuse the admin bar area and class names and styles to show notices related to PWA, including offline status, new version of page available, refresh app, etc.
For more see #166 (comment).
The text was updated successfully, but these errors were encountered: