-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
Offline cache #2508
base: master
Are you sure you want to change the base?
Offline cache #2508
Changes from 5 commits
d7c3880
503b672
b0922d5
14f1fce
4982a6b
cb79ff9
f7722ab
c2fd4ef
3680a42
29efb80
bfb73e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -388,7 +388,9 @@ export default (sbp('sbp/selectors/register', { | |
}) | ||
} else { | ||
try { | ||
await sbp('chelonia/contract/sync', identityContractID) | ||
if (navigator.onLine !== false) { | ||
await sbp('chelonia/contract/sync', identityContractID) | ||
} | ||
Comment on lines
+391
to
+393
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So... do we sync this once we do come back online? If so, where? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, we don't explicitly. It'd be handled like any other contract by that point. |
||
} catch (e) { | ||
// Since we're throwing or returning, the `await` below will not | ||
// be used. In either case, login won't complete after this point, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
import sbp from '@sbp/sbp' | ||
import { NOTIFICATION_TYPE } from '~/shared/pubsub.js' | ||
|
||
const CACHE_VERSION = '1.0.0' | ||
const CURRENT_CACHES = { | ||
assets: `assets-cache_v${CACHE_VERSION}` | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this version number be tied to some environment variable, e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking about that. It could, but I don't think it needs to, so long as it's updated if breaking changes to the cache are made. |
||
|
||
if ( | ||
typeof Cache === 'function' && | ||
typeof CacheStorage === 'function' && | ||
typeof caches === 'object' && | ||
(caches instanceof CacheStorage) | ||
) { | ||
sbp('okTurtles.events/on', NOTIFICATION_TYPE.VERSION_INFO, (data) => { | ||
if (data.GI_VERSION !== process.env.GI_VERSION) { | ||
caches.delete(CURRENT_CACHES.assets).catch(e => { | ||
console.error('Error trying to delete cache', CURRENT_CACHES.assets) | ||
}) | ||
} | ||
}) | ||
|
||
const locationUrl = new URL(self.location) | ||
const routerBase = locationUrl.searchParams.get('routerBase') ?? '/app' | ||
|
||
self.addEventListener('install', (event) => { | ||
event.waitUntil( | ||
caches | ||
.open(CURRENT_CACHES.assets) | ||
.then((cache) => | ||
cache.addAll([ | ||
`${routerBase}/` | ||
]).catch(e => { | ||
console.error('Error adding initial entries to cache') | ||
}) | ||
) | ||
) | ||
}) | ||
|
||
// Taken from the MDN example: | ||
// <https://developer.mozilla.org/en-US/docs/Web/API/Cache> | ||
self.addEventListener('activate', (event) => { | ||
const expectedCacheNamesSet = new Set(Object.values(CURRENT_CACHES)) | ||
|
||
event.waitUntil( | ||
caches.keys().then((cacheNames) => | ||
Promise.allSettled( | ||
cacheNames.map((cacheName) => { | ||
if (!expectedCacheNamesSet.has(cacheName)) { | ||
// If this cache name isn't present in the set of | ||
// "expected" cache names, then delete it. | ||
console.log('Deleting out of date cache:', cacheName) | ||
return caches.delete(cacheName) | ||
} | ||
|
||
return undefined | ||
}) | ||
) | ||
) | ||
) | ||
}) | ||
|
||
self.addEventListener('fetch', function (event) { | ||
console.debug(`[sw] fetch : ${event.request.method} - ${event.request.url}`) | ||
|
||
if (!['GET', 'HEAD', 'OPTIONS'].includes(event.request.method)) { | ||
return | ||
} | ||
|
||
let request = event.request | ||
|
||
try { | ||
const url = new URL(request.url) | ||
if (url.pathname.startsWith('/file')) { | ||
return | ||
} | ||
|
||
// If the route starts with `${routerBase}/`, use `${routerBase}/` as the | ||
// URL, since the HTML content is presumed to be the same. | ||
if (url.pathname.startsWith(`${routerBase}/`)) { | ||
request = new Request(`${routerBase}/`, request) | ||
} | ||
} catch (e) { | ||
return | ||
} | ||
|
||
event.respondWith( | ||
caches.open(CURRENT_CACHES.assets).then((cache) => { | ||
return cache | ||
.match(request) | ||
.then((cachedResponse) => { | ||
if (cachedResponse) { | ||
// If we're offline, return the cached response, if it exists | ||
if (navigator.onLine === false) { | ||
return cachedResponse | ||
} | ||
} | ||
|
||
return fetch(request.clone()).then(async (response) => { | ||
if ( | ||
// Save successful reponses | ||
response.status >= 200 && | ||
response.status < 400 && | ||
response.status !== 206 && // Partial response | ||
response.status !== 304 && // Not modified | ||
// Which don't have a 'no-store' directive | ||
!response.headers.get('cache-control')?.split(',').some(x => x.trim() === 'no-store') | ||
) { | ||
await cache.put(request, response.clone()).catch(e => { | ||
console.error('Error adding request to cache') | ||
}) | ||
} else { | ||
await cache.delete(request) | ||
} | ||
|
||
return response | ||
}).catch(e => { | ||
if (cachedResponse) { | ||
console.warn('Error while fetching', request.url, e) | ||
// If there was a network error fetching, return the cached | ||
// response, if it exists | ||
return cachedResponse | ||
} | ||
|
||
throw e | ||
}) | ||
}) | ||
}) | ||
) | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... why is this being added? I see that we have a special section for cached files above, so why should these be cached too when it seems like these are files that might change?
And if this is really necessary, could you add a comment here explaining why?
I want to avoid the app failing to update when files are updated...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because if we don't store these in the cache, the app can't possibly work offline. Adding
stale-while-revalidate
means that it's fine to serve stale files while they're being revalidated (normally, all cached requests need to be revalidated).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any case, the comment is outdated, so I agree that this should be updated.