feat(PE-8241): Persist settings in local storage #799
Conversation
WalkthroughThis change introduces a globally managed, user-configurable "Hyperbeam URL" network setting. It propagates the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant GlobalState
participant LocalStorage
participant NetworkSettings
participant Actions
User->>NetworkSettings: Edits Hyperbeam URL
NetworkSettings->>GlobalState: Dispatch setHyperbeamUrl
GlobalState->>App: Updates global state
App->>useSyncSettings: Triggers on state change
useSyncSettings->>LocalStorage: Save updated settings (including hyperbeamUrl)
User->>App: Triggers network action (e.g., domain update)
App->>Actions: Calls action with hyperbeamUrl from state
Actions->>Network: Uses hyperbeamUrl in request
Estimated code review effort4 (~90 minutes) Possibly related PRs
Suggested reviewers
Poem
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
♻️ Duplicate comments (1)
src/components/pages/Register/Register.tsx (1)
130-132: LGTM: Consistent Hyperbeam URL configuration pattern.The conditional logic for setting
hyperbeamUrlis identical to the pattern used inEditUndernameModal.tsx, ensuring consistency across ANT contract initializations. The defensive approach of usingundefinedwhen Hyperbeam is disabled is appropriate.This follows the same pattern as noted in
EditUndernameModal.tsx- consider the utility function suggestion for reducing code duplication across multiple files.
🧹 Nitpick comments (3)
src/components/modals/ant-management/EditUndernameModal/EditUndernameModal.tsx (1)
53-55: LGTM: Proper conditional Hyperbeam URL configuration.The conditional logic correctly sets the
hyperbeamUrlparameter based on the Hyperbeam configuration flags. The defensive approach of settingundefinedwhen Hyperbeam is disabled is appropriate.Consider extracting the conditional Hyperbeam URL logic into a utility function since this pattern is repeated across multiple files:
// utils/network.ts export const getHyperbeamUrl = (aoNetwork: AoNetwork): string | undefined => { return aoNetwork.HYPERBEAM.ENABLED ? aoNetwork.HYPERBEAM.URL : undefined; };This would improve maintainability and reduce code duplication.
src/hooks/useSyncSettings/useSyncSettings.tsx (2)
74-74: Remove console.log statement from production code.Console logging should be removed from production code to avoid cluttering browser logs and potential information leakage.
- console.log('Settings saved to localStorage:', walletKey, settings);
78-78: Consider memoizing complex dependencies to prevent unnecessary re-renders.The
aoNetworkandturboNetworkobjects in the dependency array could cause unnecessary effect executions if they're recreated on each render. Consider usinguseMemooruseCallbackto stabilize these references.+ const memoizedSettings = useMemo(() => ({ + gateway, + aoNetwork, + turboNetwork, + arioProcessId, + }), [gateway, aoNetwork, turboNetwork, arioProcessId]); + useEffect(() => { if (!walletAddress) { return; } - }, [walletAddress, gateway, aoNetwork, turboNetwork, arioProcessId]); + }, [walletAddress, memoizedSettings]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
src/App.tsx(2 hunks)src/components/data-display/tables/UndernamesSubtable.tsx(2 hunks)src/components/data-display/tables/UndernamesTable.tsx(2 hunks)src/components/devtools/NetworkSettings.tsx(6 hunks)src/components/forms/DomainSettings/DomainSettings.tsx(10 hunks)src/components/inputs/text/NameTokenSelector/NameTokenSelector.tsx(3 hunks)src/components/modals/PrimaryNameModal/PrimaryNameModal.tsx(2 hunks)src/components/modals/ant-management/EditUndernameModal/EditUndernameModal.tsx(2 hunks)src/components/modals/ant-management/ReassignNameModal/ReassignNameModal.tsx(1 hunks)src/components/modals/ant-management/ReturnNameModal/ReturnNameModal.tsx(1 hunks)src/components/modals/ant-management/UpgradeDomainModal/UpgradeDomainModal.tsx(1 hunks)src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx(1 hunks)src/components/pages/Register/Checkout.tsx(1 hunks)src/components/pages/Register/Register.tsx(2 hunks)src/components/pages/Transaction/TransactionReview.tsx(1 hunks)src/hooks/index.ts(1 hunks)src/hooks/useDomainInfo.tsx(2 hunks)src/hooks/useSyncSettings/useSyncSettings.tsx(1 hunks)src/state/actions/dispatchANTInteraction.ts(4 hunks)src/state/actions/dispatchArIOInteraction.ts(4 hunks)src/state/contexts/GlobalState.tsx(2 hunks)src/utils/constants.ts(1 hunks)src/utils/network.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (12)
src/components/data-display/tables/UndernamesTable.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(130-131)
src/components/data-display/tables/UndernamesSubtable.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(130-131)
src/components/inputs/text/NameTokenSelector/NameTokenSelector.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(130-131)
src/components/pages/Register/Register.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(130-131)
src/hooks/useDomainInfo.tsx (1)
src/utils/network.ts (1)
buildAntStateQuery(49-78)
src/state/actions/dispatchANTInteraction.ts (1)
src/utils/constants.ts (1)
NETWORK_DEFAULTS(127-162)
src/components/devtools/NetworkSettings.tsx (2)
src/utils/constants.ts (1)
NETWORK_DEFAULTS(127-162)src/utils/searchUtils/searchUtils.ts (1)
isValidURL(196-198)
src/hooks/useSyncSettings/useSyncSettings.tsx (2)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(130-131)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)
src/components/modals/ant-management/EditUndernameModal/EditUndernameModal.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(130-131)
src/utils/network.ts (1)
src/utils/constants.ts (1)
NETWORK_DEFAULTS(127-162)
src/components/forms/DomainSettings/DomainSettings.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(130-131)
src/state/actions/dispatchArIOInteraction.ts (1)
src/utils/constants.ts (1)
NETWORK_DEFAULTS(127-162)
🔇 Additional comments (32)
src/utils/constants.ts (1)
129-132: LGTM! Well-structured network configuration addition.The new
HYPERBEAMconfiguration follows the existing pattern inNETWORK_DEFAULTSand uses conservative defaults withENABLED: false. The URL appears valid and the structure integrates cleanly with the existing network configurations.src/components/modals/ant-management/ReturnNameModal/ReturnNameModal.tsx (1)
63-63: Consistent parameter propagation.The addition of
aoNetworkto thedispatchANTInteractioncall properly propagates the network configuration from global state, maintaining consistency with the broader update pattern across the application.src/components/data-display/tables/UndernamesTable.tsx (2)
102-102: Proper global state destructuring update.Adding
aoNetworkto the destructured global state variables follows the consistent pattern for accessing network configuration.
156-156: Consistent parameter addition to ANT interaction.The
aoNetworkparameter is properly passed todispatchANTInteraction, maintaining consistency with the network configuration propagation pattern across the application.src/components/modals/ant-management/ReassignNameModal/ReassignNameModal.tsx (1)
139-139: Completes the network configuration propagation.Adding
aoNetworkto thedispatchANTInteractioncall properly utilizes the network configuration that was already being destructured from global state, maintaining consistency with the application-wide pattern.src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx (1)
138-138: Systematic network configuration integration.The addition of
aoNetworkto thedispatchANTInteractioncall follows the consistent pattern established across all ANT interaction components, ensuring the Hyperbeam network configuration is properly propagated throughout the upgrade workflow.src/hooks/index.ts (1)
5-5: LGTM! Export follows established pattern.The addition of the
useSyncSettingsexport is consistent with the existing hook export pattern in this index file.src/components/modals/ant-management/UpgradeDomainModal/UpgradeDomainModal.tsx (1)
108-108: LGTM! Network configuration properly propagated.The addition of
aoNetworkparameter todispatchANTInteractionis consistent with the broader pattern of enabling dynamic network configuration for ANT operations.src/App.tsx (2)
21-21: LGTM! Proper import of settings synchronization hook.The import follows the established pattern and correctly references the new
useSyncSettingshook.
75-75: LGTM! Appropriate placement for settings synchronization.Invoking
useSyncSettingsat the App component level alongsideuseWanderEventsensures settings are synchronized with localStorage throughout the application lifecycle.src/components/data-display/tables/UndernamesSubtable.tsx (2)
59-59: LGTM! Proper state destructuring for network configuration.Adding
aoNetworkto the destructured global state is consistent with the pattern of enabling dynamic network configuration throughout the application.
298-298: LGTM! Network configuration properly passed to ANT interaction.The addition of
aoNetworkparameter todispatchANTInteractionmaintains consistency with the broader pattern of propagating network configuration to ANT operations.src/components/pages/Transaction/TransactionReview.tsx (1)
128-128: LGTM! Network configuration properly integrated for ArIO interactions.The addition of
aoNetworkparameter todispatchArIOInteractionextends the dynamic network configuration pattern to ArIO operations, maintaining consistency across the application.src/components/pages/Register/Checkout.tsx (2)
42-42: LGTM: Proper integration of aoNetwork from global state.The addition of
aoNetworkto the destructured variables fromuseGlobalState()is consistent with the broader pattern across the codebase for integrating Hyperbeam network configuration.
326-326: LGTM: Consistent propagation of network configuration.The addition of
aoNetworkparameter to thedispatchArIOInteractioncall ensures that the Hyperbeam network settings are properly propagated through ARIO interactions, aligning with the PR's objective to support dynamic network configurations.src/components/modals/ant-management/EditUndernameModal/EditUndernameModal.tsx (1)
35-35: LGTM: Consistent global state integration.The addition of
aoNetworkto the destructured variables fromuseGlobalState()maintains consistency with the network configuration integration pattern used throughout the application.src/components/pages/Register/Register.tsx (1)
48-50: LGTM: Consistent global state integration.The addition of
aoNetworkto the destructured variables maintains the consistent pattern of network configuration integration across the application.src/components/modals/PrimaryNameModal/PrimaryNameModal.tsx (2)
185-185: LGTM: Proper network configuration propagation for ArIO interactions.The addition of
aoNetworkparameter to thedispatchArIOInteractioncall ensures that Hyperbeam network settings are properly propagated through primary name request/change workflows.
203-203: LGTM: Consistent network configuration for ANT interactions.The addition of
aoNetworkparameter to thedispatchANTInteractioncall maintains consistency with the network configuration propagation pattern and ensures Hyperbeam settings are available for ANT-based primary name removal workflows.src/components/forms/DomainSettings/DomainSettings.tsx (2)
81-81: LGTM: Consistent global state integration.The addition of
aoNetworkto the destructured variables fromuseGlobalState()maintains the consistent pattern used throughout the application for network configuration integration.
301-301: LGTM: Comprehensive network configuration propagation across all domain operations.The systematic addition of
aoNetworkparameter to alldispatchANTInteractioncalls ensures that Hyperbeam network settings are properly propagated through every domain setting operation (nickname, target ID, ticker, controllers, owner, TTL, logo, description, keywords). This comprehensive coverage is essential for the Hyperbeam feature to work consistently across all ANT interactions in the domain settings interface.Also applies to: 365-365, 385-385, 414-414, 441-441, 464-464, 497-497, 519-519, 541-541
src/hooks/useDomainInfo.tsx (2)
102-111: LGTM: Correct Hyperbeam integration for ANT initialization.The conditional
hyperbeamUrllogic is properly implemented, checking the enabled flag before setting the URL. This follows the established pattern across the codebase.
114-114: Good: Passing aoNetwork to buildAntStateQuery ensures consistency.The
aoNetworkparameter is correctly passed to ensure the ANT state query uses the same network configuration.src/state/actions/dispatchArIOInteraction.ts (1)
230-239: LGTM: Consistent Hyperbeam integration in ANT initialization.The conditional
hyperbeamUrllogic is correctly implemented within the PRIMARY_NAME_REQUEST workflow, maintaining consistency with the broader Hyperbeam integration pattern.src/components/inputs/text/NameTokenSelector/NameTokenSelector.tsx (1)
116-124: LGTM: Consistent Hyperbeam integration across ANT initializations.Both ANT initialization calls have been correctly updated with the conditional
hyperbeamUrllogic, maintaining consistency with the broader codebase integration.Also applies to: 166-174
src/state/actions/dispatchANTInteraction.ts (1)
64-70: LGTM: Comprehensive Hyperbeam integration in ANT workflows.Both the main ANT initialization and the upgrade workflow's ANT validation have been correctly updated with conditional
hyperbeamUrllogic, ensuring consistent network configuration throughout all ANT interactions.Also applies to: 241-250
src/utils/network.ts (1)
49-78: ```shell
#!/bin/bashInspect the call in useDomainInfo.tsx to confirm aoNetwork is passed
rg -C 3 "fetchQuery(buildAntStateQuery" src/hooks/useDomainInfo.tsx
Inspect the call in useANT.tsx to confirm aoNetwork is passed
rg -C 3 "buildAntStateQuery" src/hooks/useANT/useANT.tsx
</details> <details> <summary>src/components/devtools/NetworkSettings.tsx (4)</summary> `61-118`: **LGTM! Consistent state management pattern.** The Hyperbeam state variables are properly initialized and synchronized with global state, following the same pattern as other network settings in the component. --- `222-238`: **LGTM! Well-structured configuration update function.** The `updateHyperbeamAoNetwork` function correctly merges the new Hyperbeam configuration with existing settings and follows the established error handling pattern. --- `90-97`: **LGTM! Consistent reset functionality.** The Hyperbeam reset logic follows the same pattern as other network settings and correctly uses the default values from NETWORK_DEFAULTS. --- `609-687`: **LGTM! Comprehensive and intuitive UI for Hyperbeam management.** The UI implementation provides clear visibility of current settings and intuitive controls for both the URL and enabled state. The validation and state update logic are correctly implemented following the established patterns. </details> <details> <summary>src/state/contexts/GlobalState.tsx (1)</summary> `77-124`: **LGTM! Clean integration of dynamic settings loading.** The initialization logic properly integrates the loaded settings while maintaining backward compatibility and type safety. The fallback to defaults ensures the application works correctly even when no settings are saved. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| function loadSettingsFromStorage(): { | ||
| gateway: string; | ||
| aoNetwork: typeof NETWORK_DEFAULTS.AO; | ||
| turboNetwork: typeof NETWORK_DEFAULTS.TURBO; | ||
| arioProcessId: string; | ||
| } | null { | ||
| try { | ||
| // Try to get the last used wallet address from localStorage | ||
| const walletType = localStorage.getItem('walletType'); | ||
|
|
||
| // Try multiple storage keys to find saved settings | ||
| const possibleKeys = [ | ||
| walletType ? `${SETTINGS_STORAGE_KEY}_${walletType}` : null, | ||
| `${SETTINGS_STORAGE_KEY}_default`, | ||
| `${SETTINGS_STORAGE_KEY}_last`, | ||
| ].filter(Boolean); | ||
|
|
||
| for (const storageKey of possibleKeys) { | ||
| const savedSettings = localStorage.getItem(storageKey!); | ||
|
|
||
| if (savedSettings) { | ||
| const settings = JSON.parse(savedSettings); | ||
| console.log( | ||
| 'Loading settings from localStorage:', | ||
| storageKey, | ||
| settings, | ||
| ); | ||
|
|
||
| return { | ||
| gateway: settings.network?.gateway || ARWEAVE_HOST, | ||
| aoNetwork: settings.network?.aoNetwork || NETWORK_DEFAULTS.AO, | ||
| turboNetwork: | ||
| settings.network?.turboNetwork || NETWORK_DEFAULTS.TURBO, | ||
| arioProcessId: settings.arns?.arioProcessId || ARIO_PROCESS_ID, | ||
| }; | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to load settings from localStorage:', error); | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding JSON validation for security and robustness.
The function implements good practices with multiple storage keys, error handling, and fallbacks. However, consider adding validation of the parsed JSON structure to prevent potential issues with corrupted or malicious data.
for (const storageKey of possibleKeys) {
const savedSettings = localStorage.getItem(storageKey!);
if (savedSettings) {
const settings = JSON.parse(savedSettings);
+
+ // Validate the structure of loaded settings
+ if (!settings || typeof settings !== 'object') {
+ console.warn('Invalid settings structure in localStorage:', storageKey);
+ continue;
+ }
+
console.log(
'Loading settings from localStorage:',
storageKey,
settings,
);Also consider removing or reducing the console.log for production builds to avoid log pollution.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function loadSettingsFromStorage(): { | |
| gateway: string; | |
| aoNetwork: typeof NETWORK_DEFAULTS.AO; | |
| turboNetwork: typeof NETWORK_DEFAULTS.TURBO; | |
| arioProcessId: string; | |
| } | null { | |
| try { | |
| // Try to get the last used wallet address from localStorage | |
| const walletType = localStorage.getItem('walletType'); | |
| // Try multiple storage keys to find saved settings | |
| const possibleKeys = [ | |
| walletType ? `${SETTINGS_STORAGE_KEY}_${walletType}` : null, | |
| `${SETTINGS_STORAGE_KEY}_default`, | |
| `${SETTINGS_STORAGE_KEY}_last`, | |
| ].filter(Boolean); | |
| for (const storageKey of possibleKeys) { | |
| const savedSettings = localStorage.getItem(storageKey!); | |
| if (savedSettings) { | |
| const settings = JSON.parse(savedSettings); | |
| console.log( | |
| 'Loading settings from localStorage:', | |
| storageKey, | |
| settings, | |
| ); | |
| return { | |
| gateway: settings.network?.gateway || ARWEAVE_HOST, | |
| aoNetwork: settings.network?.aoNetwork || NETWORK_DEFAULTS.AO, | |
| turboNetwork: | |
| settings.network?.turboNetwork || NETWORK_DEFAULTS.TURBO, | |
| arioProcessId: settings.arns?.arioProcessId || ARIO_PROCESS_ID, | |
| }; | |
| } | |
| } | |
| } catch (error) { | |
| console.error('Failed to load settings from localStorage:', error); | |
| } | |
| return null; | |
| } | |
| for (const storageKey of possibleKeys) { | |
| const savedSettings = localStorage.getItem(storageKey!); | |
| if (savedSettings) { | |
| const settings = JSON.parse(savedSettings); | |
| // Validate the structure of loaded settings | |
| if (!settings || typeof settings !== 'object') { | |
| console.warn('Invalid settings structure in localStorage:', storageKey); | |
| continue; | |
| } | |
| console.log( | |
| 'Loading settings from localStorage:', | |
| storageKey, | |
| settings, | |
| ); | |
| return { | |
| gateway: settings.network?.gateway || ARWEAVE_HOST, | |
| aoNetwork: settings.network?.aoNetwork || NETWORK_DEFAULTS.AO, | |
| turboNetwork: | |
| settings.network?.turboNetwork || NETWORK_DEFAULTS.TURBO, | |
| arioProcessId: settings.arns?.arioProcessId || ARIO_PROCESS_ID, | |
| }; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/state/contexts/GlobalState.tsx around lines 31 to 73, the function
loadSettingsFromStorage parses JSON from localStorage without validating its
structure, which can lead to issues if the data is corrupted or malicious. Add
explicit validation checks after parsing to ensure the expected properties and
types exist before using them. Also, remove or conditionally disable the
console.log statement for production builds to prevent log pollution.
|
Visit the preview URL for this PR (updated for commit 7aecf55): https://arns-portal--pr799-pe-8241-hyperbeam-se-tsfeuf32.web.app (expires Wed, 06 Aug 2025 16:46:02 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 1f59769442cb89cf2a92042a342e51fe94047b94 |
| const walletKey = `${SETTINGS_STORAGE_KEY}_${walletAddress}`; | ||
|
|
||
| // Also save to a "last" key for persistence across wallet switches | ||
| const lastKey = `${SETTINGS_STORAGE_KEY}_last`; |
There was a problem hiding this comment.
why do we need both? The single key for the wallet address should be sufficient
src/hooks/useDomainInfo.tsx
Outdated
| hyperbeamUrl: aoNetwork.HYPERBEAM.ENABLED | ||
| ? aoNetwork.HYPERBEAM.URL | ||
| : undefined, |
There was a problem hiding this comment.
I don't think we need both parameters. Use just the hyphem urls existence to mean it's enabled
| useEffect(() => { | ||
| setNewHyperbeamUrl(aoNetwork.HYPERBEAM.URL); | ||
| setValidHyperbeamUrl(true); | ||
| setHyperbeamEnabled(aoNetwork.HYPERBEAM.ENABLED); |
There was a problem hiding this comment.
See comment about only using the presence of a hyperbeam URL on whether or not hyperbeam should be used
atticusofsparta
force-pushed
the
PE-8241-hyperbeam-setting
branch
from
afe0c90 to
3e8cea3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/components/devtools/NetworkSettings.tsx(5 hunks)src/components/modals/PrimaryNameModal/PrimaryNameModal.tsx(1 hunks)src/components/pages/Register/Checkout.tsx(2 hunks)src/components/pages/Transaction/TransactionReview.tsx(2 hunks)src/hooks/useSyncSettings/useSyncSettings.tsx(1 hunks)src/state/actions/dispatchArIOInteraction.ts(4 hunks)src/state/contexts/GlobalState.tsx(2 hunks)src/state/reducers/GlobalReducer.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/pages/Transaction/TransactionReview.tsx
- src/components/devtools/NetworkSettings.tsx
- src/components/modals/PrimaryNameModal/PrimaryNameModal.tsx
- src/components/pages/Register/Checkout.tsx
- src/hooks/useSyncSettings/useSyncSettings.tsx
- src/state/contexts/GlobalState.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/state/actions/dispatchArIOInteraction.ts (1)
src/utils/constants.ts (1)
NETWORK_DEFAULTS(127-159)
🪛 ESLint
src/state/actions/dispatchArIOInteraction.ts
[error] 39-39: 'aoNetwork' is defined but never used.
(@typescript-eslint/no-unused-vars)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build_and_test / lint_test_build
- GitHub Check: lint_test_build
🔇 Additional comments (3)
src/state/reducers/GlobalReducer.ts (2)
47-50: LGTM! Action type definition follows established patterns.The new
setHyperbeamUrlaction type is properly defined with appropriate typing, maintaining consistency with existing action types in the union.
110-114: LGTM! Reducer implementation correctly handles the new action.The reducer case properly handles the
setHyperbeamUrlaction by spreading the existing state and updating thehyperbeamUrlproperty, following Redux best practices.src/state/actions/dispatchArIOInteraction.ts (1)
233-233: LGTM! Correct usage of hyperbeamUrl in ANT initialization.The
hyperbeamUrlparameter is properly passed toANT.init, enabling the hyperbeam URL configuration for the ANT process during primary name requests.
|
Please add HyperBEAM URL to the standard settings page, and make a ticket to consolidate dev settings into the settings panel so we do not have both. |
| return; | ||
| } | ||
|
|
||
| // Skip saving on initial load when hyperbeamUrl is undefined |
There was a problem hiding this comment.
can you explain why we should skip only if hyperbeam is undefined?
There was a problem hiding this comment.
This was during iteration, where I wasn't directly defaulting the settings in GlobalState and attempting to do it from the hook instead. Its now unnecessary and removed ca26d18
There was a problem hiding this comment.
(as for why it was there, there was an issue with overwriting the hyperbeam setting that was saved because the initial setting is undefined in the state where as you may have saved a setting to local storage, so it got overwritten before it could be saved, which was a bandaid solution and prompted the change to synchronously load the settings in the global state to avoid the problem completely)
src/state/contexts/GlobalState.tsx
Outdated
| const possibleKeys = [ | ||
| walletType ? `${SETTINGS_STORAGE_KEY}_${walletType}` : null, | ||
| `${SETTINGS_STORAGE_KEY}_default`, | ||
| `${SETTINGS_STORAGE_KEY}_last`, | ||
| ].filter(Boolean); |
There was a problem hiding this comment.
i think we should only store one key and only use the latest settings. defaults can be held as a constant within the app and do not need to be saved to local storage
There was a problem hiding this comment.
these were not removed/updated in the commit you provideed
Co-authored-by: Dylan Fiedler <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
src/components/pages/Settings/NetworkSettings.tsx(7 hunks)src/hooks/useSyncSettings/useSyncSettings.tsx(1 hunks)src/hooks/useTurboArNSClient.tsx(1 hunks)src/state/contexts/GlobalState.tsx(3 hunks)
🧬 Code Graph Analysis (1)
src/components/pages/Settings/NetworkSettings.tsx (4)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(151-152)src/utils/constants.ts (1)
NETWORK_DEFAULTS(127-161)src/state/actions/dispatchNewGateway.ts (1)
dispatchNewGateway(10-41)src/utils/searchUtils/searchUtils.ts (1)
isValidURL(196-198)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/useSyncSettings/useSyncSettings.tsx
- src/state/contexts/GlobalState.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/pages/Settings/NetworkSettings.tsx (4)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(151-152)src/utils/constants.ts (1)
NETWORK_DEFAULTS(127-161)src/state/actions/dispatchNewGateway.ts (1)
dispatchNewGateway(10-41)src/utils/searchUtils/searchUtils.ts (1)
isValidURL(196-198)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-to-firebase
🔇 Additional comments (7)
src/hooks/useTurboArNSClient.tsx (1)
18-22: LGTM! Explicit configuration improves clarity.The change from passing the entire
aoNetwork.ARIOobject to explicitly specifying individual properties (CU_URL,MU_URL,MODE) makes the configuration more explicit and clear. The hardcodedMODE: 'legacy'appears intentional for this Turbo client connection.This aligns well with the broader network configuration refactoring mentioned in the PR objectives.
src/components/pages/Settings/NetworkSettings.tsx (6)
27-36: LGTM! Proper state destructuring for new feature.The addition of
hyperbeamUrlto the destructured global state follows the established pattern and is consistent with the PR objectives to add Hyperbeam URL configuration support.
50-53: LGTM! Proper state initialization with fallback.The new state variables for Hyperbeam URL are correctly initialized with proper fallback handling (
hyperbeamUrl || '') and follow the established pattern used by other URL inputs in the component.
95-98: LGTM! Proper useEffect for state synchronization.The useEffect hook correctly synchronizes local Hyperbeam URL state with global state changes, following the same pattern used for other network settings in the component.
429-488: LGTM! Comprehensive Hyperbeam URL configuration UI.The new Hyperbeam URL section follows the established UI patterns perfectly:
- Consistent styling with other input sections
- Proper validation using
isValidURL- Clear visual feedback for disabled state
- Standard set/reset button functionality
- Proper event handling for input, clear, and enter actions
The implementation is well-structured and maintains consistency with the existing codebase patterns.
147-148: Verified ARIO Nesting ChangeAll existing code references to
aoNetwork.ARIO.*have been located and none rely on a different shape. Nesting the newconfigproperties andGATEWAY_URLunder theARIOkey maintains the expected structure for every consumer.
127-128: Gateway update without wallet check is safeI’ve verified that
dispatchNewGatewaydoesn’t reference the wallet—it only initializes Arweave with the provided gateway and updates the block height and provider in global state. Removing theif (wallet)guard here won’t introduce errors when no wallet is connected.
._dist
Outdated
| setValidTurboPaymentUrl(true); | ||
| updateTurboNetwork(NETWORK_DEFAULTS.TURBO); | ||
| // hyperbeam network | ||
| setNewHyperbeamUrl(NETWORK_DEFAULTS.AO.ARIO.HYPERBEAM_URL || ''); |
There was a problem hiding this comment.
nit: formatting seems off
There was a problem hiding this comment.
Nope thats how prettier wants it.
src/state/contexts/GlobalState.tsx
Outdated
| const possibleKeys = [ | ||
| walletType ? `${SETTINGS_STORAGE_KEY}_${walletType}` : null, | ||
| `${SETTINGS_STORAGE_KEY}_default`, | ||
| `${SETTINGS_STORAGE_KEY}_last`, | ||
| ].filter(Boolean); |
There was a problem hiding this comment.
these were not removed/updated in the commit you provideed
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/hooks/useSyncSettings/useSyncSettings.tsx (3)
6-31: Well-structured interfaces with good TypeScript typing.The interfaces provide clear contracts for the settings structure. However, I notice
hyperbeamUrlappears both at the top level (line 8) and within theaoNetworksub-objects asHYPERBEAM_URL(lines 14, 21). Consider documenting the distinction between these or consolidating if they serve the same purpose.
46-54: TheisInitialLoadref is unused.The ref is created and updated but not used in any conditional logic. Based on past review comments, this appears to be intentional after removing skipping logic. Consider removing this unused ref to clean up the code.
- const isInitialLoad = useRef(true);- isInitialLoad.current = false;
68-77: Consider reducing console verbosity for production.The success console.log (lines 70-74) might be too verbose for production environments. Consider using a more targeted logging approach or removing success logs entirely.
- console.log( - 'Settings saved to localStorage:', - SETTINGS_STORAGE_KEY, - settings, - ); + // Settings saved successfully
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/devtools/NetworkSettings.tsx(5 hunks)src/components/pages/Settings/NetworkSettings.tsx(7 hunks)src/hooks/useSyncSettings/useSyncSettings.tsx(1 hunks)src/state/contexts/GlobalState.tsx(3 hunks)
🧬 Code Graph Analysis (1)
src/hooks/useSyncSettings/useSyncSettings.tsx (2)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(138-139)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/devtools/NetworkSettings.tsx
- src/state/contexts/GlobalState.tsx
- src/components/pages/Settings/NetworkSettings.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/hooks/useSyncSettings/useSyncSettings.tsx (2)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(138-139)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build_and_test / lint_test_build
- GitHub Check: lint_test_build
🔇 Additional comments (3)
src/hooks/useSyncSettings/useSyncSettings.tsx (3)
1-2: LGTM! Clean and appropriate imports.The imports are well-organized and include only the necessary dependencies for the hook's functionality.
4-4: Good practice using a named constant for the storage key.This prevents magic strings and makes the key reusable across the codebase.
90-90: LGTM! Standard hook export pattern.The default export follows React hook conventions appropriately.
| hyperbeamUrl?: string; | ||
| } | null { | ||
| try { | ||
| const savedSettings = localStorage.getItem(SETTINGS_STORAGE_KEY); |
There was a problem hiding this comment.
i think tanstack query supports local storage requests, any thoughts on using that here?
There was a problem hiding this comment.
I'm not sure I understand what benefit you are interested in here. Do you mean like using local storage for the react query cache, or using react query for interacting with local storage?
If it's the latter, I would say it's not beneficial, and even unnecessary, because local storage is a synchronous data provider, and being local, not remote, has no benefit of being cached or deduplicated in requests for cache keys.
If it's the former, IDB is the better solution because local storage is small.
| // Load saved settings or use defaults | ||
| const savedSettings = loadSettingsFromStorage(); | ||
| const initialGateway = savedSettings?.gateway || ARWEAVE_HOST; | ||
| const initialAoNetwork = savedSettings?.aoNetwork || NETWORK_DEFAULTS.AO; | ||
| const initialTurboNetwork = | ||
| savedSettings?.turboNetwork || NETWORK_DEFAULTS.TURBO; | ||
| const initialArioProcessId = savedSettings?.arioProcessId || ARIO_PROCESS_ID; | ||
| const initialHyperbeamUrl = savedSettings?.hyperbeamUrl; |
There was a problem hiding this comment.
nit: use URLs where it makes sense
| // Load saved settings or use defaults | |
| const savedSettings = loadSettingsFromStorage(); | |
| const initialGateway = savedSettings?.gateway || ARWEAVE_HOST; | |
| const initialAoNetwork = savedSettings?.aoNetwork || NETWORK_DEFAULTS.AO; | |
| const initialTurboNetwork = | |
| savedSettings?.turboNetwork || NETWORK_DEFAULTS.TURBO; | |
| const initialArioProcessId = savedSettings?.arioProcessId || ARIO_PROCESS_ID; | |
| const initialHyperbeamUrl = savedSettings?.hyperbeamUrl; | |
| const initialGateway = new URL(savedSettings?.gateway || ARWEAVE_HOST); | |
| const initialAoNetwork = new URL(savedSettings?.aoNetwork || NETWORK_DEFAULTS.AO); | |
| const initialTurboNetwork = | |
| new URL(savedSettings?.turboNetwork || NETWORK_DEFAULTS.TURBO); | |
| const initialArioProcessId = savedSettings?.arioProcessId || ARIO_PROCESS_ID; | |
| const initialHyperbeamUrl = new URL(savedSettings?.hyperbeamUr)l; | |
| // and then use `url.hostname` or `url.toString()` below |
There was a problem hiding this comment.
Gonna wait on this... In my experience changing to url has had some unforseen side effects and we have some third party clients that are expecting strings.
dtfiedler
left a comment
dtfiedler
left a comment
There was a problem hiding this comment.
looks good - a few non-blocking questions
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes