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

Expiration not working - no way to ignore vary header #2206

Closed
jazdw opened this issue Aug 27, 2019 · 4 comments · Fixed by #2533
Closed

Expiration not working - no way to ignore vary header #2206

jazdw opened this issue Aug 27, 2019 · 4 comments · Fixed by #2533

Comments

@jazdw
Copy link

jazdw commented Aug 27, 2019

Library Affected:
workbox-sw I think

Browser & Platform:
Chrome 76.0.3809.100

Issue or Feature Request Description:
I'm not sure if this is an issue with my configuration.

I am seeing a message like Expired 1 entry and removed it from the 'module-resources' cache. in my console indicating that the entry should be removed from the cache. However it seem that CacheExpiration.mjs is calling cache.delete() with the absolute URL (with https and domain name) whereas the cache key is the relative URL (without the https and domain name).

Service worker (generated/injected by Webpack plugin):

importScripts("/ui/vendor/precache-manifest.0bda9a9248d013fd973b8db7b30cc6ef.js", "/ui/vendor/workbox-v4.3.1/workbox-sw.js");
workbox.setConfig({modulePathPrefix: "/ui/vendor/workbox-v4.3.1"});

workbox.setConfig({
    debug: true
});

workbox.core.skipWaiting();
workbox.core.clientsClaim();

const moduleResourcesCacheName = 'module-resources';

workbox.routing.registerRoute(
    /\/modules\/[\w-]+\/web\/.*\?v=.+/,
    new workbox.strategies.CacheFirst({
        cacheName: moduleResourcesCacheName,
        plugins: [
            new workbox.expiration.Plugin({
                maxEntries: 100
            })
        ]
    })
);

workbox.precaching.precacheAndRoute(self.__precacheManifest, {
    directoryIndex: null,
    cleanUrls: false
});

workbox.routing.registerNavigationRoute(
    workbox.precaching.getCacheKeyForURL('/ui/index.html')
);

The cache-entries DB contains the absolute URLs. The names in the module-resources cache storage are all relative URLs.

Is this user error or a bug? I have also noticed that adding a ^ to the start of the RegExp makes it not match, which seems contrary to what the documentation says for same origins.

@jazdw
Copy link
Author

jazdw commented Aug 27, 2019

In fact it seems to make no difference if I call cache.delete() with an absolute URL or a relative URL. According to https://developer.mozilla.org/en-US/docs/Web/API/Request/Request you should call cache.delete() with a Request, not a URL. This is indeed what PrecacheController.mjs does.

@jazdw
Copy link
Author

jazdw commented Aug 27, 2019

So it looks like cache.delete() does work with a URL, its the Vary header which is giving me problems.

Screenshot of cached request -
image

I've found there is a way to set matchOptions for the CacheFirst class -

matchOptions: {
    ignoreVary: true
}

But it seems it is not possible for the expiration plugin?

@jazdw jazdw changed the title Expiration not working - wrong url passed to cache.delete() Expiration not working - no way to ignore vary header Aug 29, 2019
@philipwalton
Copy link
Member

Thanks for the suggestion. I think it may make sense to add this as a configuration option on the expiration plugin. Perhaps something like:

new ExpirationPlugin({
  maxEntries: 100,
  deleteOptions: {
    ignoreVary: true,
  },
});

@jeffposnick what do you think? And in terms of naming, would you prefer deleteOptions or matchOptions (for consistency)?

@jeffposnick
Copy link
Contributor

Sure, that makes sense.

The underlying web platform dictionary name is CacheQueryOptions, but since we went with matchOptions elsewhere in Workbox already, I'd say we should just stick with that name here.

I'm not sure if this would make the cut for v5.0.0, or whether it would be something we'd add in the following minor/patch release.

@jeffposnick jeffposnick self-assigned this Sep 3, 2019
davidtaylorhq added a commit to discourse/discourse that referenced this issue Aug 23, 2023
By default, the workbox-expiration plugin will not expire cache entries which include a `Vary` header in the response. This means that cached entries can build up until the browser storage quota is hit.

This commit introduces the `ignoreVary: true` option, so that deletion is performed correctly. This will only apply going forward, so this commit also bumps the cache version and deletes the old caches.

Ref GoogleChrome/workbox#2206
davidtaylorhq added a commit to discourse/discourse that referenced this issue Aug 23, 2023
By default, the workbox-expiration plugin will not expire cache entries which include a `Vary` header in the response. This means that cached entries can build up until the browser storage quota is hit.

This commit introduces the `ignoreVary: true` option, so that deletion is performed correctly. This will only apply going forward, so this commit also bumps the cache version and deletes the old caches.

Ref GoogleChrome/workbox#2206
davidtaylorhq added a commit to discourse/discourse that referenced this issue Aug 23, 2023
By default, the workbox-expiration plugin will not expire cache entries which include a `Vary` header in the response. This means that cached entries can build up until the browser storage quota is hit.

This commit introduces the `ignoreVary: true` option, so that deletion is performed correctly. This will only apply going forward, so this commit also bumps the cache version and deletes the old caches.

Ref GoogleChrome/workbox#2206
davidtaylorhq added a commit to discourse/discourse that referenced this issue Aug 23, 2023
By default, the workbox-expiration plugin will not expire cache entries which include a `Vary` header in the response. This means that cached entries can build up until the browser storage quota is hit.

This commit introduces the `ignoreVary: true` option, so that deletion is performed correctly. This will only apply going forward, so this commit also bumps the cache version and deletes the old caches.

Ref GoogleChrome/workbox#2206
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants