1930 local storage version control
#1984
Conversation
| export const MASTER_CACHE_VERSION = 1; | ||
| export const FAVORITES_CACHE_VERSION = 1; | ||
| export const PROPOSAL_CACHE_VERSION = 1; | ||
| export const AVERAGE_BLOCK_TIME_CACHE_VERSION = 1; | ||
|
|
||
| export const getCacheVersion = (cacheName: CacheKeys): number => { | ||
| switch (cacheName) { | ||
| case CacheKeys.FAVORITES: | ||
| return FAVORITES_CACHE_VERSION; | ||
| case CacheKeys.MASTER_COPY: | ||
| return MASTER_CACHE_VERSION; | ||
| case CacheKeys.PROPOSAL_ARCHIVE: | ||
| return PROPOSAL_CACHE_VERSION; | ||
| case CacheKeys.AVERAGE_BLOCK_TIME: | ||
| return AVERAGE_BLOCK_TIME_CACHE_VERSION; | ||
| default: | ||
| throw new Error('Invalid cache name'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Is this kinda what you are thinking? @adamgall
✅ Deploy Preview for decent-interface-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
|
||
| /** | ||
| * Cache default values. | ||
| * | ||
| * Cache keys are not required to have a default value. | ||
| * @todo: The CACHE_DEFAULTs seem to be used for indexDB, But favorites is localstorage. We will need to revisit this. |
There was a problem hiding this comment.
I'd probably suggest migrating from indexedDB to localStorage - our cache is not that big and indexedDB was rather over-engineering. But yeah - not a topic for discussion in this PRs series
There was a problem hiding this comment.
Yeah 100% Another series can take a look at cleaning up indexDB in the future
There was a problem hiding this comment.
Seconded. Uh, thirded?
| default: | ||
| throw new Error('Invalid cache name'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Could you combine above into sth like a constant mapCacheNameToVersion?
There was a problem hiding this comment.
@DarksightKellar I don't follow what you're suggesting here...
There was a problem hiding this comment.
@DarksightKellar You mean like this? (Taking a guess)
const CACHE_VERSIONS: { [key: string]: number } = {
[CacheKeys.FAVORITES]: 1,
[CacheKeys.MASTER_COPY]: 1,
[CacheKeys.PROPOSAL_CACHE]: 1,
[CacheKeys.AVERAGE_BLOCK_TIME]: 1,
};
export const getCacheVersion = (cacheName: CacheKeys): number => {
const version = CACHE_VERSIONS[cacheName];
if (version === undefined) {
throw new Error('Invalid cache name');
}
return version;
};(In latest commit)
There was a problem hiding this comment.
I was thinking more like:
export const CACHE_VERSIONS = {
[CacheKeys.FAVORITES]: 1,
[CacheKeys.MASTER_COPY]: 1,
[CacheKeys.PROPOSAL_CACHE]: 1,
[CacheKeys.AVERAGE_BLOCK_TIME]: 1,
};
// elsewhere:
const favsCacheVersion = CACHE_VERSIONS[CacheKeys.FAVORITES]
So we can get rid of getCacheVersion and not have to do dynamic error checking, as inferred typing will prevent CACHE_VERSIONS['invalidKey'] in the first place.
There was a problem hiding this comment.
@Da-Colon @adamgall Ouuu and, apparently Object.freeze:
export const CACHE_VERSIONS = Object.freeze({
[CacheKeys.FAVORITES]: 1,
[CacheKeys.MASTER_COPY]: 1,
[CacheKeys.PROPOSAL_CACHE]: 1,
[CacheKeys.AVERAGE_BLOCK_TIME]: 1,
})
makes CACHE_VERSIONS[CacheKeys. FAVORITES] = 0; impossible -- added bonus. Learnt that from chatGPT.
There was a problem hiding this comment.
I was sooo close ha. Coolio updated and pushed sir.
|
|
||
| /** | ||
| * Cache default values. | ||
| * | ||
| * Cache keys are not required to have a default value. | ||
| * @todo: The CACHE_DEFAULTs seem to be used for indexDB, But favorites is localstorage. We will need to revisit this. |
There was a problem hiding this comment.
Seconded. Uh, thirded?
I was going keeping this around for the next PR's migration. But I don't think I really need it since I'm only going to migrate the Favorites. I'll remove this in the next PR. |
Noticed that I missed the Average Block Time so added that here.