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

In goapp.js activate should remove old cache. #302

Closed
bgokden opened this issue Oct 23, 2019 · 2 comments · Fixed by #304
Closed

In goapp.js activate should remove old cache. #302

bgokden opened this issue Oct 23, 2019 · 2 comments · Fixed by #304

Comments

@bgokden
Copy link

bgokden commented Oct 23, 2019

When a service worker updated due to a new version released. New versions of the files should be cached and old ones should be removed.

Google has a recommended way of doing this explained here:
https://developers.google.com/web/fundamentals/primers/service-workers#update-a-service-worker

We can change activate the function to this if you accept I would like to create a pull request.

self.addEventListener('activate', function(event) {
  
  console.log('goapp worker', etag, 'is activated')

  var cacheWhitelist = [
        {{range .Paths}}'{{.}}',
        {{end}}'/'
      ];

  event.waitUntil(
    caches.keys().then(function(cacheNames) {
      return Promise.all(
        cacheNames.map(function(cacheName) {
          if (cacheWhitelist.indexOf(cacheName) === -1) {
            return caches.delete(cacheName);
          }
        })
      );
    })
  );
});
@maxence-charriere
Copy link
Owner

Hi @bgokden , thank you for pointing this.

Look like your snippet does not use the right values to put in cache white list. It appears it takes caches names rather than paths.

Also goapp is using only one cache so it will never be deleted.
I think having the code that already match what is suggested by google is a good to have, especially if in the future we decide to use another cache or rename the current one.

I just got a PR that add this part.

Thanks you again.

@maxence-charriere
Copy link
Owner

Also this is an open source project, if you think you have a fix or an improvement, feel free to open a PR.
I ll review it :).

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 a pull request may close this issue.

2 participants