Skip to content
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.

Sign up for GitHub

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

Jump to bottom

1930 local storage version control #1984

Merged
merged 6 commits into from
Jun 12, 2024
Merged

1930 local storage version control #1984

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1a17869
Add Average Block time
Da-Colon Jun 8, 2024
d854b25
scope cache version
Da-Colon Jun 8, 2024
bb2b325
rename
Da-Colon Jun 10, 2024
7d39617
remove export
Da-Colon Jun 10, 2024
24dd0b2
Map to const
Da-Colon Jun 12, 2024
3ae6699
remove function
Da-Colon Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/hooks/utils/cache/cacheDefaults.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,16 @@ export interface ProposalCacheKey extends CacheKey {
contractAddress: Address;
}

export interface AverageBlockTimeCacheKey extends CacheKey {
cacheName: CacheKeys.AVERAGE_BLOCK_TIME;
chainId: number;
}

export type CacheKeyType =
| FavoritesCacheKey
| MasterCacheKey
| ProposalCacheKey
| AverageBlockTimeCacheKey
| Omit<CacheKey, 'version'>;

export type CacheValue = {
Expand All @@ -96,12 +102,31 @@ interface IndexedObject {
[key: string]: any;
}

export const CURRENT_CACHE_VERSION = 1;
export const MASTER_CACHE_VERSION = 1;
Da-Colon marked this conversation as resolved.
Show resolved Hide resolved
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;
Da-Colon marked this conversation as resolved.
Show resolved Hide resolved
case CacheKeys.PROPOSAL_ARCHIVE:
Da-Colon marked this conversation as resolved.
Show resolved Hide resolved
return PROPOSAL_CACHE_VERSION;
case CacheKeys.AVERAGE_BLOCK_TIME:
return AVERAGE_BLOCK_TIME_CACHE_VERSION;
default:
throw new Error('Invalid cache name');
}
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this kinda what you are thinking? @adamgall

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you combine above into sth like a constant mapCacheNameToVersion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DarksightKellar I don't follow what you're suggesting here...

Copy link
Contributor Author

@Da-Colon Da-Colon Jun 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah 100% Another series can take a look at cleaning up indexDB in the future

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seconded. Uh, thirded?

*/
export const CACHE_DEFAULTS: IndexedObject = {
[CacheKeys.FAVORITES.toString()]: Array<string>(),
Expand Down
10 changes: 4 additions & 6 deletions src/hooks/utils/cache/useLocalStorage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { parseISO } from 'date-fns';
import {
CacheExpiry,
CacheKeyType,
CURRENT_CACHE_VERSION,
CacheValue,
CacheValueType,
getCacheVersion,
} from './cacheDefaults';

function bigintReplacer(_: any, value: any) {
Expand Down Expand Up @@ -39,17 +39,15 @@ export const setValue = (
: Date.now() + expirationMinutes * 60000,
};
localStorage.setItem(
JSON.stringify({ ...key, version: CURRENT_CACHE_VERSION }),
JSON.stringify({ ...key, version: getCacheVersion(key.cacheName) }),
JSON.stringify(val, bigintReplacer),
);
}
};

export const getValue = <T extends CacheKeyType>(
key: T,
version = CURRENT_CACHE_VERSION,
): CacheValueType<T> | null => {
export const getValue = <T extends CacheKeyType>(key: T): CacheValueType<T> | null => {
if (typeof window !== 'undefined') {
const version = getCacheVersion(key.cacheName);
const rawVal = localStorage.getItem(JSON.stringify({ ...key, version }));
if (rawVal) {
const parsed: CacheValue = JSON.parse(rawVal, proposalObjectReviver);
Expand Down
11 changes: 9 additions & 2 deletions src/utils/contract.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,21 @@ import { setValue, getValue } from '../hooks/utils/cache/useLocalStorage';
import { Providers } from '../types/network';

export const getAverageBlockTime = async (provider: Providers) => {
let averageBlockTime = getValue({ cacheName: CacheKeys.AVERAGE_BLOCK_TIME });
let averageBlockTime = getValue({
cacheName: CacheKeys.AVERAGE_BLOCK_TIME,
chainId: provider.network.chainId,
});
if (averageBlockTime) {
return averageBlockTime;
}
const latestBlock = await provider.getBlock('latest');
const pastBlock = await provider.getBlock(latestBlock.number - 1000);
averageBlockTime = (latestBlock.timestamp - pastBlock.timestamp) / 1000;
setValue({ cacheName: CacheKeys.AVERAGE_BLOCK_TIME }, averageBlockTime, CacheExpiry.ONE_DAY);
setValue(
{ cacheName: CacheKeys.AVERAGE_BLOCK_TIME, chainId: provider.network.chainId },
averageBlockTime,
CacheExpiry.ONE_DAY,
);
return averageBlockTime;
};

Expand Down