Add cachedWritableStores to shared lib, and update hideZeroBalanceVaults to use the cached version #1698
Conversation
WalkthroughA new utility function, Changes
Sequence Diagram(s)sequenceDiagram
participant App as Svelte App
participant Store as cachedWritableStore
participant localStorage
App->>Store: Initialize cachedWritableStore(key, default, serialize, deserialize)
Store->>localStorage: Get item by key
alt Item exists and parse succeeds
Store->>App: Provide parsed value
else Item missing or parse fails
Store->>App: Provide default value
end
App->>Store: Subscribe and update value
Store->>localStorage: Set or remove item on change
sequenceDiagram
participant Settings as settings.ts
participant Store as cachedWritableStore
participant localStorage
Settings->>Store: Create hideZeroBalanceVaults store (key, true, serialize, deserialize)
Store->>localStorage: Get 'settings.hideZeroBalanceVaults'
alt Valid JSON boolean
Store->>Settings: Provide boolean value
else Invalid/corrupted data
Store->>Settings: Provide default (true)
end
Settings->>Store: Update value
Store->>localStorage: Update or remove key
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
tauri-app/src/lib/storesGeneric/fetchableStore.ts (1)
50-52: 🧹 Nitpick (assertive)Consider updating to Number.parseInt for consistency
For consistency with modern JavaScript standards, consider updating this instance of parseInt to Number.parseInt as well, though it's not part of the current changes.
- (s) => parseInt(s), + (s) => Number.parseInt(s),🧰 Tools
🪛 Biome (1.9.4)
[error] 51-51: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
tauri-app/src/lib/storesGeneric/listStore.ts (1)
37-43: 🧹 Nitpick (assertive)Verify JSON serialization error handling
The current implementation of
cachedWritablePagesdoesn't handle JSON parsing errors which could occur if the stored data becomes corrupted. Consider adding error handling.const cachedWritablePages = <T>(key: string) => cachedWritableStore<AllPages<T>>( key, [], (value) => JSON.stringify(value), - (value) => JSON.parse(value), + (value) => { + try { + return JSON.parse(value); + } catch (e) { + console.warn(`Failed to parse JSON for ${key}:`, e); + return []; + } + }, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
packages/ui-components/src/lib/components/ListViewOrderbookFilters.svelte(1 hunks)packages/ui-components/src/lib/index.ts(1 hunks)packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts(1 hunks)packages/webapp/src/lib/index.ts(0 hunks)packages/webapp/src/lib/stores/settings.ts(1 hunks)packages/webapp/src/routes/vaults/+page.svelte(1 hunks)tauri-app/src/lib/stores/settings.ts(1 hunks)tauri-app/src/lib/storesGeneric/cachedWritableStore.ts(0 hunks)tauri-app/src/lib/storesGeneric/detailStore.ts(1 hunks)tauri-app/src/lib/storesGeneric/fetchableStore.ts(1 hunks)tauri-app/src/lib/storesGeneric/listStore.ts(1 hunks)tauri-app/src/lib/storesGeneric/settingStore.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/webapp/src/lib/index.ts
- tauri-app/src/lib/storesGeneric/cachedWritableStore.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (2)
packages/ui-components/src/lib/index.ts (6)
cachedWritableStore(121-121)cachedWritableString(122-122)cachedWritableInt(123-123)cachedWritableOptionalStore(124-124)cachedWritableIntOptional(125-125)cachedWritableStringOptional(126-126)crates/settings/src/scenario.rs (1)
key(221-221)
🪛 Biome (1.9.4)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts
[error] 42-42: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 63-63: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: Deploy-Preview
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (8)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
3-28: Well-implemented localStorage-backed Svelte store!This generic cached writable store implementation provides a solid foundation for persistent state management in Svelte applications. The function correctly handles initialization from localStorage, synchronizes updates, and manages key removal when values become undefined.
tauri-app/src/lib/storesGeneric/fetchableStore.ts (1)
2-2: LGTM: Import path updated correctlyThe import source has been successfully updated to use the shared component library.
tauri-app/src/lib/storesGeneric/listStore.ts (1)
6-6: LGTM: Import path updated correctlyThe import source has been successfully updated to use the shared component library.
tauri-app/src/lib/storesGeneric/settingStore.ts (1)
2-2: LGTM: Import path updated correctlyThe import source has been successfully updated to use the shared component library.
tauri-app/src/lib/storesGeneric/detailStore.ts (1)
2-2: Import updated to use shared library implementation.The import source for
cachedWritableStorehas been updated to use the centralized implementation from@rainlanguage/ui-componentsinstead of a local module path. This change aligns with the PR objective of moving cached stores to a shared library.tauri-app/src/lib/stores/settings.ts (1)
2-2: Import path updated for shared cached store utilities.The import source for
cachedWritableStoreandcachedWritableStringOptionalhas been updated to use the centralized implementation from@rainlanguage/ui-components. This aligns with the PR objective of centralizing cached store utilities in a shared library.packages/webapp/src/lib/stores/settings.ts (1)
1-14: Added new cached settings implementation for webapp.This new file adds a cached writable store for
hideZeroBalanceVaultsthat uses the shared implementation from@rainlanguage/ui-components. The implementation includes:
- Proper key naming convention (
'settings.hideZeroBalanceVaults')- Default value of
true- JSON serialization/deserialization with error handling
This implementation is consistent with how the same setting is implemented in the Tauri app.
packages/ui-components/src/lib/components/ListViewOrderbookFilters.svelte (1)
38-38: Improved UI logic for "My Items Only" checkbox displayThis change enhances the UI behavior by showing the "My Items Only" checkbox when either the accounts store exists with no entries OR when a wallet is connected. This makes the filter more accessible to users with connected wallets, improving usability while maintaining the contextual tooltip only when needed.
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts
Outdated
Show resolved
Hide resolved
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts
Outdated
Show resolved
Hide resolved
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
3-28: 🛠️ Refactor suggestionWell-structured implementation of localStorage-backed Svelte store.
This is a solid implementation of a writable store synchronized with localStorage. The function handles initialization from cache and updates localStorage when the store changes.
However, consider adding error handling for the deserialization process to handle potentially corrupted localStorage values:
export function cachedWritableStore<T>( key: string, defaultValue: T, serialize: (value: T) => string, deserialize: (serialized: string) => T ) { const getCache = () => { const cached = localStorage.getItem(key); - return cached !== null ? deserialize(cached) : defaultValue; + if (cached === null) return defaultValue; + try { + return deserialize(cached); + } catch (e) { + console.warn(`Failed to deserialize ${key} from localStorage:`, e); + return defaultValue; + } }; // rest of the function remains the sametauri-app/src/lib/storesGeneric/cachedWritableStore.ts (3)
42-42: Use Number.parseInt instead of global parseInt.For better consistency with modern JavaScript standards, it's recommended to use the Number namespace version.
- (v) => parseInt(v) + (v) => Number.parseInt(v)🧰 Tools
🪛 Biome (1.9.4)
[error] 42-42: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
63-63: Use Number.parseInt instead of global parseInt.For better consistency with modern JavaScript standards, it's recommended to use the Number namespace version.
- (v) => parseInt(v) + (v) => Number.parseInt(v)🧰 Tools
🪛 Biome (1.9.4)
[error] 63-63: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
51-56: Consider adding error handling for deserialization.While the optional store implementation looks good, consider adding try/catch blocks for deserialization to handle potentially corrupted localStorage values gracefully.
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 ? serialize(v) : ''), - (v) => (v ? deserialize(v) : undefined) + (v) => (v ? serialize(v) : ''), + (v) => { + if (!v) return undefined; + try { + return deserialize(v); + } catch (e) { + console.warn(`Failed to deserialize ${key} from localStorage:`, e); + return defaultValue; + } + } );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts(1 hunks)tauri-app/src/lib/stores/settings.ts(1 hunks)tauri-app/src/lib/storesGeneric/cachedWritableStore.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
tauri-app/src/lib/storesGeneric/cachedWritableStore.ts
[error] 42-42: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 63-63: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: Deploy-Preview
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: test
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (3)
tauri-app/src/lib/stores/settings.ts (2)
2-2: Import consolidation looks good.The consolidation of imports for
cachedWritableStoreandcachedWritableStringOptionalis a clean approach.
255-266: Well-implemented boolean store with proper error handling.The implementation of
hideZeroBalanceVaultsincludes proper error handling for JSON deserialization, defaulting totrueif parsing fails. The default value and fallback behavior are consistent.tauri-app/src/lib/storesGeneric/cachedWritableStore.ts (1)
4-28: Code formatting looks good.The reformatting and indentation adjustments improve readability while maintaining functionality.
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts
Outdated
Show resolved
Hide resolved
hardingjam
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tauri-app/src/lib/storesGeneric/fetchableStore.ts (1)
51-51: 🧹 Nitpick (assertive)Use Number.parseInt instead of global parseInt
For better consistency with modern JavaScript standards, it's recommended to use the Number namespace version.
- (s) => parseInt(s), + (s) => Number.parseInt(s),🧰 Tools
🪛 Biome (1.9.4)
[error] 51-51: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
packages/ui-components/src/lib/index.ts (1)
120-120: Good addition of the store utility to shared exportsExporting the cached store utility from the shared UI components package is a positive architectural change that follows the DRY principle. This makes it available for reuse across different parts of the application.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
packages/ui-components/src/lib/index.ts(1 hunks)packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts(1 hunks)packages/webapp/src/lib/stores/settings.ts(1 hunks)tauri-app/src/lib/storesGeneric/fetchableStore.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/webapp/src/lib/stores/settings.ts (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
cachedWritableStore(3-36)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
tauri-app/src/lib/storesGeneric/cachedWritableStore.ts (1)
cachedWritableStore(3-28)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: test
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: test
- GitHub Check: Deploy-Preview
🔇 Additional comments (3)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
3-36: Well-implemented localStorage-backed Svelte store with proper error handlingThis implementation of a cached writable store is robust and well-designed. You've properly handled potential localStorage failures with try-catch blocks and provided appropriate fallbacks to default values.
The generic typing makes this utility highly reusable across different data types while the clear separation of serialization/deserialization logic allows for flexible data storage strategies.
tauri-app/src/lib/storesGeneric/fetchableStore.ts (1)
2-2: Import path successfully updated to use the shared implementationGood job updating the import path to use the centralized implementation of
cachedWritableStorefrom the shared library.packages/webapp/src/lib/stores/settings.ts (1)
1-14: Well-implemented persistent user setting with proper error handlingThis store implementation correctly uses the shared
cachedWritableStoreutility to create a persistent boolean setting for hiding zero balance vaults. The JSON serialization/deserialization with error handling is appropriate for this use case, and the fallback to the default value ensures a consistent user experience even if localStorage data becomes corrupted.
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts(1 hunks)packages/webapp/src/lib/stores/settings.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
tauri-app/src/lib/storesGeneric/cachedWritableStore.ts (1)
cachedWritableStore(3-28)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: git-clean
- GitHub Check: test
🔇 Additional comments (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
3-37: Well-implemented utility for persistent Svelte stores.This implementation of
cachedWritableStoreprovides a robust way to create writable stores that persist their values in localStorage with proper error handling. The try-catch blocks around localStorage operations ensure the application will function even when localStorage is unavailable.
|
@hardingjam should we not be deleting cachedWritableStore from the tauri app now? |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/ui-components/src/lib/index.ts (1)
120-120: 🧹 Nitpick (assertive)Add JSDoc for
cachedWritableStore.
Please include JSDoc comments on the implementation to describe its parameters, serialization/deserialization behavior, and error handling. This will improve discoverability and help developers understand usage patterns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/ui-components/src/lib/index.ts(1 hunks)packages/webapp/src/routes/vaults/+page.svelte(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
🔇 Additional comments (3)
packages/ui-components/src/lib/index.ts (1)
120-120: Approve addition ofcachedWritableStoreexport.
Centralizing this utility in the UI components package promotes DRY, ensures consistent behavior across the webapp and Tauri apps, and enables shared persistence logic.packages/webapp/src/routes/vaults/+page.svelte (2)
7-7: ConfirmhideZeroBalanceVaultsimport path.
Importing the new persistent store from$lib/stores/settingsis correct, but please verify that thesettings.tsfile exports the updatedhideZeroBalanceVaults(backed bycachedWritableStore) and that there are no remaining stale imports of this store elsewhere.
22-22: VerifyactiveSubgraphsremains valid.
Since you movedhideZeroBalanceVaultsout of$page.data.stores, ensure thatactiveSubgraphsis still populated by the page loader and doesn't need to be migrated to a cached store.
True, i've removed it |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
90-101: 🛠️ Refactor suggestionConsider adding error handling for deserialization
The optional store implementation should handle potential deserialization errors to prevent runtime exceptions from corrupted localStorage 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) + (v) => { + if (v === '') return undefined; + try { + return deserialize(v); + } catch (e) { + console.warn(`Failed to deserialize ${key} from localStorage:`, e); + return defaultValue; + } + } );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (2)
packages/ui-components/src/lib/index.ts (4)
cachedWritableStore(121-121)cachedWritableString(124-124)cachedWritableIntOptional(122-122)cachedWritableStringOptional(123-123)crates/settings/src/scenario.rs (1)
key(221-221)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Deploy-Preview
- GitHub Check: test
- GitHub Check: git-clean
🔇 Additional comments (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
30-64: Well-implemented localStorage persistence with proper error handling!The implementation properly handles localStorage unavailability with try/catch blocks and includes helpful comments explaining the design decision to silently ignore errors. This is a solid, reusable utility for creating persistent Svelte stores.
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts
Outdated
Show resolved
Hide resolved
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
93-104: 🧹 Nitpick (assertive)Consider improving error handling in the optional store implementation.
While the implementation works for most cases, it could be more robust with additional error handling during deserialization.
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) + (v) => (v !== undefined ? serialize(v) : ''), + (v) => { + if (v === '') return undefined; + try { + return deserialize(v); + } catch (error) { + console.warn(`Failed to deserialize value for key "${key}":`, error); + return defaultValue; + } + } );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (1)
packages/ui-components/src/lib/index.ts (4)
cachedWritableStore(121-121)cachedWritableString(124-124)cachedWritableIntOptional(122-122)cachedWritableStringOptional(123-123)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
packages/ui-components/src/lib/storesGeneric/cachedWritableStore.ts (3)
1-64: Well-implemented localStorage-backed Svelte store with proper error handling.The implementation of the generic
cachedWritableStoreis robust and follows best practices:
- Comprehensive JSDoc documentation with clear examples
- Proper error handling for localStorage operations
- Graceful fallbacks when localStorage is unavailable
- Clean integration with Svelte's reactive system
66-82: Good implementation of specialized helpers for common types.The string and integer store implementations are correctly implemented with appropriate serialization/deserialization strategies. The integer store properly handles potential NaN values, which is an excellent defensive programming practice.
113-122: Good NaN handling in the optional integer store.The implementation correctly handles the edge case of parsing invalid numeric strings, using the nullish coalescing operator to provide an appropriate fallback value.
|
@hardingjam this doesn't work for me on the preview url - when i refresh i lose the selection Screen.Recording.2025-05-01.at.07.20.00.mov |
this PR only introduces the store and adds the "hide empty vaults" setting. The other settings stores are added in #1700 |
Motivation
Move the cachedWritableStores to the shared lib, so that filter choices can be saved between refreshes
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Bug Fixes
Chores