Re: [ServiceWorker] Fetch API respondWith's implicit RETURN (#844)

> don't you want to go to cache even if the networkFetchPromise was thenable but respons.ok was false?

Sure thing. Probably want to avoid those going into the cache too…

```js
self.addEventListener('fetch', event => {
  const cacheResponsePromise = caches.match(event.request);
  const networkFetchPromise = fetch(event.request);

  // # Populating the cache from the network
  // WARNING: by adding all responses into the cache like this, you'll run out
  // of space pretty quickly, you should restrict the URLs you do this for, or clean
  // the cache at some points
  event.waitUntil(
    networkFetchPromise.then(response => {
      // If the response wasn't ok, we're not interested
      if (!response.ok) return;
      // clone the response, because we're going to use it twice
      const responseClone = response.clone();

      return caches.open('my-cache-name')
        .then(cache => cache.put(event.request, responseClone));
    })
  );

  // # Responding to the request
  event.respondWith(
    // race the network and a timeout
    Promise.race([
      networkFetchPromise,
      new Promise((_, reject) => {
        setTimeout(() => reject(Error('Timeout')), 2000);
      })
    ]).then(response => {
      // Prefer the cache if the response is not ok
      if (!response.ok) throw Error('Not ok');
      return response;
      // if this times out or rejects, see if the cache came up with anything…
    }).catch(() => cacheResponsePromise).then(response => {
      // if the cache has no match, defer to the network (without a timeout)
      // Note: you didn't say you wanted this, but it seems to make sense,
      // a slow network response is better than no response.
      return response || networkFetchPromise;
    })
  );
});
```

> is the network-hit response delayed until after you've updated the cache?

It works the same as this http://jsbin.com/lidoda/edit?js,console. The first attached `then` executes first, but async actions within a `then` handler don't block other handlers on the same promise. I'm relying on the ordering for the response cloning.

> doesn't event.request have to be cloned in there somewhere?

It is, see `const responseClone = response.clone()`.

Although we're using a timeout, this approach still suffers from being online-first. A better approach is offline-first https://jakearchibald.com/2014/offline-cookbook/#cache-then-network

---
Reply to this email directly or view it on GitHub:
https://github.com/slightlyoff/ServiceWorker/issues/844#issuecomment-196324648