Favor service worker over localstorage #906
Conversation
Local storage is a synchronous API and therefore can really slow down the performance of the website, especially when we load in a bunch of super large requests like we do for NIBRS data.
For browsers that support it, use a Service Worker to speed up HTTP requests and allow offline access. Always cache a new version of the app.css and bundle.js by adding a cache busting value to the requested URL
Take advantage of the service worker to cache the URLs that users go to as the action is updated
jeremiak
added
workflow: in-progress
workflow: review
and removed
workflow: in-progress
labels
| return keys.map(k => `${k}=${obj[k]}`).join('&') | ||
| } | ||
|
|
||
| export default (url, p = {}) => { |
There was a problem hiding this comment.
why is this no longer needed?
There was a problem hiding this comment.
We were using this to build predictable keys for the localStorage cache. However, the cache object is pretty much tailor made for storing HTTP responses and assets so we just need to pass it the request object and it handles the key name.
I didn't see anywhere else we were using this code besides localStorage key name generation.
| @@ -25,7 +25,13 @@ export const updateApp = (change, router) => dispatch => { | |||
| dispatch(updateFilters(change)) | |||
|
|
|||
| if (router) { | |||
| history.push(createNewLocation({ change, router })) | |||
| const loc = createNewLocation({ change, router }) | |||
| if (window.caches) { | |||
There was a problem hiding this comment.
can you explain this? what if loc.pathname is already in the cache - will it override or append the old one?
There was a problem hiding this comment.
Good question, I think it would replace the old one with the new one from this function.
The point of this bit is so as a user changes the URL and goes across the site, those visited URLs are also cached to enable faster (and offline) access.
There was a problem hiding this comment.
ok, let's check on this once merged
| const VERSION = 1 | ||
| const CACHE = 'crime-data-explorer' | ||
|
|
||
| self.addEventListener('install', event => { |
There was a problem hiding this comment.
any advantage to using self instead of window? have you seen this? https://stackoverflow.com/a/3217907
There was a problem hiding this comment.
For service workers, I believe you have to use self since it is in a different thread that doesn't have a window
| @@ -0,0 +1,59 @@ | |||
| /* eslint-disable no-unused-vars */ | |||
| const VERSION = 1 | |||
There was a problem hiding this comment.
Ok, so it looks weird but basically the way a service worker operates is that it checks for the byte sum of the file and if it has changed, determines that it is a new service worker to be installed. Having the version defined (though not used) enables us to easily change the byte sum of the file and therefore trigger an update.
Lots of people use the service worker version in the cache name, but because we also want to access the cache from the app (which is basically a separate code base since this is in /public) I didn't want to have a cache mismatch because we forgot to update the version number in two places.
Happy to change, not sure what the best strategy is here though.
There was a problem hiding this comment.
got it, makes sense, lgtm
|
let's do it :) |
LocalStorage is nice for small amounts of data but we were using it as an API response cache which was taxing, could result in errors, and was slow as the localStorage APIs are synchronous. You could visibly see the effect of this sometimes as the loading spinners would stop as lots of data was being inserted into storage.
Service Worker is a better, async API that enables us to provide a faster online experience and a more compelling offline one. Service Worker support isn't across our entire browser support matrix so this will only help folks on browsers that support it (https://jakearchibald.github.io/isserviceworkerready/).
At the same time, I've completely removed localStorage since it could actually introduce a bug when too much NIBRS data was attempted to be stored there.