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

Allow cache.update/add to accept request objects #118

Closed
jakearchibald opened this issue Nov 2, 2013 · 3 comments
Closed

Allow cache.update/add to accept request objects #118

jakearchibald opened this issue Nov 2, 2013 · 3 comments

Comments

@jakearchibald
Copy link
Contributor

Usecase:

this.onfetch(function(event) {
  var responsePromise = fetch(event.request);
  event.respondWith(responsePromise);

  responsePromise.then(function(response) {
    caches.get('api').addResponse(event.request.url, response);
  });
});

The above lets me respond from the network but update the cache at the same time. Pretty confident this will be a common pattern. It works, assuming "addResponse" doesn't throw if the url already exists.

This wouldn't work as well:

this.onfetch(function(event) {
  caches.get('api').add(event.request.url);
  event.respondWith(fetch(event.request));
});

Neater, but the two requests could differ in headers so the browser may make two requests.

Ideally:

this.onfetch(function(event) {
  caches.get('api').add(event.request);
  event.respondWith(fetch(event.request));
});

This depends on cache.add taking requests & being smart enough to make a single request.

@alecf
Copy link
Contributor

alecf commented Nov 4, 2013

Just to clarify, are you hoping to be able to do this:

this.onfetch = function(event) {
    event.respondWith(caches.add(event.request));
}

If so, I support this. I think this is a nice API usability thing..

I was going to say "I think this is just a generalization of the add(url) case" but I see that there's kind of a typo in the spec:

  add(...response:URL[]) : Promise;

I suspect that response is supposed to be request

@jakearchibald
Copy link
Contributor Author

I hadn't thought of caches.add returning a promise for the response. I'm happy with that.

I just want to avoid cases where:

caches.get('api').add(event.request.url);
event.respondWith(fetch(event.request));

…would trigger two requests.

Allowing add to take requests feels right, since it's current argument, a url, is going to be turned into a request anyway, you just don't get control over the methods, headers etc.

(yeah, I think response is supposed to be url or requestUrl in the spec. You can feed responses directly into caches, but urls and requests should work too)

@jakearchibald
Copy link
Contributor Author

The new cache API is request-based

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

No branches or pull requests

3 participants