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

Why doesn't cache.match support cacheName #917

Closed
remy opened this issue Jun 22, 2016 · 6 comments
Closed

Why doesn't cache.match support cacheName #917

remy opened this issue Jun 22, 2016 · 6 comments
Milestone

Comments

@remy
Copy link

remy commented Jun 22, 2016

Going by the docs on MDN it reads regarding the options.cacheName to the cache.match() method:

Note that this option is ignored by Cache.match

It seems odd that I can't filter the cache that I'm searching for the .match(), but I can filter when I use .matchAll(). Without the filtering, I have to open the specific cache, and then run the match against the specific cache.

Have I misunderstood or does it actually make sense to add the support to .match()?

@NekR
Copy link

NekR commented Jun 22, 2016

Cache.match() runs Cache.matchAll() internally.
Cache.matchAll() uses Query Cache algorithm which doesn't use CacheQueryOptions.cacheName in it.

I don't know why you may want to filter by cache name inside a cache, plus, Cache doesn't know its name and doesn't have a name property, so it's actually impossible at this moment.

I guess you just confused Cache and CacheStorage. The last uses CacheQueryOptions.cacheName to match in specified cache instead in all caches in the storage.

P.S. No idea what MDN says and why.

@wanderview
Copy link
Member

Yea, the spec defines a single CacheQueryOptions that can be used with:

  • caches.match()
  • cache.match()
  • cache.matchAll()
  • cache.delete()
  • cache.keys()

The cacheName field only applies to the caches.match() though because in all other cases you are calling the method on a specific cache object already.

Also note that cacheName was recently changed to simply return undefined if the cache object cannot be found. Previously this threw a TypeError. Right now there are probably browsers in the wild that do both, so you want to be careful with cacheName.

@remy
Copy link
Author

remy commented Jun 25, 2016

Okay, so if I've understood the replies correctly, firstly the MDN docs are wrong, but in particular, instead of writing this:

return caches.open('special-assets').then(cache => cache.match(request));

I can write this:

return caches.match(request, { cacheName: 'special-assets' });

Is that right?

@NekR
Copy link

NekR commented Jun 25, 2016

Yes, seems correct. But also add .catch(...) to the end of caches.match(...) because older versions of browsers reject that promise if cache doesn't exist (new versions should just fulfill with undefined). So it should be like this:

return caches.match(request, { cacheName: 'special-assets' }).catch(function() {});

@wanderview
Copy link
Member

I think the MDN docs are correct, if confusing. If you are doing caches.match, then you want this page;

https://developer.mozilla.org/en-US/docs/Web/API/CacheStorage/match

Where cacheName has an effect.

If you are using Cache.match (different object type from CacheStorage) then cacheName has no effect.

@jakearchibald
Copy link
Contributor

Seems like this isn't an issue. Reopen if I'm wrong.

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

No branches or pull requests

4 participants