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

Refresh Button / New Service Worker #2364

Closed
novaknole opened this issue Feb 12, 2020 · 10 comments
Closed

Refresh Button / New Service Worker #2364

novaknole opened this issue Feb 12, 2020 · 10 comments
Labels
Needs More Info Waiting on additional information from the community. workbox-window

Comments

@novaknole
Copy link

novaknole commented Feb 12, 2020

Library Affected:
workbox-webpack-plugin

Browser & Platform:
Latest

Issue or Feature Request Description:

I have been reading a lot on this url: https://redfin.engineering/how-to-fix-the-refresh-button-when-using-service-workers-a8e27af6df68

If you check the 3rd approach, that guy is talking about something I don't understand. I even downloaded his sample app, but still it's not working as he says it should.

He says that if by navigation or whatever makes browser check service worker, if there's a new one, we show users a prompt if they want to refresh or not. If user has multiple tabs, that article says that the prompt will be shown on all tabs of our apps. (This doesn't happen at all). and if clicked on refresh on one of the tabs, all the other tabs will be refreshed(This doesn't happen at all). His sample app is : https://github.com/dfabulich/service-worker-refresh-sample

I even saw this article's link on workbox's documentation. and I've read @jeffposnick @jakearchibald say it's a good article. Any ideas about what I am saying?

@jeffposnick
Copy link
Contributor

(I can't really attest to the state of the code examples in https://github.com/dfabulich/service-worker-refresh-sample, but I'd recommend opening an issue on that GitHub repo if you feel like something there isn't working.)

In general, I'd recommend using workbox-window to paper over some of the messier details of the service worker update flow, with the recipe in our documentation as one approach that tends to work well.

Have you tried that recipe out? That would be the most straightforward thing for us to support via an issue in the Workbox repo.

@jeffposnick jeffposnick added Needs More Info Waiting on additional information from the community. workbox-window labels Feb 12, 2020
@novaknole
Copy link
Author

novaknole commented Feb 12, 2020

Okay @jeffposnick Thanks for your comment. I saw that recipe.

Following questions: keep in mind i have a SPA app.

a) I think I put that code from that recipe in my main.js. Okay all is good. Now, when the route changes, nothing happens because of there's no route navigation for SPA, so I decided to have a watcher which tracks route change in App.vue. Here is how it might look for simple service worker, but if I use workbox, how do you change that code? What should I put here?

watch:{
   $route(to, from){
       //route changed. call this, but what do I put here if i use workbox?
      navigator.serviceWorker.ready.then((reg)=>{
           reg.update();
      })
   }
}

@jeffposnick
Copy link
Contributor

Calling update() like you're doing in that code sample is the correct way to explicitly request service worker update check.

(The fact that you're using Workbox doesn't change anything in regards to triggering an update check.)

@novaknole
Copy link
Author

novaknole commented Feb 12, 2020

@jeffposnick

Vue.prototype.$checkForUpdate = () => {
    navigator.serviceWorker.ready.then((reg) => {
        reg.update();
    })
}

if ('serviceWorker' in navigator) {
    const wb = new Workbox('/service-worker.js');
    
    Vue.prototype.$showButton = false;

    wb.addEventListener('waiting', (event) => {
        console.log("waiting");
    });
  
    wb.register();
}

This is what i put in my main.js

in my route watcher, i got this:

$route (to, from){
           console.log("route changed");
           this.$checkForUpdate();
   } 

The problem is checkForUpdate is called each time route changes. and reg.update() gets called. but it automatically installs new service worker. It doesn't go to waiting listener at all. I don't have skipWaiting here , but still it installs automatically. Any idea? The reason I am saying this is a problem is because as waiting listener doesn't get called (where i got my showprompt code), I can't show the user the prompt at all.

@jeffposnick jeffposnick reopened this Feb 12, 2020
@jeffposnick
Copy link
Contributor

@philipwalton might have some insight here—any idea why workbox-window's event listener for waiting might not be fired when there's an updated service worker that ends up installed via a call to reg.update()?

@novaknole, what happens in your local sample app when there's a "real" navigation following a deployment of an updated service worker. Do you see the waiting event listener called in that scenario?

@novaknole
Copy link
Author

novaknole commented Feb 12, 2020

@jeffposnick What i did as you suggested is after I changed my service worker's code, I refreshed the page(I guess this is real navigation you mean and it went into the listener waiting state and didn't try to activate my new service worker, only installed it).

Any clue?

@jeffposnick
Copy link
Contributor

Okay, thanks for confirming that.

It sounds similar to #2031, and I'm going to close this in favor of moving the discussion there.

@novaknole
Copy link
Author

novaknole commented Feb 12, 2020

@jeffposnick Question 1: Will this be solved? what's your idea of how I should proceed?

@novaknole
Copy link
Author

novaknole commented Feb 12, 2020

@jeffposnick Question 2: I've decided to take this approach because skipWaiting is kind of risky without this approach that we are trying to solve now. Just a simple question. I know you have talked about that there might be 2 risks when it's not safe to use skipWaiting and those 2 risks both are when there's lazy-loading content.

Do you see any other risks while using skipWaiting?

@jeffposnick
Copy link
Contributor

Regarding "Question 1," what you've described in this issues sounds very much like #2031, so I've moved the conversation there.

Regarding "Question 2," while I've written in the past about the risks of versioned assets and lazy-loading, and how service worker precaching + skipWaiting() can trigger some of those risks, no one could say authoritatively, "These are the only possible things that could go wrong." I don't have any other general guidance to share about how things might go wrong, but sure, there might be other risks depending on your exact architecture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info Waiting on additional information from the community. workbox-window
Projects
None yet
Development

No branches or pull requests

2 participants