fix sw caching #326
fix sw caching #326
Conversation
|
LGTM. I can't seem to find the review button on the UI anymore. Am I the only one missing it? |
| .map(cacheName => caches.delete(cacheName)) | ||
| ).then(self.clients.claim()) | ||
| self.addEventListener('activate', event => | ||
| event.waitUntil( |
There was a problem hiding this comment.
I don't think we need to waitUntil() the previous caches are cleared—that should be able to happen async. What looks more suspicious to me is that clients.claim() does not happen immediately. I think we should be calling clients.claim() when we enter the activate event—the cache should be in a good state at that point.
There was a problem hiding this comment.
True - as our sw only accesses the latest version of the cache now.
| .then(cache => cache.match(request)) | ||
| .then(response => { | ||
| const index = this._pagesBeingLoaded.indexOf(request.url); | ||
| this._pagesBeingLoaded.splice(index, 1); |
There was a problem hiding this comment.
Not really for this PR but why is _pagesBeingLoaded modified here? It looks like this keeps track of "in-flight" requests and so I would expect it to be modified when the response is received, but instead it seems to be modified when fetchWithPageTransition() is called a second time with the same request??
There was a problem hiding this comment.
This is to support Gulliver's network first strategy -> only return the cached version if a loading page has been requested before. This is not a perfect solution as it can't handle (theoretical) parallel requests to the same page.
|
PTAL |
sebastianbenz
force-pushed
the
sw-caching-fix
branch
from
2e64109
to
1ce3fa9
Compare
|
LGTM |
No description provided.