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

Best practices for cleaning up old caches #1407

Closed
zevdg opened this issue Mar 29, 2018 · 8 comments
Closed

Best practices for cleaning up old caches #1407

zevdg opened this issue Mar 29, 2018 · 8 comments
Labels

Comments

@zevdg
Copy link

zevdg commented Mar 29, 2018

Library Affected:
workbox-sw

Browser & Platform:
all browsers

Issue or Feature Request Description:
I just recently migrated from sw-precache to workbox, and I noticed that my old cache from sw-precache was sticking around indefinitely. This would also be an issue for anyone adding or changing their cacheId/prefix or suffix. I suspect that chrome will maybe clean these up eventually, but my limited understanding of service worker caches is that for the most part, chrome expects you to clean up after yourself, so lets do that. I started by looking for an option in the webpack GenerateSW plugin to delete caches that aren't part of my workbox configuration and was surprised not to find one. No matter. I expected to have to "eject" to InjectManifest eventually anyway, so I do that and wrote the following snippet

// clean up old SW caches
self.addEventListener("activate", function(event) {
  event.waitUntil(
    caches.keys().then(function(cacheNames) {
      let validCacheSet = new Set(Object.values(workbox.core.cacheNames));
      return Promise.all(
        cacheNames
          .filter(function(cacheName) {
            return !validCacheSet.has(cacheName);
          })
          .map(function(cacheName) {
            console.log("deleting cache", cacheName);
            return caches.delete(cacheName);
          })
      );
    })
  );
});

So far so good, but there's this mysterious "temp cache" that is very sparsely documented, and deleting it may cause some problems? #1368 (comment) Maybe not, but better safe than sorry.

So we add a simple validCacheSet.add(workbox.core.cacheNames.precache+"-temp"). This is hacky and depends on the implementation detail of the temp cache name, but it works.

Then I cache my google fonts as per the instructions at https://developers.google.com/web/tools/workbox/guides/common-recipes#google_fonts and notice that this new cache name (which is required by the workbox.expiration plugin) isn't getting added to workbox.core.cacheNames, so I keep deleting it. Now to fix that, I have to keep track of my dynamic cache names too. So now I have something like this.

let currentCacheNames = Object.assign(
  { precacheTemp: workbox.core.cacheNames.precache + "-temp" },
  workbox.core.cacheNames
);

currentCacheNames.fonts = "googlefonts";
workbox.routing.registerRoute(
  /https:\/\/fonts.(?:googleapis|gstatic).com\/(.*)/,
  workbox.strategies.cacheFirst({
    cacheName: currentCacheNames.fonts,
    plugins: [new workbox.expiration.Plugin({ maxEntries: 30 })]
  }),
  "GET"
);

// clean up old SW caches
self.addEventListener("activate", function(event) {
  event.waitUntil(
    caches.keys().then(function(cacheNames) {
      let validCacheSet = new Set(Object.values(currentCacheNames));
      return Promise.all(
        cacheNames
          .filter(function(cacheName) {
            return !validCacheSet.has(cacheName);
          })
          .map(function(cacheName) {
            console.log("deleting cache", cacheName);
            return caches.delete(cacheName);
          })
      );
    })
  );
});

So I think I'm more or less doing the right thing now, but

  1. I'm not sure this is even best practice (should i just let chorme clean these up? do i need to avoid deleting the temp cache?)
  2. it took me a few tries to get here
  3. I'm still depending on an undocumented implementation detail (the temp cache naming convention) and that makes me nervous

So I suggest 2 changes that would each independently improve the situation.

  1. additional documentation about cleaning up old cache names. Probably a snippet in https://developers.google.com/web/tools/workbox/guides/common-recipes and references to it under the cacheId webpack propery, the sw-precache migration guide, and workbox.setCacheNameDetails
  2. expose every cache name workbox knows about instead of just the precache and the runtime names.

I wont bike shed the semantics, but to give a concrete example, maybe something like

> workbox.core.cacheNames.runtime
"foo-runtime-v1"

> workbox.core.cacheNames.precache
"foo-precache-v1"

> workbox.core.cacheNames.precacheTemp
"foo-precache-v1-temp"

> workbox.core.cacheNames.dynamic()
["googleapis", "images"]
// or alternatively 
Set { "googleapis", "images" }

> workbox.core.cacheNames.all()
["foo-runtime-v1", "foo-precache-v1", "foo-precache-v1-temp", "googleapis", "images"]
// or alternatively
Set { "foo-runtime-v1", "foo-precache-v1", "foo-precache-v1-temp", "googleapis", "images" }

With this new hypothetical workbox.core.cacheNames.all() function, my initial solution (modified slightly to use this new function) becomes the correct one.

Of course, you could also expose even higher level options on top of these like a "deleteNonWorkboxCaches" function or option on the webpack GenerateSW plugin, but that's probably overkill unless this pattern is extremely common.

@josephliccini
Copy link
Contributor

josephliccini commented Mar 30, 2018

We used a technique outlined in this talk: https://youtu.be/fGTUIlEM0m8?t=825

We did have to create our own, parse-able cache naming scheme. It think having something default in workbox would be better.

