-
Notifications
You must be signed in to change notification settings - Fork 6
Add cached settings for listViewOrderbookFilters #1700
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
base: main
Are you sure you want to change the base?
Changes from all commits
45ac91b
ada479e
2422c7b
157a963
acf5f5c
887a313
c0bbe57
b4201c6
f598c2a
a4d0f39
fce4eed
a98fae7
10a3e67
98dc799
54def87
f0f6e4e
b1fca38
040c643
b30e576
fb0fd25
88d622d
5101234
b3795f3
0519341
fbb9fe0
4438b2a
bb464c7
33cbf62
587eb54
aa225b2
144fdb0
6b44948
e2507ba
afcbd4b
6704f25
0926719
e28e3f1
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 |
---|---|---|
@@ -0,0 +1,137 @@ | ||
import { writable } from 'svelte/store'; | ||
|
||
/** | ||
* Creates a writable Svelte store that persists its value to localStorage. | ||
* | ||
* @template T - The type of the value stored in the store | ||
* @param {string} key - The localStorage key used to store the value | ||
* @param {T} defaultValue - The default value to use when no value is found in localStorage | ||
* @param {function(T): string} serialize - Function to convert the store value to a string for storage | ||
* @param {function(string): T} deserialize - Function to convert the stored string back to the original type | ||
* @returns {import('svelte/store').Writable<T>} A writable store that automatically syncs with localStorage | ||
* | ||
* @example | ||
* // Create a store for a boolean value | ||
* const darkMode = cachedWritableStore( | ||
* 'darkMode', | ||
* false, | ||
* value => JSON.stringify(value), | ||
* str => JSON.parse(str) | ||
* ); | ||
* | ||
* // Create a store for a complex object | ||
* const userPreferences = cachedWritableStore( | ||
* 'userPrefs', | ||
* { theme: 'light', fontSize: 14 }, | ||
* value => JSON.stringify(value), | ||
* str => JSON.parse(str) | ||
* ); | ||
*/ | ||
export function cachedWritableStore<T>( | ||
key: string, | ||
defaultValue: T, | ||
serialize: (value: T) => string, | ||
deserialize: (serialized: string) => T | ||
) { | ||
const getCache = () => { | ||
try { | ||
const cached = localStorage.getItem(key); | ||
return cached !== null ? deserialize(cached) : defaultValue; | ||
} catch { | ||
return defaultValue; | ||
} | ||
}; | ||
const setCache = (value?: T) => { | ||
try { | ||
if (value !== undefined) { | ||
localStorage.setItem(key, serialize(value)); | ||
} else { | ||
localStorage.removeItem(key); | ||
} | ||
} catch { | ||
// Silently ignore localStorage errors to allow the application to function | ||
// without persistence in environments where localStorage is unavailable | ||
} | ||
}; | ||
|
||
const data = writable<T>(getCache()); | ||
|
||
data.subscribe((value) => { | ||
setCache(value); | ||
}); | ||
|
||
return data; | ||
} | ||
|
||
export const cachedWritableString = (key: string, defaultValue = '') => | ||
cachedWritableStore<string>( | ||
key, | ||
defaultValue, | ||
(v) => v, | ||
(v) => v | ||
); | ||
export const cachedWritableInt = (key: string, defaultValue = 0) => | ||
cachedWritableStore<number>( | ||
key, | ||
defaultValue, | ||
(v) => v.toString(), | ||
(v) => { | ||
const parsed = Number.parseInt(v); | ||
return isNaN(parsed) ? defaultValue : parsed; | ||
} | ||
); | ||
/** | ||
* Creates a writable store that can hold an optional value of type T and persists to localStorage. | ||
* | ||
* @template T - The type of the value stored | ||
* @param {string} key - The localStorage key to use for persistence | ||
* @param {T | undefined} defaultValue - The default value if nothing is found in localStorage | ||
* @param {function} serialize - Function to convert the value to a string for storage | ||
* @param {function} deserialize - Function to convert the stored string back to a value | ||
* @returns A writable store that persists to localStorage and can hold undefined values | ||
*/ | ||
export const cachedWritableOptionalStore = <T>( | ||
key: string, | ||
defaultValue: T | undefined = undefined, | ||
serialize: (value: T) => string, | ||
deserialize: (serialized: string) => T | ||
) => | ||
cachedWritableStore<T | undefined>( | ||
key, | ||
defaultValue, | ||
(v) => (v !== undefined ? serialize(v) : ''), | ||
(v) => (v !== '' ? deserialize(v) : undefined) | ||
); | ||
|
||
/** | ||
* Creates a writable store that can hold an optional number value and persists to localStorage. | ||
* | ||
* @param {string} key - The localStorage key to use for persistence | ||
* @param {number | undefined} defaultValue - The default value if nothing is found in localStorage | ||
* @returns A writable store that persists to localStorage and can hold an optional number | ||
*/ | ||
export const cachedWritableIntOptional = (key: string, defaultValue = undefined) => | ||
cachedWritableOptionalStore<number>( | ||
key, | ||
defaultValue, | ||
(v) => v.toString(), | ||
(v) => { | ||
const parsed = Number.parseInt(v); | ||
return isNaN(parsed) ? (defaultValue ?? 0) : parsed; | ||
} | ||
); | ||
|
||
/** | ||
* Creates a writable store that can hold an optional string value and persists to localStorage. | ||
* | ||
* @param {string} key - The localStorage key to use for persistence | ||
* @param {string | undefined} defaultValue - The default value if nothing is found in localStorage | ||
* @returns A writable store that persists to localStorage and can hold an optional string | ||
*/ | ||
export const cachedWritableStringOptional = (key: string, defaultValue = undefined) => | ||
cachedWritableOptionalStore<string>( | ||
key, | ||
defaultValue, | ||
(v) => v, | ||
(v) => v | ||
); |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
import { cachedWritableStore, type ConfigSource } from '@rainlanguage/ui-components'; | ||
|
||
/** | ||
* A persistent store that holds the application configuration settings. | ||
* | ||
* This store is saved to local storage and persists between sessions. | ||
* | ||
* @default undefined - No configuration is set by default | ||
* @returns A writable store containing the application configuration | ||
*/ | ||
export const settings = cachedWritableStore<ConfigSource | undefined>( | ||
'settings', | ||
undefined, | ||
(value) => JSON.stringify(value), | ||
(str) => { | ||
try { | ||
return JSON.parse(str) as ConfigSource; | ||
} catch { | ||
return undefined; | ||
} | ||
} | ||
); | ||
|
||
/** | ||
* A persistent store that controls whether vaults with zero balance should be hidden in the UI. | ||
* | ||
* This setting is saved to local storage and persists between sessions. | ||
* | ||
* @default true - Zero balance vaults are hidden by default | ||
* @returns A writable store containing a boolean value | ||
*/ | ||
export const hideZeroBalanceVaults = cachedWritableStore<boolean>( | ||
'settings.hideZeroBalanceVaults', | ||
true, // default value is true | ||
(value) => JSON.stringify(value), | ||
(str) => { | ||
try { | ||
const value = JSON.parse(str); | ||
return typeof value === 'boolean' ? value : true; | ||
} catch { | ||
return true; | ||
} | ||
} | ||
); | ||
|
||
/** | ||
* A persistent store that controls whether to show only the user's items in lists. | ||
* | ||
* This setting is saved to local storage and persists between sessions. | ||
* | ||
* @default false - All items are shown by default | ||
* @returns A writable store containing a boolean value | ||
*/ | ||
export const showMyItemsOnly = cachedWritableStore<boolean>( | ||
'settings.showMyItemsOnly', | ||
false, | ||
(value) => JSON.stringify(value), | ||
(str) => { | ||
try { | ||
const value = JSON.parse(str); | ||
return typeof value === 'boolean' ? value : false; | ||
} catch { | ||
return false; | ||
} | ||
} | ||
); | ||
|
||
/** | ||
* A persistent store that holds active subgraph URLs for different networks/orderbooks. | ||
* | ||
* This store maps network/orderbook identifiers to their corresponding subgraph URLs. | ||
* The setting is saved to local storage and persists between sessions. | ||
* | ||
* @default {} - Empty object by default | ||
* @returns A writable store containing a record of subgraph URLs | ||
*/ | ||
export const activeSubgraphs = cachedWritableStore<Record<string, string>>( | ||
'settings.activeSubgraphs', | ||
{}, | ||
JSON.stringify, | ||
(s) => { | ||
try { | ||
return JSON.parse(s); | ||
} catch { | ||
return {}; | ||
} | ||
} | ||
); | ||
|
||
/** | ||
* A persistent store that holds the currently selected order hash. | ||
* | ||
* This setting is saved to local storage and persists between sessions. | ||
* | ||
* @default "" - Empty string by default | ||
* @returns A writable store containing the order hash string | ||
*/ | ||
export const orderHash = cachedWritableStore<string>( | ||
'settings.orderHash', | ||
'', | ||
(value) => value, | ||
(str) => str || '' | ||
); | ||
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -14,6 +14,10 @@ | |||||||||||||||||||
import LoadingWrapper from '$lib/components/LoadingWrapper.svelte'; | ||||||||||||||||||||
import { WalletProvider } from '@rainlanguage/ui-components'; | ||||||||||||||||||||
import { signerAddress } from '$lib/stores/wagmi'; | ||||||||||||||||||||
import { settings as cachedSettings } from '$lib/stores/settings'; | ||||||||||||||||||||
|
||||||||||||||||||||
const { settings } = $page.data.stores; | ||||||||||||||||||||
cachedSettings.set(settings); | ||||||||||||||||||||
Comment on lines
+17
to
+20
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. 🧹 Nitpick (assertive) Consider making the settings synchronization reactive. The current implementation synchronizes settings only once on component initialization. For consistency, you may want to make this synchronization reactive to handle subsequent changes to page data. -const { settings } = $page.data.stores;
-cachedSettings.set(settings);
+$: if ($page.data.stores?.settings) {
+ cachedSettings.set($page.data.stores.settings);
+} 📝 Committable suggestion
Suggested change
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. it's a hard-coded value that's fetched from strategies repo, so it won't change 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. @hardingjam Thanks for implementing the reactive synchronization! This improvement ensures that any subsequent changes to page data will be properly reflected in the cached settings. Great work! If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
||||||||||||||||||||
|
||||||||||||||||||||
// Query client for caching | ||||||||||||||||||||
const queryClient = new QueryClient({ | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,37 @@ | ||||||||||||||||||
import type { ConfigSource } from '@rainlanguage/orderbook'; | ||||||||||||||||||
|
||||||||||||||||||
export const mockConfigSource: ConfigSource = { | ||||||||||||||||||
networks: { | ||||||||||||||||||
mainnet: { | ||||||||||||||||||
rpc: 'https://mainnet.infura.io/v3/YOUR-PROJECT-ID', | ||||||||||||||||||
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. 🧹 Nitpick (assertive) Consider using a clearer mock URL format While this works as mock data, using a URL with a placeholder PROJECT-ID might be confusing in tests. - rpc: 'https://mainnet.infura.io/v3/YOUR-PROJECT-ID',
+ rpc: 'https://mock.example.com/mainnet', 📝 Committable suggestion
Suggested change
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. this mock is fine 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. @hardingjam Understood! I'll respect your preference for the mock URL format. ✏️ Learnings added
|
||||||||||||||||||
'chain-id': 1, | ||||||||||||||||||
label: 'Ethereum Mainnet', | ||||||||||||||||||
currency: 'ETH', | ||||||||||||||||||
}, | ||||||||||||||||||
}, | ||||||||||||||||||
subgraphs: { | ||||||||||||||||||
mainnet: 'https://api.thegraph.com/subgraphs/name/mainnet', | ||||||||||||||||||
}, | ||||||||||||||||||
orderbooks: { | ||||||||||||||||||
orderbook1: { | ||||||||||||||||||
address: '0xOrderbookAddress1', | ||||||||||||||||||
network: 'mainnet', | ||||||||||||||||||
subgraph: 'uniswap', | ||||||||||||||||||
label: 'Orderbook 1', | ||||||||||||||||||
Comment on lines
+17
to
+20
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. Fix inconsistency in subgraph reference The orderbook references a 'uniswap' subgraph on line 19, but only 'mainnet' is defined in the subgraphs object (line 13). This could cause issues in tests that attempt to access this subgraph. - subgraph: 'uniswap',
+ subgraph: 'mainnet', 📝 Committable suggestion
Suggested change
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. this mock is fine 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. @hardingjam I understand, thanks for the clarification. I'll accept the mock configuration as is. ✏️ Learnings added
|
||||||||||||||||||
}, | ||||||||||||||||||
}, | ||||||||||||||||||
deployers: { | ||||||||||||||||||
deployer1: { | ||||||||||||||||||
address: '0xDeployerAddress1', | ||||||||||||||||||
network: 'mainnet', | ||||||||||||||||||
label: 'Deployer 1', | ||||||||||||||||||
}, | ||||||||||||||||||
}, | ||||||||||||||||||
metaboards: { | ||||||||||||||||||
metaboard1: 'https://example.com/metaboard1', | ||||||||||||||||||
}, | ||||||||||||||||||
accounts: { | ||||||||||||||||||
name_one: 'address_one', | ||||||||||||||||||
name_two: 'address_two', | ||||||||||||||||||
}, | ||||||||||||||||||
}; |
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.
🧹 Nitpick (assertive)
Consider adding type validation to the orderHash deserializer
The implementation for the orderHash store looks good, but consider adding type validation in the deserializer similar to other stores.
📝 Committable suggestion
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.
done
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.
@hardingjam Thanks for implementing the type validation! This small change will improve the code's robustness by ensuring consistent handling of unexpected data types from localStorage. Good job on maintaining type safety across your stores.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!