Pass-through caching demo. #40
Conversation
| console.log(' No response for %s found in cache. About to fetch from network...', event.request.url); | ||
|
|
||
| return fetch(event.request).then(function(response) { | ||
| console.log(' Response for %s from network is: %O', event.request.url, response); |
There was a problem hiding this comment.
I'd recommended checking the response was valid AND that you got a response.status == 200. If either of those are broken, then bail.
There was a problem hiding this comment.
What would checking that the response is valid entail—just a null check (are there any cases in which fetch() resolves with null instead of rejecting?) or is there some specific property of response that should be checking? I see that there's a termination reason defined as part of the response object, but I don't actually see it mapped to a property of what gets retruned in the current Chrome implementation.
Additionally, since Chrome returns opaque filtered responses for non-CORS requests, it's possible that response.status == 0 is a legitimate, cachable response. So it's a tradeoff between never caching opaque responses or caching them unconditionally, and risking caching a transient error response.
That being said, I will at least check response.status >= 400, to avoid caching any non-opaque error responses.
There was a problem hiding this comment.
I was discussing this point with a colleague and we both ended up agreeing that caching only 200 and 0 responses would be the safest bet for a recommended sample.
It will save people a lot of pain should a bad response get cached.
There was a problem hiding this comment.
I talked to @slightlyoff about this, and he was (somewhat?) surprised that opaque filtered responses always had a status of 0, and suggested that I file whatwg/fetch#14 to see whether that restriction could be relaxed in the fetch() spec.
I'll keep caching whenever response.status < 400 and make a point of specifically highlighting this tradeoff in the comments. As long as the fetch() spec stays the same, it'll be each developer's decision whether they want to err on the side of caching too much or too little.
| <link rel="icon" sizes="192x192" href="../../images/touch/chrome-touch-icon-192x192.png"> | ||
|
|
||
| <!-- Add to homescreen for Safari on iOS --> | ||
| <!-- TODO: Replace PLACEHOLDER with feature name. --> |
There was a problem hiding this comment.
PLACEHOLDER seems to have been removed?
There was a problem hiding this comment.
Yes, thanks for catching that.
|
So I've added some caveats to explain that this particular aggressive form of caching is not appropriate for many use cases. I am going to merge this now since I think it's still useful as a sample, but the other samples we put out might be more relevant for real-world usage. |
Pass-through caching demo.
@KenjiBaheux @jakearchibald @slightlyoff & co.:
Here's my initial attempt at a pass-through caching example, closing #31.
You can see a live demo of this code at https://rawgit.com/jeffposnick/samples/sw-pass-through-caching/service-worker/pass-through-caching/index.html