Basically our cacheName of our caches was a value like this:
my-app-runtime-cache<|globalVersion=1:localVersion=2:cacheName=foo|>

Then on activate, clear out globalVersion mismatches, localVersion mismatches, and any unrecognized cacheNames

@mocheng
Copy link

mocheng commented Sep 11, 2018

@zevdg Thanks for the sharing.
One more thing, workbox takes advantage of indexed DB to store info about content in Cache Storage. To totally clean out cache, Indexed DB should be cleared as well.

Unfortunately, it looks workbox doesn't expose any helper function to access indexedDB. So, we have to manually code it.

@jeffposnick
Copy link
Contributor

Unfortunately, it looks workbox doesn't expose any helper function to access indexedDB. So, we have to manually code it.

We added a deleteCacheAndMetadata() method to the workbox.expiration.Plugin class in Workbox v3.3.0. So manually cleaning up both the cache and IndexedDB data for a runtime cache that uses expiration is now easier.

I'm leaving this open as this issue deals with automatically cleaning up caches from sw-precache or an incompatible, versioned cache from Workbox, which still can't be done in an automated fashion.

@davidmaxwaterman
Copy link

davidmaxwaterman commented Oct 16, 2018

There doesn't seem to be an example of how to use deleteCacheAndMetadata(). The example in the README focuses on purgeOnQuotaError. Any chance one could be added?

[edit] https://stackoverflow.com/questions/51770033/manually-execute-workbox-expiration-plugin-deletecacheandmetadata-for-a-route

I don't seem to be able to make an expiration plugin without also configuring either maxEntries or maxAgeSeconds, either of which changes the behaviour of my cache. The docs mark them both as optional, but it seems they aren't optional at the same time. When I run it from the console, I get:

workbox-cache-expiration.dev.js:490 Uncaught WorkboxError: You must define either config.maxEntries or config.maxAgeSecondsin workbox-cache-expiration.Plugin.constructor
    at new Plugin (http://localhost:8088/workbox-v3.6.2/workbox-cache-expiration.dev.js:490:17)
    at <anonymous>:1:17

(missing space in error message is sic)

Is there something I'm missing that implicitly ties deleteCacheAndMetadata() to the functionality of the expiration plugin? Perhaps the IndexedDB is created and used only by the expiration plugin and otherwise we can just use caches.delete(<cacheName>)? This isn't clear to me.

@jeffposnick
Copy link
Contributor

Perhaps the IndexedDB is created and used only by the expiration plugin and otherwise we can just use caches.delete(<cacheName>)? This isn't clear to me.

Apologies that this isn't clear—you're correct that IndexedDB only enters the picture if you're using workbox.expiration. If you're not using expiration already, you can just call caches.delete(<cacheName>) as you suggest.

It's also worth noting that in the v4.0.0 betas, the -temp cache is now automatically cleaned up (#1736).

Providing either a complete list of "known" cache names, or a way of passing in a cache name and getting back a Boolean indicating whether it "belongs" to Workbox is still not implemented, and has some challenges that might make it difficult to work for arbitrary runtime caches.

@jeffposnick
Copy link
Contributor

FYI, Workbox v4 brings a breaking change to the precache format. (This is different from the -temp cache deletion mentioned above. We're just moving away from -temp caches entirely.) All existing precaches will have to be recreated following an update to Workbox v4.

We're proving a new method, workbox.precaching.cleanupOutdatedCaches() that developers can opt-in to using, which will attempt to find outdated precaches during activate and delete them.

It relies on precache naming conventions, and it's possible (if unlikely) that if a developer used a similar naming convention for their own runtime caches, it could end up deleted those runtime caches as well. For that reason, it's not enabled by default, but if you know that you've never created a runtime cache with '-precache-' in its name, you can safely call that new method in v4.

I'm going to leave this open until Workbox v4 is finalized and folks have a chance to try it out, but hopefully workbox.precaching.cleanupOutdatedCaches() will be the solution.

@ishu-koundal
Copy link

@jeffposnick

I need some help.

I am creating PWA with cakePHP. My app is working fine(PWA workbox). But I have some problems or requirements for the app.

Can App cache be updated automatically on any app changes(text,
images, CSS, js).
In my case I cannot define the cache version in the Service worker and cannot define the Maxagesecond method of Workbox.

@davidmaxwaterman
Copy link

Perhaps the IndexedDB is created and used only by the expiration plugin and otherwise we can just use caches.delete(<cacheName>)? This isn't clear to me.

Apologies that this isn't clear—you're correct that IndexedDB only enters the picture if you're using workbox.expiration. If you're not using expiration already, you can just call caches.delete(<cacheName>) as you suggest.

@jeffposnick
FWIW, this doesn't seem to work for me. All that caches.delete('...precache-v1') does is empty/delete the cache. If I reloaded the page, workbox doesn't not then repopulate the cache. I notice that, while the precache is empty, the indexedDB is still populated. I did try deleting that, but workbox still does not repopulate the precache.

Essentially, I'm attempting to have a 'reset' button in my app, which returns everything to its initial state, much like the Application/Clear storage/Clear site data button does in chrome devtools.
Any hints for making this work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants