refactor: eslint-disable exhaustive-deps x split store actions out & neaten airdrop auth flow
#1520
Conversation
📝 WalkthroughWalkthroughThe changes refactor various parts of the codebase by updating import paths, simplifying error handling, and centralizing state management using direct store action imports. New ESLint rules and dependencies for React compilation have been added. Several React components, hooks, and store modules have been restructured to remove redundant logic, improve performance via memoization, and standardize API interactions and type definitions. Additionally, new functionalities related to airdrop management and application configuration have been introduced, enhancing the overall capabilities of the application. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Component
participant H as useFetchAirdropToken Hook
participant R as handleAirdropRequest
participant S as Store
participant F as fetchAllUserData
C->>H: Initialize hook with canListen=true
H->>C: Setup interval & timeout
H->>R: Call handleAirdropRequest (every second)
R-->>H: Return token response
H->>S: Update airdrop tokens in store
H->>F: Trigger fetchAllUserData if valid
H->>C: Clear interval and finish fetch cycle
Poem
🪧 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
Documentation and Community
|
shanimal08
changed the title
shanimal08
changed the title
…ustive dep rule FOR NOW
shanimal08
changed the title
shanimal08
changed the title
eslint-disable exhaustive-deps x split store actions out & neaten airdrop auth flow
shanimal08
requested review from
PanchoBubble,
brianp,
peps and
stringhandler
shanimal08
requested review from
BalazsSevecsek,
MCozhusheck,
Misieq01,
Krakaw and
mmrrnn
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (64)
src/store/useMiningStore.ts (1)
4-14: Consider enumerating possible values for thenetworkfield.
Defining a string union or enum fornetwork(e.g.,'testnet' | 'mainnet' | 'unknown') helps enforce type safety and clarity.src/store/appStateStore.ts (1)
22-32: Use consistent naming for the initial state variable.
Consider renaminginitialstatetoinitialStatefor readability and consistency across files.-const initialstate: AppState = { +const initialState: AppState = {src/store/useAppConfigStore.ts (1)
5-32: Consider providing a default value forairdrop_tokens.
Leavingairdrop_tokensasundefinedmight require extra null checks. Setting it to an empty array/object can simplify usage while preserving an uninitialized state.src/hooks/airdrop/utils/useHandleRequest.ts (3)
8-9: Consider defaulting to an empty headers object.By making
headersoptional, you rely on external callers to supply essential headers or skip them entirely. If a caller forgets to pass headers, the function returns early in line 18. Providing a default (e.g.,{}) might reduce unexpected behavior.
24-27: Merge default headers with user-defined values.Currently, passing
headersmeans you lose the default'Content-Type'and'Authorization'unless you replicate them yourself. Merging them ensures partial overrides while preserving defaults. For instance:-headers: headers ?? { +headers: headers ? { ...{ 'Content-Type': 'application/json', Authorization: `Bearer ${airdropToken}` }, ...headers } : { 'Content-Type': 'application/json', Authorization: `Bearer ${airdropToken}`, }
31-39: Handle non-JSON error payloads gracefully.If the server returns an error in a non-JSON format (HTML, text, etc.), attempting
response.json()(line 38) could fail or yield unexpected results. Consider checking theContent-Typeheader or handling text responses separately to produce clearer error messages.src/containers/main/Airdrop/AirdropGiftTracker/sections/LoggedOut/LoggedOut.tsx (1)
25-36: Add error handling for the open() promise.While the
thencallback setslinkOpenedtotrue, there is no.catchin caseopen()fails. Consider catching errors or user cancellations to handle scenarios like the OS blocking the external URL.src/store/useAirdropStore.ts (2)
72-72: Document the installReward flag.The optional
installReward?: boolean;property suggests a new feature, but its usage isn’t clear within this file. Consider adding a short comment or inline doc explaining how it integrates into the airdrop logic.
104-115: Prefer a typed factory function if advanced features are needed.You’re calling
create<AirdropStoreState>()(() => ({ ...initialState })), which is effective for a straightforward store. If you anticipate actions or expansions, consider an extended store pattern with typed methods to maintain clarity.src/containers/floating/Settings/sections/airdrop/ApplyInviteCode.tsx (2)
24-24: Local state linkOpened.
Maintaining locallinkOpenedstate is straightforward. A small recommendation: double-check if this flag is always reset appropriately when needed (e.g., on unmount or after a successful token fetch).
29-30: Consistent store usage.
DestructuringallowTelemetryandairdropUrlfrom separate stores is logical, but watch out for concurrency or mismatch if these values can be updated in other parts of the app simultaneously.src/store/actions/miningStoreActions.ts (6)
18-45: Implementing changeMiningMode.
- Pausing mining before changing the mode is a solid approach to reduce collisions.
- Re-check the logic for
if (metricsState.cpu_mining_status.is_mining || metricsState.gpu_mining_status.is_mining)to confirm it sufficiently covers all possible partial mining states.- Consider improving error handling if partial actions (like pausing CPU but failing GPU) occur.
- Good final
finallyblock to resetisChangingMode.
56-66: Pausing mining flow.
- Straightforward approach.
- If the
stop_miningcommand partially fails, the store setsminingInitiated: true. Check if this might cause confusion if the system is partially paused. Possibly set a more descriptive flag to reflect partial success/failure.
67-83: Restarting mining.
- The logic to pause then start is fine.
- Consider a guard if “already paused” to avoid repeated calls.
- Retain consistent error handling as done in other methods.
86-102: setExcludedGpuDevices logic.
- Good to see thorough logging.
- The approach automatically disables GPU mining if all GPUs are excluded. That makes sense, but confirm that you want to preserve CPU mining status.
103-111: Enabling/disabling mining controls.
- Checks
isChangingMode+ GPU/CPU availability.- Potential improvement: local variables or a small helper function to reduce the nested logic in the return statement.
112-121: Setting the mining network.
Handles the Tauri invoke call with error logging. Good. Consider standardizing error logs to reduce noise (e.g., structured logs).src/store/actions/airdropStoreActions.ts (6)
20-27: Defining TokenResponse interface.
Provides clarity around the shape of JWT payload. Consider adding optional fields if the token’s schema can expand.
29-43: Parsing JWT tokens on the client.
- Decodes base64 and attempts to parse. If the token structure is malformed, an error can be thrown.
- Could encapsulate a try/catch to gracefully handle broken tokens.
56-84: fetchBackendInMemoryConfig.
- Calls Tauri endpoints, storing config & tokens if present.
- Throws an error if there's a problem with fetching config, which is good.
- Logs an error if
airdropUrlis missing. Possibly propagate a more user-friendly message if this is critical to the operational flow.
111-125: airdropSetup.
- Good orchestrator function calling
fetchBackendInMemoryConfig+getExistingTokens.- Runs
handleRefreshAirdropTokensandfetchAllUserDataif an airdrop URL is present. This is a logical flow, but ensure performance is acceptable if these calls happen frequently.
126-129: handleAirdropLogout.
Logs out a user upon errors. Straightforward. Make sure other parts of the app handle the user timeline or re-auth flows gracefully.
182-254: fetchAllUserData.
- Aggregates user details, points, referral, and bonus tier requests in parallel.
- Gracefully logs out if any call fails. This ensures consistent security posture.
- Consider re-trying or partial updates if one call fails while others succeed (an optional improvement).
src/store/create.ts (1)
16-20: LGTM! Clean implementation of store reset functionality.The implementation properly registers reset functions and ensures complete state reset. Consider adding a type for the reset function to improve type safety.
-const storeResetFns = new Set<() => void>(); +type StoreResetFn = () => void; +const storeResetFns = new Set<StoreResetFn>();src/hooks/airdrop/stateHelpers/useAirdropSetTokens.ts (1)
3-3: LGTM! Clean refactor of airdrop token handling.The implementation correctly handles the airdrop token setting flow. Consider adding error handling for the request.
return useCallback(async () => { if (!airdropUser) return; const generatedAuthUuid = uuidv4(); - const data = await handleAirdropRequest<{ success: boolean }>({ - path: `/auth/set-token/${generatedAuthUuid}`, - method: 'POST', - }); - if (!data?.success) return; - return encodeURI(generatedAuthUuid); + try { + const data = await handleAirdropRequest<{ success: boolean }>({ + path: `/auth/set-token/${generatedAuthUuid}`, + method: 'POST', + }); + if (!data?.success) return; + return encodeURI(generatedAuthUuid); + } catch (error) { + console.error('Failed to set airdrop token:', error); + return; + } }, [airdropUser]);Also applies to: 9-18
src/store/actions/uiStoreActions.ts (1)
1-19: LGTM! Well-organized store actions.The file structure aligns with the PR objectives by centralizing UI store actions. Each function has a clear and focused responsibility.
Consider adding JSDoc comments to document the purpose and parameters of each function. For example:
+/** + * Updates the visibility of the external dependencies dialog. + * @param showExternalDependenciesDialog - Whether to show or hide the dialog + */ export const setShowExternalDependenciesDialog = (showExternalDependenciesDialog: boolean) => useUIStore.setState({ showExternalDependenciesDialog }); +/** + * Updates the UI theme and corresponding animation properties. + * @param theme - The theme to apply ('light' or 'dark') + */ export const setUITheme = (theme: Theme) => { setAnimationProperties(theme === 'light' ? animationLightBg : animationDarkBg); useUIStore.setState({ theme }); };src/containers/floating/Settings/components/LanguageDropdown.tsx (1)
22-37: Consider memoizing the onChange handler.The component is correctly memoized, but the
onChangehandler is recreated on each render.const LanguageDropdown = memo(function LanguageDropdown() { + const handleChange = useCallback((value: string) => + setApplicationLanguage(value as Language), + []); + return ( <Wrapper> <Select options={languageOptions} - onChange={(value) => setApplicationLanguage(value as Language)} + onChange={handleChange} selectedValue={resolveI18nLanguage(i18n.language)} variant="bordered" forceHeight={36} /> </Wrapper> ); });src/containers/main/SideBar/Miner/components/CustomPowerLevels/CustomPowerLevelsDialogContainer.tsx (1)
5-5: Consider handling potential race condition in useEffect.While the refactoring to import store actions directly is good, the
useEffecthook might trigger multiplegetMaxAvailableThreadscalls if the component unmounts before the async operation completes.Consider using an abort controller or cleanup function:
useEffect(() => { + let mounted = true; if (!maxThreads) { - getMaxAvailableThreads(); + void getMaxAvailableThreads().then(() => { + if (!mounted) return; + }); } + return () => { + mounted = false; + }; }, [maxThreads]);Also applies to: 12-14, 16-20
src/containers/floating/ReleaseNotesDialog/ReleaseNotesDialog.tsx (1)
9-9: Consider optimizing memo usage.While the component is memoized, the
openprop derived fromuseUIStoreisn't stable between renders. Consider usinguseMemofor the dialog content to optimize performance:-const ReleaseNotesDialog = memo(function ReleaseNotesDialog() { +function ReleaseNotesDialog() { const open = useUIStore((s) => s.dialogToShow === 'releaseNotes'); const { t } = useTranslation('components', { useSuspense: false }); const handleClose = useCallback(async () => { setDialogToShow(null); }, []); + const dialogContent = useMemo(() => ( + <DialogContent> + <Wrapper> + <Title>{t('releaseNotesDialog.title')}</Title> + <ReleaseNotes noHeader={true} showScrollBars={true} /> + <ButtonWrapper> + <Button onClick={handleClose}> + <span>{t('releaseNotesDialog.close')}</span> + </Button> + </ButtonWrapper> + </Wrapper> + </DialogContent> + ), [handleClose, t]); return ( <Dialog open={open} onOpenChange={handleClose}> - <DialogContent> - <Wrapper> - <Title>{t('releaseNotesDialog.title')}</Title> - <ReleaseNotes noHeader={true} showScrollBars={true} /> - <ButtonWrapper> - <Button onClick={handleClose}> - <span>{t('releaseNotesDialog.close')}</span> - </Button> - </ButtonWrapper> - </Wrapper> - </DialogContent> + {dialogContent} </Dialog> ); -}); +} export default ReleaseNotesDialog;Also applies to: 35-35
src/hooks/airdrop/stateHelpers/useAirdropUserPointsListener.ts (1)
23-23: Track technical debt: TODO comment about WebSocket updates.The TODO comment indicates future work to remove this listener. Consider creating a tracking issue to ensure this technical debt is addressed.
Would you like me to create an issue to track the removal of this listener and backend emit per WebSocket update conversation?
src/store/useP2poolStatsStore.ts (1)
31-46: Enhance error logging for better debugging.While the error handling is good, the error messages could be more descriptive to aid in debugging.
- console.error('Could not get p2p stats: ', e); + console.error('Failed to fetch P2Pool stats. This might affect mining statistics display: ', e); - console.error('Could not get p2p connections: ', e); + console.error('Failed to fetch P2Pool connections. This might affect peer information display: ', e);src/containers/main/ShellOfSecrets/SoSWidget/segments/Timer/Timer.tsx (1)
1-1: LGTM! Performance optimization with memo and simplified state management.The changes improve performance by:
- Memoizing the component to prevent unnecessary re-renders
- Replacing store hook with direct function call, aligning with PR objectives
However, there's a typo in the state variable name.
- const [reminingTime, setRemainingTime] = useState({ days: 0, hours: 0, totalRemainingMs: 0 }); + const [remainingTime, setRemainingTime] = useState({ days: 0, hours: 0, totalRemainingMs: 0 });Also applies to: 13-13, 15-15, 21-21, 49-51
src/containers/main/Airdrop/AirdropGiftTracker/sections/LoggedIn/segments/Flare/Flare.tsx (1)
38-38: Consider memoizing the onClick handler.The inline function in the onClick prop will be recreated on every render. Consider memoizing it with useCallback.
+ const handleClick = useCallback(() => setFlareAnimationType(), [setFlareAnimationType]); + return ( <Wrapper initial={{ opacity: 0 }} animate={{ opacity: 1 }} exit={{ opacity: 0 }} - onClick={() => setFlareAnimationType()} + onClick={handleClick} >src/hooks/airdrop/stateHelpers/useGetReferralQuestPoints.ts (1)
26-52: Add error handling and cleanup for the effect hook.The effect hook could benefit from:
- Error handling for the fetch operation
- A cleanup function to prevent state updates if the component unmounts during the request
- Justification for the empty dependency array
Consider this implementation:
useEffect(() => { + let isSubscribed = true; const handleFetch = async () => { + try { const response = await handleAirdropRequest<QuestDataResponse>({ path: `/quest/list-with-fulfillment`, method: 'GET', }); - if (!response?.quests.length) return; + if (!isSubscribed || !response?.quests.length) return; const reducedQuest = response.quests.reduce( // ... existing reduce logic ); setReferralQuestPoints(reducedQuest); + } catch (error) { + console.error('Failed to fetch referral quest points:', error); + } }; handleFetch(); + return () => { + isSubscribed = false; + }; + // Effect runs once on mount to fetch initial quest points }, []);src/components/AdminUI/groups/OtherUIGroup.tsx (2)
30-30: Address or remove the TODO comment.The TODO comment suggests adding other sections but lacks specificity.
Would you like me to help identify and implement the missing sections or should we remove this comment?
32-34: Memoize click handlers and add type safety for animation types.The click handlers could be memoized, and animation types should be type-safe.
Consider this implementation:
+const FLARE_ANIMATION_TYPES = { + FriendAccepted: 'FriendAccepted', + GoalComplete: 'GoalComplete', + BonusGems: 'BonusGems', +} as const; + +type FlareAnimationType = keyof typeof FLARE_ANIMATION_TYPES; + export function OtherUIGroup() { + const handleFlareAnimation = useCallback((type: FlareAnimationType) => { + setFlareAnimationType(type); + }, []); + // ... rest of the component <ButtonGroup> - <Button onClick={() => setFlareAnimationType('FriendAccepted')}>FriendAccepted</Button> - <Button onClick={() => setFlareAnimationType('GoalComplete')}>GoalComplete</Button> - <Button onClick={() => setFlareAnimationType('BonusGems')}>BonusGems</Button> + {Object.entries(FLARE_ANIMATION_TYPES).map(([type]) => ( + <Button key={type} onClick={() => handleFlareAnimation(type as FlareAnimationType)}> + {type} + </Button> + ))} </ButtonGroup>src/containers/main/Airdrop/Settings/Logout.tsx (1)
37-37: Memoize click handler and add loading state.The logout handler could be memoized and should show loading state during the operation.
Consider this implementation:
export default function AirdropLogout() { + const [isLoggingOut, setIsLoggingOut] = useState(false); + const handleLogout = useCallback(async () => { + setIsLoggingOut(true); + try { + await handleAirdropLogout(); + } finally { + setIsLoggingOut(false); + } + }, []); // ... rest of the component - <Button color="warning" size="small" onClick={() => handleAirdropLogout()}> + <Button + color="warning" + size="small" + onClick={handleLogout} + disabled={isLoggingOut} + > - {t('disconnect')} + {isLoggingOut ? t('disconnecting') : t('disconnect')} </Button>src/containers/floating/Settings/sections/mining/CpuMiningMarkup.tsx (2)
23-25: Add error handling and loading state for mining toggle.The mining toggle handler should handle errors gracefully and show loading state.
Consider this implementation:
+ const [isUpdating, setIsUpdating] = useState(false); const handleCpuMiningEnabled = useCallback(async () => { + setIsUpdating(true); + try { await setCpuMiningEnabled(!isCpuMiningEnabled); + } catch (error) { + console.error('Failed to toggle CPU mining:', error); + // Consider showing an error toast/notification here + } finally { + setIsUpdating(false); + } }, [isCpuMiningEnabled]);
37-41: Update ToggleSwitch to reflect loading state.The ToggleSwitch should be disabled during state updates.
<ToggleSwitch checked={isCpuMiningEnabled} - disabled={isSettingUp} + disabled={isSettingUp || isUpdating} onChange={handleCpuMiningEnabled} />src/containers/main/Dashboard/components/VisualMode.tsx (1)
26-32: Add error handling for DOM manipulation.The direct DOM manipulation could fail if the element is not found. Consider adding error handling:
const handleSwitch = useCallback(() => { const canvasElement = document.getElementById('canvas'); - if (canvasElement) { - canvasElement.style.display = visualMode ? 'none' : 'block'; - } + try { + if (canvasElement) { + canvasElement.style.display = visualMode ? 'none' : 'block'; + } else { + console.warn('Canvas element not found'); + } + } catch (error) { + console.error('Error updating canvas visibility:', error); + } setVisualMode(!visualMode); }, [visualMode]);src/hooks/app/useSetUp.ts (1)
17-21: Add error handling for async operations.Multiple async operations are performed without error handling, which could lead to silent failures.
const handlePostSetup = useCallback(async () => { - await setSetupComplete(); - await fetchApplicationsVersionsWithRetry(); - await airdropSetup(); + try { + await setSetupComplete(); + await fetchApplicationsVersionsWithRetry(); + await airdropSetup(); + } catch (error) { + console.error('Error during post-setup:', error); + // Consider showing an error message to the user + } }, []);src/hooks/airdrop/stateHelpers/useFetchAirdropToken.ts (1)
38-38: Extract magic numbers into named constants.The interval and timeout durations should be extracted into named constants for better maintainability.
+const POLLING_INTERVAL_MS = 1000; +const TOKEN_FETCH_TIMEOUT_MS = 1000 * 60 * 5; // 5 minutes // ... - }, 1000); + }, POLLING_INTERVAL_MS); const timeout = setTimeout( () => { clearInterval(interval); setAuthUuid(''); }, - 1000 * 60 * 5 + TOKEN_FETCH_TIMEOUT_MS );Also applies to: 44-44
src/containers/floating/Settings/sections/p2p/P2pMarkup.tsx (1)
30-37: Consider handling errors in the async state updates.The function performs multiple state updates but lacks error handling. If
setP2poolEnabledfails, the subsequent state updates might execute incorrectly.const handleP2poolEnabled = useCallback( async (event: React.ChangeEvent<HTMLInputElement>) => { - await setP2poolEnabled(event.target.checked); - setDisabledStats(!event.target.checked); - setDialogToShow('restart'); + try { + await setP2poolEnabled(event.target.checked); + setDisabledStats(!event.target.checked); + setDialogToShow('restart'); + } catch (error) { + console.error('Failed to update P2Pool settings:', error); + // Consider showing an error dialog + } }, [setDisabledStats] );src/containers/floating/Settings/sections/mining/GpuMiningMarkup.tsx (1)
30-32: Add loading state and error handling for better UX.The GPU mining toggle lacks feedback during state updates and error handling.
+const [isUpdating, setIsUpdating] = useState(false); const handleGpuMiningEnabled = useCallback(async () => { + setIsUpdating(true); + try { await setGpuMiningEnabled(!isGpuMiningEnabled); + } catch (error) { + console.error('Failed to update GPU mining settings:', error); + // Consider showing an error dialog + } finally { + setIsUpdating(false); + } }, [isGpuMiningEnabled]);Then update the ToggleSwitch:
<ToggleSwitch checked={isGpuMiningEnabled} - disabled={isSettingUp} + disabled={isSettingUp || isUpdating} onChange={handleGpuMiningEnabled} />src/containers/floating/LudicrousCofirmationDialog/LudicrousCofirmationDialog.tsx (2)
32-35: Add error handling for mining mode changes.The handleChange function performs state updates without error handling.
const handleChange = useCallback(async () => { + try { await changeMiningMode({ mode: 'Ludicrous' }); setDialogToShow(null); + } catch (error) { + console.error('Failed to change mining mode:', error); + // Consider showing an error dialog + } }, []);
9-22: Consider cleanup for the countdown timer on dialog close.The Countdown component should clear its timer when the dialog closes to prevent memory leaks.
const Countdown = memo(function Countdown({ onComplete }: { onComplete: () => void }) { const [count, setCount] = useState(30); + const open = useUIStore((s) => s.dialogToShow === 'ludicrousConfirmation'); useEffect(() => { + if (!open) return; if (count === 0) { onComplete(); return; } const timer = setTimeout(() => setCount(count - 1), 1000); return () => clearTimeout(timer); - }, [count, onComplete]); + }, [count, onComplete, open]); return <CountdownNumber>{count < 10 ? `0${count}` : count}</CountdownNumber>; });eslint.config.js (1)
57-63: Track temporary rule disabling.Both
react-hooks/exhaustive-depsand its compiler variant are temporarily disabled with a comment "just for now until WS is sorted". Consider creating a tracking issue to re-enable these rules once the WebSocket refactoring is complete.Would you like me to create an issue to track the re-enabling of these ESLint rules?
src/hooks/airdrop/stateHelpers/useGetSosReferrals.ts (1)
7-7: Consider extracting MAX_REFERRALS to a configuration file.Magic numbers should be defined in a centralized configuration file with descriptive names for better maintainability.
-const MAX_REFERRALS = 100; +import { MAX_REFERRALS } from '@app/config/constants';src/containers/floating/Settings/sections/general/LogsSettings.tsx (1)
47-47: Address the TODO comment about moving reference to dialog.The comment suggests moving the reference handling to the dialog component, which could improve component encapsulation.
Would you like me to help implement this refactoring to move the reference handling into the SendLogsDialog component?
src/store/useShellOfSecretsStore.ts (1)
88-97: Consider memoizing time calculations to prevent unnecessary re-renders.The
getSOSTimeRemainingfunction directly accesses store state usinggetState(). Consider using a selector or memoizing the result to prevent unnecessary re-renders when the component using this function re-renders but the actual time values haven't changed significantly.-export const getSOSTimeRemaining = () => { +export const getSOSTimeRemaining = (() => { + let lastCalculation: { time: number; result: ReturnType<typeof calculateRemaining> } | null = null; + const CACHE_DURATION = 1000; // 1 second + + const calculateRemaining = () => { + const state = useShellOfSecretsStore.getState(); + const now = new Date(); + const targetDateBroughtForwardByTimeBonus = + state.revealDate.getTime() - state.totalBonusTimeMs; + const remainingMs = targetDateBroughtForwardByTimeBonus - now.getTime(); + + const days = Math.floor(remainingMs / (1000 * 60 * 60 * 24)); + const hours = Math.floor((remainingMs % (1000 * 60 * 60 * 24)) / (1000 * 60 * 60)); + return { days, hours, totalRemainingMs: remainingMs }; + }; + + return () => { + const now = Date.now(); + if (!lastCalculation || now - lastCalculation.time > CACHE_DURATION) { + lastCalculation = { + time: now, + result: calculateRemaining() + }; + } + return lastCalculation.result; + }; +})();src/containers/floating/Settings/sections/p2p/P2PoolStats.tsx (1)
29-39: Consider adding error handling and cleanup for pending requests.While the interval cleanup is handled, there's no cleanup for pending fetch requests when the component unmounts. This could lead to memory leaks or state updates on unmounted components.
useEffect(() => { + let mounted = true; const handleFetchP2pStats = async () => { - await fetchP2poolStats?.(); - await fetchP2poolConnections?.(); + try { + if (mounted) { + await fetchP2poolStats?.(); + await fetchP2poolConnections?.(); + } + } catch (error) { + console.error('Failed to fetch P2P stats:', error); + } }; void handleFetchP2pStats(); const fetchP2pStatsInterval = setInterval(handleFetchP2pStats, 5000); return () => { + mounted = false; clearInterval(fetchP2pStatsInterval); }; }, []);src/containers/main/Dashboard/MiningView/components/Ruler.tsx (1)
32-58: Simplify ruler segment calculations for better readability.The logic for calculating ruler segments and number marks is complex and could be simplified for better maintainability.
const bottomMarkSegments = rulerSegments.map((segment, i) => { - let heightSegment = height; const diff = height && height > 50 ? 10 : 5; - const renderNumber = heightSegment && heightSegment > diff; - - if (renderNumber && heightSegment) { - heightSegment -= diff; - } - - const prevSegment = (heightSegment || 0) + diff; + const heightSegment = height && height > diff ? height - diff : height; + const shouldRenderNumber = heightSegment && heightSegment > diff; const groupOpacity = (rulerSegments.length * 1.25 - segment) * 0.075; - const numberMark = - heightSegment && heightSegment > diff && heightSegment != prevSegment - ? heightSegment?.toLocaleString() - : ''; + const numberMark = shouldRenderNumber ? heightSegment?.toLocaleString() : ''; return ( <MarkGroup key={`row-${segment}-${i}`} style={{ opacity: groupOpacity }}> <RulerMarkGroup> <RulerMark $opacity={1} data-before={numberMark} /> <RulerMark /> <RulerMark /> <RulerMark /> <RulerMark /> </RulerMarkGroup> </MarkGroup> ); });src/components/dialogs/SendLogsDialog.tsx (1)
34-54: Consider improving error handling insendLogs.The current error handling simply converts the error to string, which might not provide the most user-friendly message.
.catch((error) => { console.error('Error sending feedback: ', error); - setError(error.toString()); + setError( + error.message || + t('error-sending-feedback', { ns: 'settings', defaultValue: 'Failed to send feedback' }) + ); })src/components/ToastStack/Toast/Toast.tsx (1)
45-75: Consider using requestAnimationFrame for smoother progress animation.The current implementation uses setTimeout which can lead to timing inaccuracies. Using requestAnimationFrame would provide smoother animations and better performance.
useEffect(() => { if (show) { if (!isHovered) { lastUpdateRef.current = Date.now(); + let animationFrame: number; const updateProgress = () => { const now = Date.now(); elapsedTimeRef.current += now - lastUpdateRef.current; lastUpdateRef.current = now; const newProgress = (elapsedTimeRef.current / timeout) * 100; if (newProgress <= 100) { progress.set(newProgress); - progressInterval.current = setTimeout(updateProgress, 10); + animationFrame = requestAnimationFrame(updateProgress); } else if (!finished) { setFinished(true); setTimeout(() => handleHide(id), 200); } }; - updateProgress(); + animationFrame = requestAnimationFrame(updateProgress); + return () => cancelAnimationFrame(animationFrame); } else { // save the pause point const now = Date.now(); elapsedTimeRef.current += now - lastUpdateRef.current; - clearTimeout(progressInterval.current); } } - - return () => clearTimeout(progressInterval.current); }, [show, isHovered, timeout, id, handleHide, finished, progress]);src/containers/floating/Settings/sections/general/ResetSettingsButton.tsx (1)
5-5: Consider improving error handling with i18n support.While the state management changes look good, the error message could be more user-friendly with translation support.
.catch((e) => { console.error('Error when resetting settings: ', e); - setError('Resetting settings failed: ' + e); + setError( + t('reset-settings-error', { + error: e.message || e, + defaultValue: 'Failed to reset settings: {{error}}' + }) + ); setLoading(false); setOpen(false); });Also applies to: 38-42
src/containers/floating/ExternalDependenciesDialog/ExternalDependenciesDialog.tsx (1)
7-7: Consider consolidating store imports.While the change to import
setShowExternalDependenciesDialogfrom '@app/store' is good, there's still a direct import from 'useUIStore' for accessing state. Consider consolidating both state and actions to be imported from '@app/store' for consistency.-import { useUIStore } from '@app/store/useUIStore'; -import { setShowExternalDependenciesDialog } from '@app/store'; +import { useUIStore, setShowExternalDependenciesDialog } from '@app/store';Also applies to: 13-13
src/containers/floating/Settings/sections/mining/GpuDevices.tsx (1)
33-44: Optimize array operations in handleSetExcludedDevice.The current implementation mutates the array directly and then spreads it. Consider using immutable array operations for better predictability and to avoid potential side effects.
const handleSetExcludedDevice = useCallback( async (device: number) => { - if (!excludedDevices.includes(device)) { - excludedDevices.push(device); - await setExcludedGpuDevices([...excludedDevices]); - } else { - excludedDevices.splice(excludedDevices.indexOf(device), 1); - await setExcludedGpuDevices([...excludedDevices]); - } + const newExcludedDevices = excludedDevices.includes(device) + ? excludedDevices.filter(d => d !== device) + : [...excludedDevices, device]; + await setExcludedGpuDevices(newExcludedDevices); }, [excludedDevices] );src/store/actions/appStateStoreActions.ts (2)
10-34: Consider improving retry logic.The retry mechanism in
fetchApplicationsVersionsWithRetrydecrements the retry counter after a successful fetch, which could lead to unnecessary retries. Also, there's no delay between retries which could overwhelm the system.Consider adding exponential backoff and only decrementing retries on failure:
export const fetchApplicationsVersionsWithRetry = async () => { let retries = 5; + let delay = 1000; // Start with 1 second delay while (retries) { const applications_versions = useAppStateStore.getState().applications_versions; if (applications_versions && Object.values(applications_versions).every((version) => Boolean(version))) { break; } try { await fetchApplicationsVersions(); - retries--; break; } catch (error) { console.error('Error getting applications versions', error); + retries--; + if (retries) { + await new Promise(resolve => setTimeout(resolve, delay)); + delay *= 2; // Exponential backoff + } } } };
48-52: Consider adding error categorization.The
setErrorfunction could benefit from error categorization to help users better understand the nature of the error.Consider adding error categories:
-export const setError = (error: string | undefined) => { +export const setError = (error: string | undefined, category: 'network' | 'system' | 'user' | 'unknown' = 'unknown') => { useAppStateStore.setState({ error }); console.error('setError:', error); - addToast({ title: 'Error', text: error, type: 'error' }); + addToast({ title: `${category.charAt(0).toUpperCase() + category.slice(1)} Error`, text: error, type: 'error' }); };src/containers/floating/Settings/sections/wallet/MoneroSeedWords/MoneroSeedWordSettings.tsx (1)
29-47: Consider improving error handling ingetSeedWords.The error handling could be more specific and provide better feedback to users.
Consider this improvement:
const getSeedWords = useCallback(async () => { setIsFetching(true); try { const mSeedWords = await invoke('get_monero_seed_words'); if (mSeedWords) { setSeedWords(mSeedWords); hasFetched.current = true; return mSeedWords; } + throw new Error('No seed words returned'); } catch (e) { const errorMessage = e as unknown as string; - if (errorMessage && errorMessage.includes('Keychain access')) { - setError(errorMessage); + if (typeof errorMessage === 'string') { + if (errorMessage.includes('Keychain access')) { + setError('Unable to access keychain. Please check your system permissions.', 'system'); + } else { + setError('Failed to retrieve seed words. Please try again.', 'system'); + } } console.error('Could not get Monero seed words', e); } finally { setIsFetching(false); } }, []);🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/containers/floating/Settings/sections/experimental/MonerodMarkup/MonerodMarkup.tsx (1)
58-62: Consider adding error handling to the save operation.The promise chain lacks error handling, which could lead to silent failures.
Consider adding error handling:
const onSave = async (formValues: FormValues) => { - setMonerodConfig(formValues.use_monero_fail, formValues.monero_nodes).then(() => { - setDialogToShow('restart'); - }); + try { + await setMonerodConfig(formValues.use_monero_fail, formValues.monero_nodes); + setDialogToShow('restart'); + } catch (error) { + console.error('Failed to save Monerod configuration:', error); + setError('Failed to save configuration. Please try again.', 'system'); + } };src/types/app-status.ts (1)
52-55: Consider aligning property names with backend convention.The
airdrop_tokenstype usesrefreshTokenwhile the backend expectsrefresh_token. Consider using consistent naming to avoid confusion:airdrop_tokens?: { token: string; - refreshToken: string; + refresh_token: string; };src/hooks/airdrop/useWebsocket.ts (1)
35-35: Consider usinguseReffor socket instance.Since the socket instance is used for event handling and doesn't require re-rendering on updates, consider using
useRefinstead ofuseState.- const [socket, setSocket] = useState<ReturnType<typeof io> | null>(null); + const socket = useRef<ReturnType<typeof io> | null>(null);src/store/actions/appConfigStoreActions.ts (1)
11-15: Add type forcustomGpuLevelsandcustomCpuLevels.Consider adding more specific types for the custom levels to improve type safety.
interface SetModeProps { mode: modeType; - customGpuLevels?: GpuThreads[]; - customCpuLevels?: number; + customGpuLevels?: readonly GpuThreads[]; + customCpuLevels?: Readonly<number>; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (92)
eslint.config.js(2 hunks)package.json(1 hunks)src/App/App.tsx(2 hunks)src/App/AppEffects.tsx(1 hunks)src/components/AdminUI/groups/DialogsGroup.tsx(1 hunks)src/components/AdminUI/groups/OtherUIGroup.tsx(2 hunks)src/components/AdminUI/groups/ThemeGroup.tsx(1 hunks)src/components/ToastStack/Toast/Toast.tsx(1 hunks)src/components/dialogs/RestartDialog.tsx(1 hunks)src/components/dialogs/SendLogsDialog.tsx(3 hunks)src/containers/floating/AutoUpdateDialog/AutoUpdateDialog.tsx(1 hunks)src/containers/floating/CriticalErrorDialog/CriticalErrorDialog.tsx(1 hunks)src/containers/floating/Error/ErrorSnackbar.styles.ts(0 hunks)src/containers/floating/Error/ErrorSnackbar.tsx(0 hunks)src/containers/floating/ExternalDependenciesDialog/ExternalDependenciesDialog.tsx(1 hunks)src/containers/floating/ExternalDependenciesDialog/ExternalDependencyCard.tsx(3 hunks)src/containers/floating/LudicrousCofirmationDialog/LudicrousCofirmationDialog.tsx(2 hunks)src/containers/floating/ReleaseNotesDialog/ReleaseNotesDialog.tsx(1 hunks)src/containers/floating/Settings/SettingsModal.tsx(1 hunks)src/containers/floating/Settings/components/LanguageDropdown.tsx(3 hunks)src/containers/floating/Settings/components/OpenSettingsButton.tsx(1 hunks)src/containers/floating/Settings/components/ThemeSelector.tsx(2 hunks)src/containers/floating/Settings/sections/airdrop/ApplyInviteCode.tsx(1 hunks)src/containers/floating/Settings/sections/experimental/AppVersions.tsx(1 hunks)src/containers/floating/Settings/sections/experimental/ExperimentalWarning.tsx(2 hunks)src/containers/floating/Settings/sections/experimental/MonerodMarkup/MonerodMarkup.tsx(3 hunks)src/containers/floating/Settings/sections/experimental/P2poolMarkup.tsx(3 hunks)src/containers/floating/Settings/sections/experimental/TorMarkup/TorMarkup.tsx(5 hunks)src/containers/floating/Settings/sections/general/AirdropPermissionSettings.tsx(1 hunks)src/containers/floating/Settings/sections/general/AudioSettings.tsx(1 hunks)src/containers/floating/Settings/sections/general/AutoUpdate.tsx(1 hunks)src/containers/floating/Settings/sections/general/LanguageSettings.tsx(1 hunks)src/containers/floating/Settings/sections/general/LogsSettings.tsx(2 hunks)src/containers/floating/Settings/sections/general/PreReleaseSettings.tsx(2 hunks)src/containers/floating/Settings/sections/general/ResetSettingsButton.tsx(2 hunks)src/containers/floating/Settings/sections/general/StartApplicationOnBootSettings.tsx(1 hunks)src/containers/floating/Settings/sections/mining/CpuMiningMarkup.tsx(1 hunks)src/containers/floating/Settings/sections/mining/GpuDevices.tsx(4 hunks)src/containers/floating/Settings/sections/mining/GpuMiningMarkup.tsx(2 hunks)src/containers/floating/Settings/sections/mining/MineOnStartMarkup.tsx(1 hunks)src/containers/floating/Settings/sections/p2p/P2PoolStats.tsx(2 hunks)src/containers/floating/Settings/sections/p2p/P2pMarkup.tsx(3 hunks)src/containers/floating/Settings/sections/wallet/MoneroAddressMarkup/MoneroAddressMarkup.tsx(1 hunks)src/containers/floating/Settings/sections/wallet/MoneroSeedWords/MoneroSeedWordSettings.tsx(2 hunks)src/containers/main/Airdrop/AirdropGiftTracker/AirdropGiftTracker.tsx(1 hunks)src/containers/main/Airdrop/AirdropGiftTracker/sections/LoggedIn/segments/Flare/Flare.tsx(1 hunks)src/containers/main/Airdrop/AirdropGiftTracker/sections/LoggedOut/LoggedOut.tsx(1 hunks)src/containers/main/Airdrop/Settings/Logout.tsx(2 hunks)src/containers/main/Dashboard/MiningView/components/Ruler.tsx(1 hunks)src/containers/main/Dashboard/components/VisualMode.tsx(2 hunks)src/containers/main/ShellOfSecrets/SoSWidget/segments/Timer/Timer.tsx(3 hunks)src/containers/main/SideBar/Miner/components/CustomPowerLevels/CustomPowerLevelsDialog.tsx(1 hunks)src/containers/main/SideBar/Miner/components/CustomPowerLevels/CustomPowerLevelsDialogContainer.tsx(1 hunks)src/containers/main/SideBar/Miner/components/ModeSelect.tsx(2 hunks)src/containers/main/SideBar/components/MiningButton/MiningButton.tsx(1 hunks)src/containers/phase/Setup/components/AirdropPermission/AirdropPermission.tsx(1 hunks)src/hooks/airdrop/stateHelpers/useAirdropSetTokens.ts(1 hunks)src/hooks/airdrop/stateHelpers/useAirdropTokensRefresh.ts(1 hunks)src/hooks/airdrop/stateHelpers/useAirdropUserPointsListener.ts(2 hunks)src/hooks/airdrop/stateHelpers/useFetchAirdropToken.ts(1 hunks)src/hooks/airdrop/stateHelpers/useGetAirdropUserDetails.ts(0 hunks)src/hooks/airdrop/stateHelpers/useGetReferralQuestPoints.ts(2 hunks)src/hooks/airdrop/stateHelpers/useGetSosReferrals.ts(3 hunks)src/hooks/airdrop/useAirdropSyncState.ts(0 hunks)src/hooks/airdrop/useWebsocket.ts(6 hunks)src/hooks/airdrop/utils/useHandleRequest.ts(1 hunks)src/hooks/airdrop/ws/useHandleWsUserIdEvent.ts(1 hunks)src/hooks/app/isAppReady.ts(1 hunks)src/hooks/app/useListenForAppUpdated.ts(2 hunks)src/hooks/app/useListenForExternalDependencies.ts(1 hunks)src/hooks/app/useSetUp.ts(1 hunks)src/hooks/helpers/useDetectMode.ts(2 hunks)src/hooks/useListenForCriticalProblem.tsx(1 hunks)src/store/actions/airdropStoreActions.ts(1 hunks)src/store/actions/appConfigStoreActions.ts(1 hunks)src/store/actions/appStateStoreActions.ts(1 hunks)src/store/actions/index.ts(1 hunks)src/store/actions/miningStoreActions.ts(1 hunks)src/store/actions/uiStoreActions.ts(1 hunks)src/store/appStateStore.ts(3 hunks)src/store/create.ts(1 hunks)src/store/index.ts(1 hunks)src/store/miningStoreActions.ts(0 hunks)src/store/useAirdropStore.ts(4 hunks)src/store/useAppConfigStore.ts(2 hunks)src/store/useBlockchainVisualisationStore.ts(4 hunks)src/store/useMiningStore.ts(3 hunks)src/store/useP2poolStatsStore.ts(2 hunks)src/store/useShellOfSecretsStore.ts(2 hunks)src/store/useUIStore.ts(1 hunks)src/types/app-status.ts(1 hunks)src/types/invoke.ts(1 hunks)
💤 Files with no reviewable changes (5)
- src/hooks/airdrop/useAirdropSyncState.ts
- src/containers/floating/Error/ErrorSnackbar.tsx
- src/hooks/airdrop/stateHelpers/useGetAirdropUserDetails.ts
- src/containers/floating/Error/ErrorSnackbar.styles.ts
- src/store/miningStoreActions.ts
✅ Files skipped from review due to trivial changes (6)
- src/hooks/app/isAppReady.ts
- src/containers/main/SideBar/Miner/components/CustomPowerLevels/CustomPowerLevelsDialog.tsx
- src/containers/main/SideBar/components/MiningButton/MiningButton.tsx
- src/hooks/airdrop/stateHelpers/useAirdropTokensRefresh.ts
- src/store/index.ts
- src/store/actions/index.ts
🔇 Additional comments (88)
src/store/useMiningStore.ts (3)
2-2: Import usage looks correct.
Nothing problematic here.
16-26: Initial state definitions look consistent.
All fields match theMiningStoreStateinterface, and default values are logically coherent. If'unknown'is used as a fallback fornetwork, ensure that higher-level logic handles this appropriately.
28-30: Zustand store creation aligns with the refactor strategy.
No actions are defined here, presumably because they have been moved elsewhere. This makes the store concise and readable.src/store/appStateStore.ts (3)
3-3: No issues with importing the create function.
This is consistent with other stores.
5-20: Validate optional fields usage.
Several fields (likeerror,criticalProblem, etc.) are optional. Ensure that any consumers check forundefinedor a default fallback to avoid runtime errors.
34-36: Simplistic state-only store is acceptable.
Similar to other files in this refactor, no actions are exposed here. This keeps things straightforward.src/store/useAppConfigStore.ts (3)
2-2: Import is correct.
No concerns with referencingAppConfighere.
4-4: Verify usage of Partial.
UsingPartial<AppConfig>allows for some fields to be undefined at runtime. Confirm that all dependent code can handle missing fields gracefully.
34-36: Store creation follows the refactor pattern.
This is consistent with the approach taken in other stores.src/hooks/airdrop/utils/useHandleRequest.ts (1)
11-18: Ensure consistent authorization flow.Because line 18 skips further checks if
headersis present, you bypass the built-in check for expired tokens. This might unintentionally allow requests with invalid tokens if custom headers are used. Consider validating the token regardless of theheadersparameter to maintain consistent security checks.src/containers/main/Airdrop/AirdropGiftTracker/sections/LoggedOut/LoggedOut.tsx (2)
14-15: Initialize local state with caution.Using
[linkOpened, setLinkOpened]to guard token fetching (useFetchAirdropToken) is logical, but ensure you handle edge cases where the link might open briefly and close. If you require repeated fetch attempts based on the link being active again, consider resettinglinkOpenedor adding more checks.
45-45: Include consistent referral code usage.When invoking
handleAuth, you pass no referral code. If a user might have a code, consider callinghandleAuth(codeValue). Otherwise, ensure code-based logic is intentionally bypassed.src/store/useAirdropStore.ts (1)
91-113: Confirm the absence of certain fields.Compared to the old store, fields like
MAX_GEMSorTokenResponseappear removed. Verify that no legacy code depends on them. If they are indeed obsolete, removing them is a good cleanup effort.src/containers/floating/Settings/sections/airdrop/ApplyInviteCode.tsx (4)
11-11: Use of useCallback for performance.
The addition ofuseCallbackis a good approach to prevent unnecessary re-renders. Ensure that any variables updated or referenced within the callback are included in the dependency array to prevent stale closures if they are expected to be reactive.
15-15: Importing from airdrop store.
ImportinguseAirdropStorehere is consistent with the application’s state management pattern. No issues found.
19-20: Centralized store imports.
PullingsetAllowTelemetry,setAuthUuid, anduseAppConfigStorecentrally streamlines updates to global state, making the code more consistent. Good practice to keep store actions consolidated.
28-28: Custom hook usage.
useFetchAirdropToken({ canListen: linkOpened })is descriptive. Verify that the hook gracefully handles any network errors or race conditions iflinkOpenedtoggles multiple times.src/store/actions/miningStoreActions.ts (9)
1-2: Initial imports.
These imports look fine. Ensure that@tauri-apps/api/coreusage is tested across all supported environments.
3-5: Mining & visualization stores.
Centralized store references (e.g.,useBlockchainVisualisationStore,useMiningMetricsStore) keep the code consistent. No immediate concerns.
6-7: Importing mining store & mode types.
No issues. The addition ofuseMiningStoreandmodeTypeis aligned with the codebase’s architecture.
8-9: Merging config store actions.
setGpuMiningEnabledandsetModeare key to controlling mining. Ensure thorough unit tests for these if not already present.
12-16: Defining ChangeMiningModeArgs interface.
Great for clarity on expected parameters. Type safety here helps prevent misconfiguration. No further remarks.
46-55: Retrieving maximum threads.
- Uses
invoke('get_max_consumption_levels')then updates store.- Handles errors by logging and setting error state.
- Consider verifying if the returned
maxAvailableThreadsshape meets all usage points.
84-86: setCustomLevelsDialogOpen setter.
Simple store mutation. No concerns except to ensure closing logic is invoked consistently in the UI.
122-136: Starting mining.
- Sets
miningInitiated: trueearly.- Good approach for updating UI states, but consider rolling back that flag if
invoke('start_mining')fails, so the UI always reflects the real status.
137-148: Stopping mining.
Implementation is parallel to pause logic. Ensure you re-check usage for any subtle differences between “stop” vs. “pause” if the design expects them to be conceptually distinct.src/store/actions/airdropStoreActions.ts (5)
1-19: New and consolidated imports.
Bringing in multiple store references and utility hooks (useAirdropTokensRefresh,handleAirdropRequest) helps unify airdrop logic. No apparent concerns.
45-54: clearState definition.
A single source of truth for resetting airdrop store state is beneficial. No immediate notes.
85-109: getExistingTokens.
- Reads localStorage or config store for tokens.
- A robust approach to rehydrate tokens from the user’s environment.
- If tokens are invalid, you gracefully do nothing. Good default path.
131-158: setAirdropTokens.
- A single entry point for setting or clearing tokens.
- Good to see the tokens also updated in
appConfigStore.- If any of these steps fail, the store is cleared, so double-check that this is the intended behavior for partial errors (like a refresh token failing but an access token still valid).
160-180: Updaters for various store states.
Each helper function updates a relevant piece of the store. Straightforward usage. Good practice to keep them dedicated and self-documenting.src/containers/floating/Settings/components/OpenSettingsButton.tsx (1)
2-2: LGTM! Clean refactor of state management.The direct import of
setIsSettingsOpenfrom store actions aligns with the PR's objective to move store actions out of Zustand stores, making the code more maintainable.Also applies to: 7-7
src/hooks/useListenForCriticalProblem.tsx (1)
2-4: LGTM! Clean refactor of state management.The direct import of
setCriticalProblemfrom store actions aligns with the PR's objective. The hook maintains proper cleanup and event handling.Also applies to: 8-10
src/components/AdminUI/groups/ThemeGroup.tsx (1)
3-4: LGTM! Clean refactor of theme management.The changes align with the PR objectives by moving store actions out of Zustand stores. The component's functionality remains unchanged while improving code organization through centralized state management.
Also applies to: 7-7, 13-13, 16-16
src/hooks/app/useListenForExternalDependencies.ts (1)
1-2: LGTM! Clean refactor of import paths.The changes align with the PR objectives by moving store actions out of Zustand stores. The hook's functionality remains unchanged while improving code organization through centralized state management.
src/hooks/helpers/useDetectMode.ts (1)
5-5: LGTM! Clean refactor of theme management.The changes align with the PR objectives by moving store actions out of Zustand stores. The hook's functionality remains unchanged while improving code organization through centralized state management.
Also applies to: 16-16
src/store/useUIStore.ts (3)
2-2: LGTM!The import statement is correctly formatted and uses a relative path.
7-9: LGTM! Type definitions are clear and well-structured.The types are well-defined:
DialogTypenow properly handles the null caseAdminShowtype clearly specifies all possible values
11-21: LGTM! Interface is well-structured.The
UIStoreStateinterface is properly defined with clear types for all properties.src/containers/floating/Settings/components/LanguageDropdown.tsx (1)
6-7: LGTM! Imports align with PR objectives.The changes properly implement direct store action imports and add React's memo for optimization.
src/containers/main/Airdrop/AirdropGiftTracker/AirdropGiftTracker.tsx (1)
17-17: LGTM! Efficient store selector implementation.The change to use a selector function instead of destructuring improves performance by preventing unnecessary re-renders.
src/hooks/app/useListenForAppUpdated.ts (2)
3-3: LGTM! Direct store action imports.The change to import actions directly from the store aligns with the PR's objective of moving actions out of Zustand stores.
35-35: LGTM! Simplified dependencies array.The dependencies array correctly includes only
triggerEffectsince the store actions are now stable references imported from the centralized store.src/containers/floating/Settings/sections/general/AudioSettings.tsx (1)
12-12:✅ Verification successful
LGTM! Changes align with PR objectives.
The refactoring to import
setAudioEnableddirectly from the store and simplify state access improves code organization by separating actions from the store.Let's verify that the store action is properly exported:
Also applies to: 17-17, 28-28
🏁 Script executed:
#!/bin/bash # Description: Verify that setAudioEnabled is properly exported from the store. rg -A 2 "export.*setAudioEnabled" src/store/Length of output: 371
LGTM! Verified that
setAudioEnabledis correctly exported fromsrc/store/actions/appConfigStoreActions.ts.The refactoring to import
setAudioEnableddirectly from the store and simplify state access not only improves code organization by separating actions from state management but also aligns with the PR objectives. This has been confirmed by verifying the export via our grep search. The same adjustments appropriately apply to lines 17 and 28.src/App/AppEffects.tsx (1)
3-8: LGTM! Import organization improves code maintainability.The changes to use relative imports and group them by functionality enhance code organization without affecting the component's behavior.
src/containers/floating/ReleaseNotesDialog/ReleaseNotesDialog.tsx (1)
7-7: LGTM! Empty dependency array is correct for useCallback.The imported
setDialogToShowaction is stable, so the empty dependency array inuseCallbackis appropriate.Also applies to: 14-16
src/containers/floating/Settings/sections/general/AutoUpdate.tsx (1)
12-12: LGTM! Changes align with store action refactoring.The refactoring to import
setAutoUpdatedirectly from the store instead of accessing it throughuseAppConfigStorealigns with the PR's objective of moving store actions out of Zustand stores.Also applies to: 28-28
src/hooks/airdrop/stateHelpers/useAirdropUserPointsListener.ts (1)
11-20:❓ Verification inconclusive
Verify dependency array completeness.
The dependency array only includes
currentReferralData?.count, but the function might need to react to other changes incurrentReferralData. Please verify if other fields should trigger a callback update.
🏁 Script executed:
#!/bin/bash # Search for uses of currentReferralData fields rg -A 5 "currentReferralData\." src/Length of output: 38
Dependency Array Verification:
The callback currently only depends oncurrentReferralData?.count, and our search didn't reveal any additional usage of other properties fromcurrentReferralDatawithin this file. However, since the callback’s intent might change over time—and especially given the existing TODO regarding the listener removal—please manually verify that no other fields fromcurrentReferralDatashould trigger an update. If future logic requires comparing additional properties, the dependency array may need to be updated accordingly.src/containers/floating/Settings/sections/experimental/ExperimentalWarning.tsx (1)
7-7: LGTM! Clean refactor of state management.The changes effectively separate state updates from state access, improving code organization and maintainability. The direct import of
setShowExperimentalSettingsaligns with the PR's objective of moving store actions out of Zustand stores.Also applies to: 27-30
src/containers/floating/Settings/sections/mining/MineOnStartMarkup.tsx (1)
12-12: LGTM! Consistent application of state management pattern.The changes maintain the same clean pattern of separating state updates from state access, demonstrating consistent implementation across components.
Also applies to: 16-16, 30-30
src/store/useP2poolStatsStore.ts (1)
5-7: LGTM! Clean separation of store state from actions.The store definition is now focused solely on state management, with actions moved to separate exports. This improves code organization and maintainability.
Also applies to: 27-29
src/containers/floating/Settings/sections/general/StartApplicationOnBootSettings.tsx (1)
12-12: LGTM! Maintains consistent state management pattern.The changes follow the established pattern of separating state updates from state access, demonstrating a consistent approach across the codebase.
Also applies to: 16-16, 30-30
src/containers/floating/Settings/components/ThemeSelector.tsx (1)
6-6: LGTM! Clean refactor of theme management.The changes align with the PR objectives by moving store actions out of Zustand stores. The
handleChangecallback is now simpler and more maintainable.Also applies to: 19-24
src/containers/floating/Settings/sections/general/AirdropPermissionSettings.tsx (1)
13-13: LGTM! Clean refactor of telemetry settings.The changes align with the PR objectives by:
- Moving store actions out of Zustand stores
- Simplifying the dependency array
Also applies to: 17-17, 19-21
src/containers/floating/Settings/sections/general/LanguageSettings.tsx (1)
14-14: LGTM! Good refactoring of store actions.The changes improve code organization by:
- Importing the action directly from the store
- Simplifying the hook to only retrieve state
- Using the imported action in the event handler
Also applies to: 18-18, 34-34
src/containers/main/Dashboard/components/VisualMode.tsx (1)
15-15: LGTM! Good refactoring of store actions.The changes improve code organization by importing the action directly from the store and removing it from the dependency array.
Also applies to: 32-32
src/hooks/app/useSetUp.ts (1)
3-12: LGTM! Good centralization of store actions.The changes improve code organization by importing all actions from a centralized location.
src/containers/floating/Settings/sections/general/PreReleaseSettings.tsx (1)
14-14: LGTM! Improved state management and async handling.The changes enhance the code by:
- Centralizing state management through direct imports
- Properly handling async state updates
- Ensuring dialog closes only after the state update completes
Also applies to: 18-18, 29-33
src/hooks/airdrop/ws/useHandleWsUserIdEvent.ts (2)
4-4: LGTM! Clean refactoring of store actions.The changes align with the PR objectives by moving store actions out of dependency arrays and simplifying the imports.
Also applies to: 10-10
16-16:❓ Verification inconclusive
Verify the function name change impact.
The function name has been updated from
setFlaretosetFlareAnimationType. Please ensure all other components using this function have been updated accordingly.
🏁 Script executed:
#!/bin/bash # Search for any remaining usage of old function name rg "setFlare\(" --type ts --type tsx # Search for usage of new function name rg "setFlareAnimationType\(" --type ts --type tsxLength of output: 146
Action Required: Manual Verification of Function Name Replacement
The function name has been updated to
setFlareAnimationType, but the automated search usingrgwith the--type tsxflag produced errors. To ensure that there are no remaining references to the oldsetFlarefunction, please run an updated search manually with correct file type specifications. For example, try the following commands:#!/bin/bash echo "Searching for occurrences of the old function name 'setFlare(':" rg -g '*.{ts,tsx}' "setFlare\(" . echo "Searching for occurrences of the new function name 'setFlareAnimationType(':" rg -g '*.{ts,tsx}' "setFlareAnimationType\(" .Please verify that:
- All references to the old function (
setFlare) have been removed.- All components consistently use the new function name (
setFlareAnimationType).Once these manual checks confirm the updates, we can safely merge this change.
eslint.config.js (1)
1-1: LGTM! Added React compiler plugin.The addition of
eslint-plugin-react-compilerwill help identify broken React rules, aligning with the PR objectives.Also applies to: 66-66
src/containers/floating/Settings/sections/experimental/AppVersions.tsx (1)
12-12: LGTM! Clean refactoring of store actions and event handlers.The changes align with the PR objectives:
- Store actions are moved out and imported directly
- Event handlers properly wrap the action calls in arrow functions
Also applies to: 25-26, 29-30
src/containers/floating/Settings/SettingsModal.tsx (2)
26-26: LGTM! Clean refactoring of store actions.The changes align with the PR objectives by moving store actions out and importing them directly.
Also applies to: 31-31
46-51:❓ Verification inconclusive
Verify state management in onOpenChange.
The state management logic looks correct, but let's verify that:
- The section reset works correctly when closing
- The settings toggle works as expected
🏁 Script executed:
#!/bin/bash # Search for other components that might be affected by this state management rg "setIsSettingsOpen\(" --type ts --type tsx -A 3Length of output: 80
Below is an updated review comment. Since the previous automated check ran into issues with file type recognition, please rerun the verification manually (or use a modified script that excludes file type filtering) to ensure that the active section is correctly reset and that the settings toggle behaves as expected.
Manual Verification Required: Confirm onOpenChange State Logic
The current implementation in
src/containers/floating/Settings/SettingsModal.tsx— where the function resets the active section usingsetActiveSection(SETTINGS_TYPES[0])when the settings modal is open, and then toggles the modal state — appears correct. However, please manually verify that:
- The active section is only reset when closing the modal.
- The toggle state (via
setIsSettingsOpen) updates as intended, without unintended side effects in related components.Note: The initial search command failed due to an error with the
tsxfile type. You might rerun the search without file type restrictions, for example:rg "setIsSettingsOpen\(" -A 3 src/containers/floating/Settings/SettingsModal.tsxto confirm there are no unintended interactions elsewhere.
src/components/AdminUI/groups/DialogsGroup.tsx (1)
5-10: LGTM! Good refactoring of state management.The changes improve code organization by:
- Centralizing state management logic in dedicated action files
- Using selector functions to optimize re-renders
- Making the component more focused on rendering
Also applies to: 14-21
src/App/App.tsx (1)
68-71:❓ Verification inconclusive
Verify the impact of removing Sentry error tracking.
The change replaces Sentry error tracking with console.error. While this simplifies the code, it might impact our ability to monitor and debug production issues.
🌐 Web query:
What are the best practices for error tracking in production React applications?💡 Result:
Effective error tracking in production React applications requires a combination of strategic error boundaries, robust logging integrations, and proactive monitoring tools. Here are the best practices based on current industry standards:
1. Use Error Boundaries Strategically
- Wrap critical components (e.g., data-heavy sections, third-party integrations) with React Error Boundaries to isolate failures[1][3].
- Log errors within
componentDidCatchor React 19’sonCaughtError/onUncaughtErrorhooks to capture stack traces and component context[1][7]. Example:class ErrorBoundary extends React.Component { componentDidCatch(error, info) { logErrorToService(error, info.componentStack); // Send to Sentry/LogRocket[2][8] } }
2. Integrate Dedicated Error Monitoring Services
- Adopt tools like Sentry, LogRocket, or Bugfender for real-time error tracking:
- Automatically capture unhandled exceptions, network failures, and UI state[5][7].
- Include user context (browser/OS, actions leading to errors) for faster debugging[5][8].
- Use service-specific SDKs for seamless integration:
import * as Sentry from "@sentry/react"; Sentry.init({ dsn: "YOUR_DSN" }); // Initialize in root component[4][8]
3. Log Errors with Context
- Capture metadata such as:
- Timestamp and route (
window.location.pathname)[2].- Component stack traces and Redux state (if applicable)[4][8].
- Filter logs by environment: Only log in production to avoid noise[2][4].
4. Handle Asynchronous Errors Gracefully
- Wrap API calls in
try/catchblocks and log failures:try { const data = await fetchData(); } catch (error) { logAPIError(error); // Include request parameters/headers[4][7] }- Use libraries like
react-queryorSWRfor built-in error handling in data-fetching[6][8].
5. Provide User-Friendly Feedback
- Display fallback UIs (e.g., “Something went wrong – please refresh”) instead of blank screens[1][3].
- Avoid technical jargon and offer actionable steps (e.g., retry buttons)[3][10].
6. Monitor Performance Impact
- Ensure logging does not degrade app performance by:
- Using background queues for error reporting[5].
- Sampling high-traffic errors (e.g., Sentry’s
tracesSampleRate)[7].
7. Test Error Scenarios Proactively
- Simulate network failures/edge cases during development[6][9].
- Conduct A/B testing to validate error UIs under real-world conditions[10].
By combining these practices, teams can quickly identify, diagnose, and resolve issues while maintaining a seamless user experience. Tools like Sentry or Bugfender streamline this process by automating error aggregation and providing actionable insights[5][7][8].
Citations:
- 1: https://refine.dev/blog/react-error-boundaries/
- 2: https://dev.to/kipyegonline/tracking-react-apps-production-errors-using-google-analytics-and-react-error-boundary-35o6
- 3: https://blog.pixelfreestudio.com/graceful-error-handling-in-react-applications/
- 4: https://www.loggly.com/blog/best-practices-for-client-side-logging-and-error-handling-in-react/
- 5: https://bugfender.com/platforms/react/
- 6: https://javascript.plainenglish.io/mastering-error-handling-for-react-in-2025-best-practices-and-examples-781180b5d023
- 7: https://blog.sentry.io/guide-to-error-and-exception-handling-in-react/
- 8: https://www.zipy.ai/framework/reactjs
- 9: https://dev.to/adampaul/error-handling-in-react-applications-best-practices-for-debugging-22ej
- 10: https://uxcam.com/blog/react-error-handling-best-practices/
Action Required: Assess Error Tracking Strategy in Production
In
src/App/App.tsx(Lines 68-71), Sentry error tracking has been removed and replaced with aconsole.errorcall. While this simplifies the code, best practices for production React applications strongly recommend using dedicated error monitoring (via Sentry, LogRocket, etc.) combined with error boundaries. Relying solely on console logging may limit our ability to capture contextual error data (stack traces, user context, etc.) and adequately support debugging in a production environment.
- Revisit the removal of Sentry: Ensure that if Sentry is being removed, a robust alternative exists (e.g., using error boundaries that log to a dedicated monitoring service).
- Assess production impact: Verify that the fallback (console logging) meets our operational requirements for error tracking and debugging.
src/hooks/airdrop/stateHelpers/useGetSosReferrals.ts (1)
15-22: LGTM! Good refactoring of request handling.The change to use
handleAirdropRequestimproves consistency in how API requests are handled across the application.Also applies to: 37-40
src/containers/floating/Settings/sections/general/LogsSettings.tsx (1)
21-21: LGTM! Good refactoring of state management.The changes improve code organization by using centralized actions and explicit callback wrappers.
Also applies to: 68-68
src/store/useShellOfSecretsStore.ts (1)
56-86: LGTM! Clean implementation of the store with proper error handling.The store implementation and error handling in
registerWsConnectionEventlook good. The error state is properly managed with optional chaining and fallbacks.src/containers/floating/Settings/sections/p2p/P2PoolStats.tsx (1)
10-10: LGTM! Good practice using centralized actions.The direct import of actions from the store follows the centralized state management pattern, making the code more maintainable and easier to test.
src/containers/main/SideBar/Miner/components/ModeSelect.tsx (1)
19-19: LGTM! Good use of memo for performance optimization.The memo implementation will prevent unnecessary re-renders of the ModeSelect component when parent components update.
src/components/dialogs/RestartDialog.tsx (1)
16-16: LGTM! State management refactoring looks good.The changes align with the PR's objective of moving store actions out. The direct import of
setDialogToShowand reordering of state declarations improve code organization.Also applies to: 20-21, 27-28
src/components/dialogs/SendLogsDialog.tsx (1)
14-14: LGTM! State management refactoring looks good.The direct import of
setDialogToShowand simplified dependency array align with the PR's objectives.Also applies to: 26-32
src/components/ToastStack/Toast/Toast.tsx (1)
4-4: LGTM! State management refactoring looks good.The direct import of
removeToastaligns with the PR's objectives.Also applies to: 28-31
src/containers/floating/ExternalDependenciesDialog/ExternalDependencyCard.tsx (2)
13-13: LGTM! Improved state management approach.The change to import store actions directly aligns with the PR's objective of centralizing state management and reducing the use of local hooks.
48-48:❓ Verification inconclusive
Verify dependency array completeness.
The dependency array includes
freeInstallationSlot,missingDependency, andoccupyInstallationSlot, but excludesfetchExternalDependenciesandsetError. While this aligns with the new store action approach, verify that the component behaves correctly with these dependencies removed.
🏁 Script executed:
#!/bin/bash # Search for any similar patterns in the codebase where these functions might be included in dependency arrays rg -A 2 "fetchExternalDependencies.*\]|setError.*\]" --type ts --type tsxLength of output: 103
I'll re-run the search using an updated command that properly recognizes TSX files so that we can confirm whether similar dependency arrays elsewhere include the functions in question. Please run the following script and share its output:
Once we have the output from this command, we can definitively verify that omitting these two functions from the dependency array aligns with the new store action approach across the codebase. Until then, please double-check manually that the component behaves correctly without including
fetchExternalDependenciesandsetError.
🏁 Script executed:
#!/bin/bash # Re-run search for patterns where `fetchExternalDependencies` or `setError` might be included in dependency arrays rg --type-add 'tsx:*.tsx' -A 2 'fetchExternalDependencies.*\]|setError.*\]' -g '*.{ts,tsx}'Length of output: 364
Dependency Array Verification: Confirming Store Action Adjustments
The dependency array for
ExternalDependencyCard.tsxnow only includesfreeInstallationSlot,missingDependency, andoccupyInstallationSlot. Our repository search (including inSendLogsDialog.tsx) did not reveal any common patterns that introducefetchExternalDependenciesorsetErrorinto such dependency arrays. This aligns with the updated approach using store actions. However, please manually verify that excluding these dependencies does not impact the component's behavior.src/containers/floating/AutoUpdateDialog/AutoUpdateDialog.tsx (1)
14-14: LGTM! Consistent state management implementation.The change to import and use
setDialogToShowdirectly from the store aligns with the PR's objective of centralizing state management. The implementation is consistent across all usage points in the component.Also applies to: 44-44, 50-50, 68-68
src/containers/floating/Settings/sections/mining/GpuDevices.tsx (1)
1-1: LGTM! Performance optimization through memoization.The addition of
memois a good optimization that prevents unnecessary re-renders when props remain unchanged.Also applies to: 20-20, 86-86
src/containers/floating/CriticalErrorDialog/CriticalErrorDialog.tsx (1)
13-13: LGTM! Clean refactor of store action.The change to import
setDialogToShowdirectly from@app/storealigns well with the PR's objective of moving store actions out of Zustand stores for better maintainability.Also applies to: 18-18
src/store/actions/appStateStoreActions.ts (1)
58-73: Consider adding error handling for mining startup.The
setSetupCompletefunction attempts to start mining without proper error handling, which could silently fail.Consider adding error handling:
export const setSetupComplete = async () => { const visualMode = useAppConfigStore.getState().visual_mode; if (visualMode) { const canvas = document.getElementById('canvas'); if (canvas) { canvas.style.opacity = '1'; setAnimationState('showVisual'); } } // Proceed with auto mining when enabled const { mine_on_app_start, cpu_mining_enabled, gpu_mining_enabled } = useAppConfigStore.getState(); if (mine_on_app_start && (cpu_mining_enabled || gpu_mining_enabled)) { - await startMining(); + try { + await startMining(); + } catch (error) { + console.error('Failed to start mining:', error); + setError('Failed to start mining automatically. Please try starting it manually.', 'system'); + } } useAppStateStore.setState({ setupComplete: true }); };src/containers/floating/Settings/sections/experimental/P2poolMarkup.tsx (2)
20-20: LGTM! Store actions imported directly.The change aligns with the PR's objective to move store actions out of dependency arrays.
52-52: LGTM! Dependency array updated correctly.Removing store actions from the dependency array is safe as they are stable references.
src/types/invoke.ts (1)
79-80: LGTM! Type definition updated for clarity.The explicit object type definition better reflects the backend's expected structure with
refresh_tokeninstead ofrefreshToken.src/store/useBlockchainVisualisationStore.ts (2)
11-11: LGTM! Store action imported directly.The change aligns with the PR's objective to move store actions out of stores.
94-94: LGTM! Store action usage updated consistently.Direct store access replaced with imported action across all functions, maintaining consistent state management.
Also applies to: 106-106, 120-120, 128-128, 136-136, 141-141
src/containers/floating/Settings/sections/experimental/TorMarkup/TorMarkup.tsx (1)
47-47: LGTM! Good change from ref to state.The change from
useReftouseStateforhasCheckedOsis appropriate as it correctly triggers re-renders when the platform check is completed.src/store/actions/appConfigStoreActions.ts (1)
17-33: LGTM! Well-structured initialization function.The
fetchAppConfigfunction properly handles the initialization of app configuration, theme, and canvas visibility.package.json (1)
65-65:✅ Verification successful
Verify the beta version of eslint-plugin-react-compiler.
The plugin version is a beta release with a commit hash. Consider using a stable version or pin to a specific beta release without the commit hash.
🌐 Web query:
What is the latest stable version of eslint-plugin-react-compiler?💡 Result:
As of February 13, 2025, there is no stable release of
eslint-plugin-react-compileryet. The latest available versions are still in Beta, with multiple experimental builds published under@betatags[1][4][6].The most recent Beta versions include:
19.0.0-beta-714736e-20250131(published 4 days ago)[4]19.0.0-beta-decd7b8-20250118[7]19.0.0-beta-a7bf2bd-20241110[2]According to React's official roadmap:
- The compiler is currently in Public Beta phase[6]
- A Release Candidate (RC) will follow once most React apps/libraries confirm compatibility[6]
- Stable Release will come after final community feedback[6]
The React team recommends using Beta versions for testing via:
npm install eslint-plugin-react-compiler@betaor
yarn add eslint-plugin-react-compiler@betaNo official date has been announced for the stable release, but it's expected after sufficient real-world testing of the Beta versions[1][6].
Citations:
- 1: https://www.angularminds.com/blog/react-compiler-release
- 2: https://www.npmjs.com/package/eslint-plugin-react-compiler/v/0.0.0-experimental-63b359f-20241029?activeTab=versions
- 3: https://eslint.org
- 4: https://www.npmjs.com/package/eslint-plugin-react-compiler
- 5: https://github.com/jsx-eslint/eslint-plugin-react
- 6: https://react.dev/blog/2024/10/21/react-compiler-beta-release
- 7: https://security.snyk.io/package/npm/eslint-plugin-react-compiler/19.0.0-beta-0dec889-20241115
- 8: https://classic.yarnpkg.com/en/package/eslint-plugin-react
- 9: https://codesandbox.io/examples/package/eslint-plugin-react-compiler
- 10: https://react.dev/learn/react-compiler
Action Required: Update the beta version reference in package.json
The web query confirms that no stable release of
eslint-plugin-react-compileris available yet, and only beta versions are published. Since using a beta release with a commit hash (e.g.,"^19.0.0-beta-30d8a17-20250209") can lead to non-deterministic behavior, please update the reference as follows:
- Use the npm alias for beta releases (e.g., install via
npm install eslint-plugin-react-compiler@beta) so that the dependency resolves to the officially recommended beta version.- Alternatively, if you need to pin to a specific beta version, remove the commit hash from the version string to ensure consistency (for example, change to something like
"19.0.0-beta"or the appropriate beta version number without the commit hash).This adjustment will align with the React team's recommendations and help maintain a more predictable dependency environment.
| const initialState: UIStoreState = { | ||
| isWebglNotSupported: false, | ||
| theme: 'light', | ||
| background: 'onboarding', | ||
| view: 'setup', | ||
| sidebarOpen: false, | ||
| dialogToShow: null, | ||
| showExperimental: false, | ||
| showExternalDependenciesDialog: false, | ||
| }; | ||
|
|
||
| export const useUIStore = create<UIStoreState>()((set) => ({ | ||
| export const useUIStore = create<UIStoreState>()(() => ({ | ||
| ...initialState, | ||
| setTheme: (theme) => { | ||
| setAnimationProperties(theme === 'light' ? animationLightBg : animationDarkBg); | ||
| set({ theme }); | ||
| }, | ||
| setBackground: (background) => set({ background }), | ||
| setView: (view) => set({ view }), | ||
| setSidebarOpen: (sidebarOpen) => set({ sidebarOpen }), | ||
| setShowExperimental: (showExperimental) => set({ showExperimental }), | ||
| setDialogToShow: (dialogToShow) => set({ dialogToShow }), | ||
| setLatestVersion: (latestVersion) => set({ latestVersion }), | ||
| setIsWebglNotSupported: (isWebglNotSupported) => { | ||
| useAppConfigStore.getState().setVisualMode(false); | ||
| set({ isWebglNotSupported }); | ||
| }, | ||
| setAdminShow: (adminShow) => set({ adminShow }), | ||
| })); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the impact of removing store actions.
The store implementation looks good, with the actions moved out as per PR objectives. However, let's verify that all consumers of the removed actions have been updated.
🏁 Script executed:
#!/bin/bash
# Search for any remaining direct references to the old action methods
rg -l "setTheme|setBackground|setDialogToShow" src/Length of output: 1125
Ensure all consumers of removed store actions are updated
The UI store in src/store/useUIStore.ts is now correctly simplified by moving the actions out. However, running the search command still shows multiple references to setTheme, setBackground, and setDialogToShow across the codebase. Please verify that:
-
These references in files like:
src/store/actions/uiStoreActions.tssrc/store/actions/appConfigStoreActions.tssrc/store/actions/index.ts- Various UI components such as those in
src/components/,src/containers/, andsrc/hooks/
are updated to either use the new action implementations or are intentionally left for backward compatibility.
Addressing these references will prevent any unintended issues stemming from the removal of the original actions.
src/containers/floating/Settings/sections/wallet/MoneroAddressMarkup/MoneroAddressMarkup.tsx
Show resolved
Hide resolved
src/containers/phase/Setup/components/AirdropPermission/AirdropPermission.tsx
Show resolved
Hide resolved
src/containers/floating/Settings/sections/experimental/TorMarkup/TorMarkup.tsx
Show resolved
Hide resolved
| const metricsState = useMiningMetricsStore.getState(); | ||
| const totalGpuDevices = metricsState.gpu_devices.length; | ||
| const excludedDevices = miningState.excludedGpuDevices.length; | ||
| if (metricsState.cpu_mining_status.is_mining || metricsState.cpu_mining_status.is_mining) { |
There was a problem hiding this comment.
Fix copy-paste error in condition.
The condition is checking CPU status twice instead of GPU status.
- if (metricsState.cpu_mining_status.is_mining || metricsState.cpu_mining_status.is_mining) {
+ if (metricsState.cpu_mining_status.is_mining || metricsState.gpu_mining_status.is_mining) {📝 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.
| if (metricsState.cpu_mining_status.is_mining || metricsState.cpu_mining_status.is_mining) { | |
| if (metricsState.cpu_mining_status.is_mining || metricsState.gpu_mining_status.is_mining) { |
There was a problem hiding this comment.
@shanimal08 Thanks! Always happy to help catch these tricky copy-paste issues! 🐰✨
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/store/actions/appConfigStoreActions.ts (2)
232-244: Potential mismatch when reverting display mode
Ifinvoke('set_display_mode', ...)fails, the code revertsdisplay_modein the store toprevThemebut does not reset the UI theme withsetUITheme(...). This could lead to a UI mode mismatch (e.g., displaying dark mode while the store is back to light).invoke('set_display_mode', { displayMode: display_mode as displayMode }).catch((e) => { console.error('Could not set theme', e); setError('Could not change theme'); useAppConfigStore.setState({ display_mode: prevTheme }); + setUITheme(prevTheme); });
1-261: Optional: Unify async/await usage
Across the file, some functions use.then(...).catch(...), while others (likefetchAppConfig) leverageawait/try-catch. Standardizing on a single style can improve readability and consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/store/actions/appConfigStoreActions.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: tauri-build
🔇 Additional comments (2)
src/store/actions/appConfigStoreActions.ts (2)
163-173: Retain custom usage settings on errors
If theinvoke('set_mode', ...)call fails, the function reverts onlymodetoprevModebut not the associatedcustom_max_cpu_usageorcustom_max_gpu_usage. This can leave the store in an inconsistent state when the user sets a custom usage limit alongside a mode.
87-114: Confirm concurrency handling for mining toggles
BothsetCpuMiningEnabledandsetGpuMiningEnabledrely on pausing/stopping/starting mining. Quickly toggling these calls or calling them in parallel may result in race conditions or partial updates (e.g., CPU re-enabled while GPU still paused).Would you like me to provide a shell script to track how often these functions get invoked in short intervals to confirm safe concurrency handling?
Also applies to: 123-154
| export const setCustomStatsServerPort = async (port: number | null) => { | ||
| useAppConfigStore.setState({ p2pool_stats_server_port: port }); | ||
| invoke('set_p2pool_stats_server_port', { port }).catch((e) => { | ||
| console.error('Could not set p2pool stats server port', e); | ||
| setError('Could not change p2pool stats server port'); | ||
| useAppConfigStore.setState({ p2pool_stats_server_port: port }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Revert logic likely incorrect
When the invoke('set_p2pool_stats_server_port', { port }) call fails, the catch block re-sets the store to the same port value instead of restoring the old one. This breaks revert semantics and can leave the user with a non-working port.
export const setCustomStatsServerPort = async (port: number | null) => {
const prevPort = useAppConfigStore.getState().p2pool_stats_server_port;
useAppConfigStore.setState({ p2pool_stats_server_port: port });
invoke('set_p2pool_stats_server_port', { port }).catch((e) => {
console.error('Could not set p2pool stats server port', e);
setError('Could not change p2pool stats server port');
- useAppConfigStore.setState({ p2pool_stats_server_port: port });
+ // Revert to the previous port
+ useAppConfigStore.setState({ p2pool_stats_server_port: prevPort });
});
};📝 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.
| export const setCustomStatsServerPort = async (port: number | null) => { | |
| useAppConfigStore.setState({ p2pool_stats_server_port: port }); | |
| invoke('set_p2pool_stats_server_port', { port }).catch((e) => { | |
| console.error('Could not set p2pool stats server port', e); | |
| setError('Could not change p2pool stats server port'); | |
| useAppConfigStore.setState({ p2pool_stats_server_port: port }); | |
| }); | |
| }; | |
| export const setCustomStatsServerPort = async (port: number | null) => { | |
| const prevPort = useAppConfigStore.getState().p2pool_stats_server_port; | |
| useAppConfigStore.setState({ p2pool_stats_server_port: port }); | |
| invoke('set_p2pool_stats_server_port', { port }).catch((e) => { | |
| console.error('Could not set p2pool stats server port', e); | |
| setError('Could not change p2pool stats server port'); | |
| // Revert to the previous port | |
| useAppConfigStore.setState({ p2pool_stats_server_port: prevPort }); | |
| }); | |
| }; |
| export const setAirdropTokensInConfig = ( | ||
| airdropTokensParam: Pick<AirdropTokens, 'refreshToken' | 'token'> | undefined | ||
| ) => { | ||
| const airdropTokens = airdropTokensParam | ||
| ? { | ||
| token: airdropTokensParam.token, | ||
| refresh_token: airdropTokensParam.refreshToken, | ||
| } | ||
| : undefined; | ||
|
|
||
| invoke('set_airdrop_tokens', { airdropTokens }) | ||
| .then(() => { | ||
| useAppConfigStore.setState({ airdrop_tokens: airdropTokensParam }); | ||
| }) | ||
| .catch((e) => console.error('Failed to store airdrop tokens: ', e)); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider reverting state in error handler for token updates
Currently, the .catch(...) block just logs the error without reverting airdrop_tokens in the store. If the invoke call fails, the store remains updated with new tokens, causing a potential mismatch between local and persisted states.
export const setAirdropTokensInConfig = (
airdropTokensParam: Pick<AirdropTokens, 'refreshToken' | 'token'> | undefined
) => {
...
invoke('set_airdrop_tokens', { airdropTokens })
.then(() => {
useAppConfigStore.setState({ airdrop_tokens: airdropTokensParam });
})
.catch((e) => {
console.error('Failed to store airdrop tokens: ', e);
+ // Consider reverting state here if needed:
+ // useAppConfigStore.setState({ airdrop_tokens: oldTokens });
});
};📝 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.
| export const setAirdropTokensInConfig = ( | |
| airdropTokensParam: Pick<AirdropTokens, 'refreshToken' | 'token'> | undefined | |
| ) => { | |
| const airdropTokens = airdropTokensParam | |
| ? { | |
| token: airdropTokensParam.token, | |
| refresh_token: airdropTokensParam.refreshToken, | |
| } | |
| : undefined; | |
| invoke('set_airdrop_tokens', { airdropTokens }) | |
| .then(() => { | |
| useAppConfigStore.setState({ airdrop_tokens: airdropTokensParam }); | |
| }) | |
| .catch((e) => console.error('Failed to store airdrop tokens: ', e)); | |
| }; | |
| export const setAirdropTokensInConfig = ( | |
| airdropTokensParam: Pick<AirdropTokens, 'refreshToken' | 'token'> | undefined | |
| ) => { | |
| const airdropTokens = airdropTokensParam | |
| ? { | |
| token: airdropTokensParam.token, | |
| refresh_token: airdropTokensParam.refreshToken, | |
| } | |
| : undefined; | |
| invoke('set_airdrop_tokens', { airdropTokens }) | |
| .then(() => { | |
| useAppConfigStore.setState({ airdrop_tokens: airdropTokensParam }); | |
| }) | |
| .catch((e) => { | |
| console.error('Failed to store airdrop tokens: ', e); | |
| // Consider reverting state here if needed: | |
| // useAppConfigStore.setState({ airdrop_tokens: oldTokens }); | |
| }); | |
| }; |
shanimal08
added
dependencies
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/store/actions/appConfigStoreActions.ts (2)
34-49:⚠️ Potential issuePrevious review comment about state reversion is still valid.
The error handler should revert the state if the backend call fails.
export const setAirdropTokensInConfig = ( airdropTokensParam: Pick<AirdropTokens, 'refreshToken' | 'token'> | undefined ) => { + const prevTokens = useAppConfigStore.getState().airdrop_tokens; const airdropTokens = airdropTokensParam ? { token: airdropTokensParam.token, refresh_token: airdropTokensParam.refreshToken, } : undefined; invoke('set_airdrop_tokens', { airdropTokens }) .then(() => { useAppConfigStore.setState({ airdrop_tokens: airdropTokensParam }); }) .catch((e) => { console.error('Failed to store airdrop tokens: ', e); + useAppConfigStore.setState({ airdrop_tokens: prevTokens }); }); };
107-114:⚠️ Potential issuePrevious review comment about incorrect port reversion is still valid.
The error handler incorrectly sets the port to the new value instead of reverting to the previous port.
export const setCustomStatsServerPort = async (port: number | null) => { + const prevPort = useAppConfigStore.getState().p2pool_stats_server_port; useAppConfigStore.setState({ p2pool_stats_server_port: port }); invoke('set_p2pool_stats_server_port', { port }).catch((e) => { console.error('Could not set p2pool stats server port', e); setError('Could not change p2pool stats server port'); - useAppConfigStore.setState({ p2pool_stats_server_port: port }); + useAppConfigStore.setState({ p2pool_stats_server_port: prevPort }); }); };
🧹 Nitpick comments (1)
src/store/actions/appConfigStoreActions.ts (1)
17-33: Consider propagating errors in fetchAppConfig.The function swallows errors instead of propagating them, which could make it difficult for callers to handle failure cases.
export const fetchAppConfig = async () => { try { const appConfig = await invoke('get_app_config'); useAppConfigStore.setState(appConfig); const configTheme = appConfig.display_mode?.toLowerCase(); const canvasElement = document.getElementById('canvas'); if (canvasElement && !appConfig.visual_mode) { canvasElement.style.display = 'none'; } if (configTheme) { await setTheme(configTheme as displayMode); } return appConfig; } catch (e) { console.error('Could not get app config:', e); + throw e; // Propagate error to caller } };
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/store/actions/appConfigStoreActions.ts(1 hunks)src/store/useAppConfigStore.ts(2 hunks)src/store/useBlockchainVisualisationStore.ts(4 hunks)src/types/app-status.ts(1 hunks)src/types/invoke.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/types/app-status.ts
- src/store/useBlockchainVisualisationStore.ts
- src/types/invoke.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cargo checks (fmt, clippy, check)
- GitHub Check: tauri-build
🔇 Additional comments (6)
src/store/actions/appConfigStoreActions.ts (3)
1-16: LGTM! Clean imports and well-defined interface.The imports are organized logically, and the
SetModePropsinterface is well-structured with appropriate optional properties.
115-146: LGTM! Robust mining state management.The implementation handles complex mining states well, including:
- Proper coordination between CPU and GPU mining
- Correct handling of device exclusions
- Appropriate error handling with state reversion
147-252: LGTM! Consistent implementation patterns.The configuration functions demonstrate:
- Consistent error handling with proper state reversion
- Strong typing
- Clear logging
src/store/useAppConfigStore.ts (3)
1-4: LGTM! Good type safety improvements.The use of
Partial<AppConfig>for the store state type improves type safety and maintainability by directly utilizing the application's configuration type.
33-35: LGTM! Good separation of concerns.The simplified store creation, focusing solely on state management without actions, aligns well with the PR's goal of moving store actions out. This separation makes the code more maintainable and easier to test.
31-31:❓ Verification inconclusive
Verify the type and usage of airdrop_tokens.
Please ensure that:
- The
airdrop_tokensfield inAppConfigtype matches its intended usage- The initialization as
undefinedaligns with the airdrop authentication flow requirements
🏁 Script executed:
#!/bin/bash # Description: Check the type definition and usage of airdrop_tokens # Check the type definition echo "Checking AppConfig type definition:" rg -A 5 "airdrop_tokens" "src/types/app-status.ts" # Check usage in airdrop-related code echo -e "\nChecking usage in airdrop-related code:" rg -A 5 "airdrop_tokens" "src/**/airdrop/**"Length of output: 413
Action: Verify the integration of
airdrop_tokenswithin the airdrop authentication flowThe type definition for
airdrop_tokensinAppConfigis correctly defined as an optional field:airdrop_tokens?: { token: string; refreshToken: string; };Its initialization to
undefinedinsrc/store/useAppConfigStore.tsaligns with the intended optional usage. However, our initial check for airdrop-related usage via thesrc/**/airdrop/**path produced no output, which might be due to either an incorrect search path or that the consumption ofairdrop_tokensoccurs elsewhere.Please verify that:
- The
airdrop_tokensfield is eventually utilized by the airdrop authentication flow (or related modules) and that its structure is handled correctly when defined.- The absence of matches in the initially targeted airdrop directories is expected—if not, consider searching the entire repository for any references to "airdrop_tokens" to ensure comprehensive coverage.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/store/actions/index.ts (1)
1-74: Consider adding JSDoc comments for better maintainability.While the code organization is excellent, adding JSDoc comments for each export group would improve maintainability by documenting the purpose and scope of each action group.
Add descriptive comments like this:
+/** + * Airdrop store actions + * Manages airdrop-related state including authentication, tokens, and user details + */ export { airdropSetup, // ... other exports } from './airdropStoreActions.ts'; +/** + * App configuration store actions + * Handles application-wide settings and configurations + */ export { fetchAppConfig, // ... other exports } from './appConfigStoreActions.ts';
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/store/actions/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: tauri-build
- GitHub Check: cargo checks (fmt, clippy, check)
🔇 Additional comments (2)
src/store/actions/index.ts (2)
1-74: Well-organized centralization of store actions!The file effectively centralizes store actions into logical groups, making it easier to manage and import actions across the application. This aligns well with the PR's objective of moving store actions out of Zustand stores.
1-13: Airdrop authentication flow actions are properly structured.The airdrop-related actions align perfectly with the PR's objectives of refactoring the airdrop authentication flow. The actions cover all necessary aspects: token management, user details, and authentication state.
Description
eslint-plugin-react-compilerso we know which react rules we're breakingreact-hooks/exhaustive-depsrule ineslint.config.jsbecause this PR addresses all of them, other than the ones inhooks/airdrop/useWebsocket.ts. to verify you can remove the rule overrides and run lint locally// eslint-disable-next-line react-hooks/exhaustive-depsin a lot of places) OUT of the zustand stores. so they won't be needed in the dep arrays. see Zustand docs [these changes are the majority of the file changes]useFetchAirdropToken) to dedupe the auth logic + interval inLoggedOut&ApplyInviteCodeuseGetAirdropUserDetailshook, and moved all that logic into an action - which is called only on airdrop login, or on startup (via airdropSetup) if there is an airdrop token stored in app_config. (was previously in auseEffectwith a// eslint-disable-next-line react-hooks/exhaustive-deps)useAirdropRequestso it's not longer a hook, but just a request helper functioninvoke- addedsetAirdropTokensInConfigaction for the app config storeNOTE: airdrop WS refactor is being addressed in a separate PR based off this one: shanimal08#7
Motivation and Context
How Has This Been Tested?
What process can a PR reviewer use to test or verify this change?
Summary by CodeRabbit
Summary by CodeRabbit
Refactor
Chores