Rename redux hooks #4101
Rename redux hooks #4101
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
joel-jeremy
force-pushed
the
redux-toolkit
branch
2 times, most recently
from
2601c05 to
630170c
Compare
joel-jeremy
force-pushed
the
rename-redux-hooks
branch
from
0e46aa4 to
612dd8d
Compare
|
What benefit does this provide? Can't we continue using |
This change is separated out from the redux toolkit migration PR: #4000 and the naming is as per the redux toolkit recommendation: https://redux.js.org/usage/usage-with-typescript#define-typed-hooks. For me it also helps reduce cognitive load when coding and using autocomplete. Prevents devs from needing to decide between the untyped |
joel-jeremy
force-pushed
the
redux-toolkit
branch
from
630170c to
8420ae4
Compare
joel-jeremy
force-pushed
the
rename-redux-hooks
branch
2 times, most recently
from
9e6ae74 to
683b2ab
Compare
joel-jeremy
force-pushed
the
rename-redux-hooks
branch
from
683b2ab to
94181f0
Compare
|
Warning Rate limit exceeded@joel-jeremy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 18 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request involves a comprehensive refactoring of Redux hooks across multiple components in the desktop client application. The primary change is the systematic replacement of standard Redux hooks The changes appear to be part of a broader effort to enhance type safety and potentially standardize state management across the application. By introducing application-specific dispatch and selector hooks, the codebase likely gains improved type inference, more consistent state access patterns, and potentially better integration with TypeScript. Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/desktop-client/src/components/manager/subscribe/OpenIdCallback.ts (1)
Error handling needed for token authentication flow
The codebase has established error handling patterns using the Error component and navigation, but they're not utilized in the OpenIdCallback. The review suggestions are valid and should be implemented:
- Add token validation before processing
- Handle network errors in the promise chain
- Navigate to the error route on failure
🔗 Analysis chain
Line range hint
11-15: Consider adding error handling for token processing.The token retrieval and processing lacks error handling for cases where:
- Token is null/undefined
- Network request fails
- Invalid token format
Let's check if error handling exists in related authentication files:
Consider adding error handling:
useEffect(() => { const token = new URLSearchParams(window.location.search).get('token'); + if (!token) { + // Handle missing token case + return; + } send('subscribe-set-token', { token: token as string }).then(() => { dispatch(loggedIn()); + }).catch(error => { + // Handle error case }); });🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for error handling patterns in authentication-related files rg -A 5 "subscribe-set-token" rg -A 5 "loggedIn"Length of output: 7253
Script:
#!/bin/bash # Check for error handling patterns in auth flows rg -A 5 "catch\(" packages/desktop-client/src/components/manager/subscribe/ rg -A 5 "navigate\(" packages/desktop-client/src/components/manager/subscribe/ rg -A 5 "error.*handling" packages/desktop-client/src/components/manager/Length of output: 4712
Script:
#!/bin/bash # Check Error component implementation cat packages/desktop-client/src/components/manager/subscribe/Error.tsx # Check for error boundary components rg -l "ErrorBoundary" packages/desktop-client/src/Length of output: 1472
🧹 Nitpick comments (12)
packages/desktop-client/src/components/manager/ConfigServer.tsx (1)
Line range hint
1-1: Consider removing@ts-strict-ignoreas a follow-up.Now that we're moving towards typed Redux hooks, it would be beneficial to remove the
@ts-strict-ignoredirective and properly type this component. This would further enhance type safety and align with the TypeScript migration efforts.Would you like me to help identify and fix the TypeScript issues in this file as a follow-up PR?
packages/desktop-client/src/components/reports/reports/Spending.tsx (1)
Line range hint
46-63: Consider component decomposition for better maintainability.While outside the scope of this PR, the
SpendingInternalcomponent handles multiple responsibilities (widget management, date comparison, filtering, visualization). Consider breaking it down into smaller, focused components for better maintainability.Example areas for decomposition:
- Date range comparison controls
- Filter controls
- Graph section with its controls
- Widget management section
packages/desktop-client/src/components/manager/ManagementApp.tsx (1)
74-74: LGTM! Typed dispatch implementation.The migration to
useAppDispatchenhances type safety for dispatched actions. Consider creating typed action creators using Redux Toolkit'screateActionorcreateSliceto fully leverage the TypeScript benefits.packages/desktop-client/src/components/UpdateNotification.tsx (1)
17-20: Consider centralizing state selectors.To improve maintainability, consider moving the state selectors to a dedicated selectors file:
// state/selectors/app.ts export const selectUpdateInfo = (state: RootState) => state.app.updateInfo; export const selectShowUpdateNotification = (state: RootState) => state.app.showUpdateNotification;Then import and use them:
const updateInfo = useAppSelector(selectUpdateInfo); const showUpdateNotification = useAppSelector(selectShowUpdateNotification);This would:
- Reduce duplication if these selectors are used elsewhere
- Make it easier to maintain state path changes
- Improve reusability
packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx (3)
147-147: Consider enhancing error handling and validation.The error state management could be improved in the following ways:
- Add validation for the multiplier amount before applying it to transactions
- Provide more descriptive error messages for parsing failures
const dispatch = useAppDispatch(); const [multiplierAmount, setMultiplierAmount] = useState(''); const [loadingState, setLoadingState] = useState('parsing'); -const [error, setError] = useState(null); +const [error, setError] = useState({ parsed: false, message: '', details: null });Also applies to: 149-152
Line range hint
191-270: Consider performance optimizations for transaction preview.The
getImportPreviewfunction performs complex calculations and transformations. Consider these optimizations:
- Memoize expensive calculations using
useMemo- Batch state updates for better performance
- Consider using a virtual list for large transaction sets
Example optimization:
+const memoizedPreviewTransactions = useMemo(() => transactions .filter(trans => !trans.isMatchedTransaction) .reduce((previous, current_trx) => { // ... existing reduction logic }, []), + [transactions, matchedUpdateMap] +);
Line range hint
644-683: Enhance accessibility for better user experience.Consider adding the following accessibility improvements:
- Add ARIA labels to interactive elements
- Improve keyboard navigation
- Add screen reader descriptions for complex interactions
<View style={{ marginTop: 10 }}> <Stack direction="row" align="flex-start" spacing={1} + role="group" + aria-label="Import options" style={{ marginTop: 5 }} >packages/desktop-client/src/components/reports/reports/CustomReportListCards.tsx (1)
Line range hint
102-111: Consider extracting error handling logicThe error notification dispatch could be moved to a custom hook or utility function for reusability across components.
+// Create a new file: src/hooks/useNotification.ts +import { addNotification } from 'loot-core/src/client/actions'; +import { useAppDispatch } from '../redux'; + +export function useNotification() { + const dispatch = useAppDispatch(); + + const showError = (message: string) => { + dispatch(addNotification({ type: 'error', message })); + }; + + return { showError }; +} // In CustomReportListCards.tsx: +const { showError } = useNotification(); const onSaveName = async (name: string) => { const response = await sendCatch('report/update', { ...report, name }); if (response.error) { - dispatch( - addNotification({ - type: 'error', - message: `Failed saving report name: ${response.error.message}`, - }), - ); + showError(`Failed saving report name: ${response.error.message}`); setNameMenuOpen(true); return; }packages/desktop-client/src/components/util/GenericInput.jsx (1)
9-9: Consider updating file extension for TypeScript consistency.While the typed hooks are correctly implemented, consider renaming the file from
.jsxto.tsxto maintain consistency with TypeScript usage across the codebase.Also applies to: 39-39
packages/desktop-client/src/components/mobile/budget/index.tsx (1)
27-27: LGTM! Consider adding a type annotation for better clarity.The migration to
useAppDispatchis correctly implemented. For better code clarity, consider adding a type annotation to the dispatch variable:-const dispatch = useAppDispatch(); +const dispatch = useAppDispatch<AppDispatch>();Also applies to: 58-58
packages/desktop-client/src/components/manager/BudgetList.tsx (2)
501-506: LGTM! Consider adding type annotations for better clarity.The migration to typed hooks is correctly implemented in the BudgetList component.
Consider adding type annotations for better code clarity:
-const allFiles = useAppSelector(state => state.budgets.allFiles || []); +const allFiles = useAppSelector((state: RootState) => state.budgets.allFiles || []); -const userData = useAppSelector(state => state.user.data); +const userData = useAppSelector((state: RootState) => state.user.data);
673-673: LGTM! Consider adding type annotation for selector.The migration to
useAppSelectoris correctly implemented in the UserAccessForFile component.Consider adding a type annotation for better code clarity:
-const allFiles = useAppSelector(state => state.budgets.allFiles || []); +const allFiles = useAppSelector((state: RootState) => state.budgets.allFiles || []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
upcoming-release-notes/4101.mdis excluded by!**/*.md
📒 Files selected for processing (82)
packages/desktop-client/src/auth/AuthProvider.tsx(2 hunks)packages/desktop-client/src/auth/ProtectedRoute.tsx(2 hunks)packages/desktop-client/src/components/App.tsx(3 hunks)packages/desktop-client/src/components/AppBackground.tsx(2 hunks)packages/desktop-client/src/components/BankSyncStatus.tsx(1 hunks)packages/desktop-client/src/components/FinancesApp.tsx(2 hunks)packages/desktop-client/src/components/HelpMenu.tsx(2 hunks)packages/desktop-client/src/components/LoggedInUser.tsx(3 hunks)packages/desktop-client/src/components/ManageRules.tsx(2 hunks)packages/desktop-client/src/components/Modals.tsx(2 hunks)packages/desktop-client/src/components/Notifications.tsx(2 hunks)packages/desktop-client/src/components/Titlebar.tsx(2 hunks)packages/desktop-client/src/components/UpdateNotification.tsx(2 hunks)packages/desktop-client/src/components/accounts/Account.tsx(3 hunks)packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx(2 hunks)packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx(2 hunks)packages/desktop-client/src/components/budget/index.tsx(2 hunks)packages/desktop-client/src/components/manager/BudgetList.tsx(3 hunks)packages/desktop-client/src/components/manager/ConfigServer.tsx(2 hunks)packages/desktop-client/src/components/manager/ManagementApp.tsx(2 hunks)packages/desktop-client/src/components/manager/WelcomeScreen.tsx(2 hunks)packages/desktop-client/src/components/manager/subscribe/Bootstrap.tsx(2 hunks)packages/desktop-client/src/components/manager/subscribe/Login.tsx(2 hunks)packages/desktop-client/src/components/manager/subscribe/OpenIdCallback.ts(1 hunks)packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx(4 hunks)packages/desktop-client/src/components/mobile/accounts/Accounts.tsx(3 hunks)packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx(3 hunks)packages/desktop-client/src/components/mobile/budget/CategoryTransactions.tsx(2 hunks)packages/desktop-client/src/components/mobile/budget/index.tsx(2 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx(4 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx(2 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx(2 hunks)packages/desktop-client/src/components/modals/BudgetListModal.tsx(2 hunks)packages/desktop-client/src/components/modals/CloseAccountModal.tsx(2 hunks)packages/desktop-client/src/components/modals/CoverModal.tsx(2 hunks)packages/desktop-client/src/components/modals/CreateAccountModal.tsx(2 hunks)packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx(2 hunks)packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx(2 hunks)packages/desktop-client/src/components/modals/EditRuleModal.jsx(2 hunks)packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx(2 hunks)packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx(2 hunks)packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx(2 hunks)packages/desktop-client/src/components/modals/LoadBackupModal.tsx(2 hunks)packages/desktop-client/src/components/modals/MergeUnusedPayeesModal.tsx(2 hunks)packages/desktop-client/src/components/modals/OutOfSyncMigrationsModal.tsx(2 hunks)packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx(2 hunks)packages/desktop-client/src/components/modals/TransferModal.tsx(2 hunks)packages/desktop-client/src/components/modals/TransferOwnership.tsx(2 hunks)packages/desktop-client/src/components/modals/manager/ConfirmChangeDocumentDir.tsx(2 hunks)packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx(2 hunks)packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx(2 hunks)packages/desktop-client/src/components/modals/manager/FilesSettingsModal.tsx(3 hunks)packages/desktop-client/src/components/modals/manager/ImportActualModal.tsx(2 hunks)packages/desktop-client/src/components/modals/manager/ImportModal.tsx(2 hunks)packages/desktop-client/src/components/modals/manager/ImportYNAB4Modal.tsx(2 hunks)packages/desktop-client/src/components/modals/manager/ImportYNAB5Modal.tsx(2 hunks)packages/desktop-client/src/components/payees/ManagePayeesWithData.tsx(2 hunks)packages/desktop-client/src/components/reports/Overview.tsx(2 hunks)packages/desktop-client/src/components/reports/reports/Calendar.tsx(2 hunks)packages/desktop-client/src/components/reports/reports/CashFlow.tsx(2 hunks)packages/desktop-client/src/components/reports/reports/CustomReportListCards.tsx(2 hunks)packages/desktop-client/src/components/reports/reports/NetWorth.tsx(2 hunks)packages/desktop-client/src/components/reports/reports/Spending.tsx(2 hunks)packages/desktop-client/src/components/reports/reports/Summary.tsx(2 hunks)packages/desktop-client/src/components/schedules/PostsOfflineNotification.tsx(2 hunks)packages/desktop-client/src/components/schedules/ScheduleDetails.tsx(2 hunks)packages/desktop-client/src/components/schedules/ScheduleLink.tsx(2 hunks)packages/desktop-client/src/components/schedules/index.tsx(2 hunks)packages/desktop-client/src/components/settings/AuthSettings.tsx(2 hunks)packages/desktop-client/src/components/settings/Encryption.tsx(2 hunks)packages/desktop-client/src/components/settings/Reset.tsx(2 hunks)packages/desktop-client/src/components/settings/index.tsx(2 hunks)packages/desktop-client/src/components/sidebar/Account.tsx(2 hunks)packages/desktop-client/src/components/sidebar/Accounts.tsx(2 hunks)packages/desktop-client/src/components/sidebar/BudgetName.tsx(2 hunks)packages/desktop-client/src/components/sidebar/Sidebar.tsx(2 hunks)packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx(2 hunks)packages/desktop-client/src/components/transactions/TransactionList.jsx(2 hunks)packages/desktop-client/src/components/transactions/TransactionMenu.tsx(2 hunks)packages/desktop-client/src/components/transactions/TransactionsTable.jsx(3 hunks)packages/desktop-client/src/components/util/GenericInput.jsx(2 hunks)packages/desktop-client/src/hooks/useAccounts.ts(1 hunks)
⛔ Files not processed due to max files limit (14)
- packages/desktop-client/src/hooks/useActions.ts
- packages/desktop-client/src/hooks/useCategories.ts
- packages/desktop-client/src/hooks/useFailedAccounts.ts
- packages/desktop-client/src/hooks/useGlobalPref.ts
- packages/desktop-client/src/hooks/useMetadataPref.ts
- packages/desktop-client/src/hooks/useModalState.ts
- packages/desktop-client/src/hooks/usePayees.ts
- packages/desktop-client/src/hooks/useSyncServerStatus.ts
- packages/desktop-client/src/hooks/useSyncedPref.ts
- packages/desktop-client/src/hooks/useSyncedPrefs.ts
- packages/desktop-client/src/hooks/useTransactionBatchActions.ts
- packages/desktop-client/src/hooks/useUndo.ts
- packages/desktop-client/src/hooks/useUpdatedAccounts.ts
- packages/desktop-client/src/redux/index.ts
✅ Files skipped from review due to trivial changes (1)
- packages/desktop-client/src/components/transactions/TransactionsTable.jsx
🧰 Additional context used
📓 Learnings (9)
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx (2)
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:261-277
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In React components (TypeScript), avoid using hooks like `useCallback` inside callbacks or nested functions. Hooks must be called at the top level of functional components.
Learnt from: jfdoming
PR: actualbudget/actual#3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
packages/desktop-client/src/components/sidebar/Account.tsx (2)
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-11-10T16:45:31.225Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Learnt from: jfdoming
PR: actualbudget/actual#3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
packages/desktop-client/src/components/schedules/index.tsx (3)
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/schedules/index.tsx:26-31
Timestamp: 2024-11-10T16:45:31.225Z
Learning: In this codebase, ESLint requires `dispatch` from `useDispatch` to be included in the dependency array of `useCallback` hooks, even though `dispatch` is stable.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/schedules/index.tsx:33-35
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In the project's React components, when using `dispatch` from `useDispatch` in `useCallback` or `useEffect` hooks, `dispatch` must be included in the dependency array due to the `react-hooks/exhaustive-deps` ESLint rule.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/schedules/index.tsx:37-39
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In our React function components, we should include `dispatch` in the dependency array of `useCallback` hooks to comply with ESLint rules, even though `dispatch` is a stable function.
packages/desktop-client/src/components/schedules/ScheduleLink.tsx (3)
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/schedules/index.tsx:26-31
Timestamp: 2024-11-10T16:45:31.225Z
Learning: In this codebase, ESLint requires `dispatch` from `useDispatch` to be included in the dependency array of `useCallback` hooks, even though `dispatch` is stable.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/schedules/index.tsx:37-39
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In our React function components, we should include `dispatch` in the dependency array of `useCallback` hooks to comply with ESLint rules, even though `dispatch` is a stable function.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/schedules/index.tsx:33-35
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In the project's React components, when using `dispatch` from `useDispatch` in `useCallback` or `useEffect` hooks, `dispatch` must be included in the dependency array due to the `react-hooks/exhaustive-deps` ESLint rule.
packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx (1)
Learnt from: csenel
PR: actualbudget/actual#3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
packages/desktop-client/src/components/sidebar/Accounts.tsx (2)
Learnt from: tlesicka
PR: actualbudget/actual#3554
File: packages/desktop-client/src/components/sidebar/Accounts.tsx:60-64
Timestamp: 2024-11-10T16:45:31.225Z
Learning: The `onReorder` function in `Accounts.tsx` was moved from `Sidebar.tsx`, and the `targetId` parameter remains typed as `unknown` intentionally.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-11-10T16:45:31.225Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
packages/desktop-client/src/components/mobile/accounts/Accounts.tsx (1)
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-11-10T16:45:31.225Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
packages/desktop-client/src/components/accounts/Account.tsx (2)
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-11-10T16:45:31.225Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Learnt from: jfdoming
PR: actualbudget/actual#3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
packages/desktop-client/src/components/schedules/ScheduleDetails.tsx (3)
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/schedules/index.tsx:26-31
Timestamp: 2024-11-10T16:45:31.225Z
Learning: In this codebase, ESLint requires `dispatch` from `useDispatch` to be included in the dependency array of `useCallback` hooks, even though `dispatch` is stable.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/schedules/index.tsx:37-39
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In our React function components, we should include `dispatch` in the dependency array of `useCallback` hooks to comply with ESLint rules, even though `dispatch` is a stable function.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/schedules/index.tsx:33-35
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In the project's React components, when using `dispatch` from `useDispatch` in `useCallback` or `useEffect` hooks, `dispatch` must be included in the dependency array due to the `react-hooks/exhaustive-deps` ESLint rule.
🔇 Additional comments (114)
packages/desktop-client/src/components/sidebar/BudgetName.tsx (2)
11-11: LGTM! Import statement updated correctly.The change from
useDispatchtouseAppDispatchaligns with the PR objectives and Redux Toolkit recommendations for TypeScript usage.
56-56: LGTM! Hook usage updated correctly.The
useAppDispatchhook is properly instantiated and used consistently within the component.packages/desktop-client/src/components/modals/LoadBackupModal.tsx (2)
9-9: LGTM! Import change aligns with Redux Toolkit recommendations.The change from
useDispatchtouseAppDispatchfollows the Redux Toolkit's TypeScript usage guidelines, providing better type safety.
55-55: LGTM! Hook usage properly updated.The
useAppDispatchhook is correctly used, and all dispatch calls throughout the component are properly typed.packages/desktop-client/src/components/modals/TransferModal.tsx (2)
8-8: LGTM! Import change aligns with Redux Toolkit's TypeScript recommendations.The change from
useDispatchtouseAppDispatchfollows the recommended pattern for better type inference with TypeScript.
58-58: LGTM! Hook usage updated consistently.The dispatch hook usage is updated to match the new typed version, maintaining consistency with the import change.
packages/desktop-client/src/components/schedules/PostsOfflineNotification.tsx (2)
10-10: LGTM! Import statement updated according to Redux Toolkit recommendations.The change from
useDispatchtouseAppDispatchaligns with the PR objectives and follows Redux Toolkit's TypeScript best practices.
23-23: LGTM! Hook usage updated consistently.The
useAppDispatchhook is correctly used and maintains the existing dispatch functionality.packages/desktop-client/src/components/reports/reports/CashFlow.tsx (2)
20-20: LGTM! Import statement updated to use typed dispatch.The change aligns with Redux Toolkit's TypeScript recommendations by importing the typed
useAppDispatchhook.
66-66: LGTM! Hook usage updated consistently.The
useAppDispatchhook is correctly used to obtain the typed dispatch function.packages/desktop-client/src/components/modals/OutOfSyncMigrationsModal.tsx (2)
6-6: LGTM! Import change aligns with Redux Toolkit recommendations.The change from
useDispatchtouseAppDispatchfollows Redux Toolkit's TypeScript best practices, providing better type inference for the dispatch function.
15-15: LGTM! Hook usage updated consistently.The dispatch hook usage is correctly updated to match the new import, maintaining functionality while gaining improved type safety for dispatched actions.
packages/desktop-client/src/components/modals/EditRuleModal.jsx (2)
39-39: LGTM! Import change aligns with Redux Toolkit recommendations.The change from
useDispatchtouseAppDispatchfollows Redux Toolkit's TypeScript usage guidelines.
787-787: LGTM! Hook usage updated consistently.The
useDispatchhook has been correctly replaced withuseAppDispatch, maintaining the same functionality while providing better TypeScript support.packages/desktop-client/src/components/settings/AuthSettings.tsx (2)
7-7: LGTM! Import change aligns with Redux Toolkit recommendations.The change from
useDispatchtouseAppDispatchfollows the Redux Toolkit's TypeScript usage guidelines, providing better type safety.
21-21: LGTM! Hook usage updated consistently.The dispatch hook usage is updated to match the new import, maintaining consistency throughout the component.
packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx (2)
12-12: LGTM! Import statement updated correctly.The change from
useDispatchtouseAppDispatchaligns with the Redux Toolkit recommendations for TypeScript usage.
44-44: LGTM! Hook usage updated consistently.The
dispatchinitialization has been correctly updated to useuseAppDispatch, maintaining consistency with the import change.packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx (3)
57-57: LGTM! Import statement updated correctly.The import statement has been updated to use the new hook names
useAppDispatchanduseAppSelectorfrom the redux folder, aligning with the PR objectives.
454-454: LGTM! useAppDispatch implementation is correct.The
useDispatchhook has been correctly replaced withuseAppDispatch, maintaining the existing dispatch functionality for modal actions.
1249-1251: LGTM! useAppSelector implementation is correct.The
useSelectorhook has been properly replaced withuseAppSelectorfor selecting the lastTransaction from the state.packages/desktop-client/src/components/modals/manager/ImportYNAB5Modal.tsx (2)
8-8: LGTM! Import change aligns with Redux Toolkit recommendations.The change from
useDispatchtouseAppDispatchfollows the Redux Toolkit's TypeScript best practices.
31-31: LGTM! Improved type safety for Redux dispatch.The switch to
useAppDispatchenhances type checking for dispatched actions likeimportBudget, making it easier to catch potential type errors during development.packages/desktop-client/src/components/modals/MergeUnusedPayeesModal.tsx (3)
9-9: LGTM! Import statement updated correctly.The import statement has been updated to use the new typed hooks
useAppSelectoranduseAppDispatchfrom the local redux folder, aligning with the PR objectives and Redux Toolkit recommendations.
31-31: LGTM! Selector hook updated correctly.The
useSelectorhook has been properly replaced withuseAppSelectorfor accessing the modal stack state, maintaining type safety.
33-33: LGTM! Dispatch hook updated correctly.The
useDispatchhook has been properly replaced withuseAppDispatchfor dispatching actions, ensuring type safety for Redux actions.packages/desktop-client/src/components/manager/ConfigServer.tsx (1)
13-13: LGTM! Changes align with Redux Toolkit's TypeScript recommendations.The replacement of
useDispatchwithuseAppDispatchfollows the recommended pattern for typed Redux hooks, which will improve type safety and developer experience.Also applies to: 26-26
packages/desktop-client/src/components/transactions/TransactionList.jsx (1)
15-15: LGTM! Hook renaming implemented correctly.The Redux hook has been properly renamed from
useDispatchtouseAppDispatchas per Redux Toolkit's TypeScript recommendations. The change is consistent with the PR objectives and maintains the component's functionality.Also applies to: 101-101
packages/desktop-client/src/components/mobile/transactions/TransactionList.jsx (1)
29-29: LGTM! Hook renaming implemented correctly.The Redux hook has been properly renamed from
useDispatchtouseAppDispatchas per Redux Toolkit's TypeScript recommendations. The change is consistent across the mobile implementation and maintains the notification inset management functionality.Also applies to: 249-249
packages/desktop-client/src/components/reports/reports/Spending.tsx (2)
17-17: LGTM! Import change aligns with Redux Toolkit's TypeScript recommendations.The change from
useDispatchtouseAppDispatchfollows the Redux Toolkit's best practices for TypeScript usage, providing better type inference for the dispatch function.
63-63: LGTM! Proper usage of the typed dispatch hook.The typed dispatch hook is correctly implemented and used for dispatching notifications in the
onSaveWidgetfunction.packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx (3)
40-40: LGTM! Import statement aligns with Redux Toolkit's TypeScript recommendations.The change from importing
useDispatchanduseSelectorto importinguseAppDispatchanduseAppSelectorfrom the local Redux folder follows best practices for TypeScript usage with Redux.
100-102: LGTM! Proper usage of typed Redux hooks.The implementation correctly:
- Uses
useAppSelectorfor type-safe state access- Uses
useAppDispatchat the component's top level- Maintains proper memoization in callbacks
Also applies to: 112-112
256-256: LGTM! Proper hook placement and usage.The implementation correctly:
- Places
useAppDispatchat the component's top level- Maintains proper memoization in callbacks
- Follows best practices for hooks usage
packages/desktop-client/src/components/manager/ManagementApp.tsx (2)
9-9: LGTM! Import of typed Redux hooks follows best practices.The change aligns with Redux Toolkit's TypeScript recommendations by importing typed hooks from a local Redux folder.
65-67: LGTM! Consistent usage of typed selectors.The migration to
useAppSelectoris applied consistently across all state selections, improving type safety while maintaining clear state access patterns.Also applies to: 70-72
packages/desktop-client/src/components/UpdateNotification.tsx (3)
7-7: LGTM! Import of typed Redux hooks.The change to import typed hooks from the local Redux setup aligns with Redux Toolkit's TypeScript recommendations.
17-20: LGTM! Consistent usage of typed selectors.The replacement with
useAppSelectormaintains the same functionality while adding type safety. The implementation is consistent across both state selections.
22-22: LGTM! Typed dispatch implementation.The change to
useAppDispatchenhances type safety for dispatched actions while maintaining the same functionality.packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx (1)
16-16: LGTM! Hook replacement aligns with Redux Toolkit TypeScript recommendations.The change from
useDispatchtouseAppDispatchfollows the Redux Toolkit's TypeScript usage guidelines, ensuring proper typing of the dispatch function.packages/desktop-client/src/components/modals/manager/ImportModal.tsx (2)
6-6: LGTM! Import statement updated correctly.The change from
useDispatchtouseAppDispatchaligns with Redux Toolkit's TypeScript recommendations.
26-26: LGTM! Hook usage updated correctly.The dispatch hook has been properly updated to use the typed version while maintaining the existing functionality.
Let's verify that all dispatch calls in the component are properly typed:
✅ Verification successful
Verification successful: All dispatch calls are properly typed
- Hook correctly updated to
useAppDispatch- All dispatch calls use the typed dispatch consistently with
pushModalaction creator- String literal arguments ensure proper type inference
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the component only uses typed dispatch # and that the action types are properly inferred # Check for any remaining untyped useDispatch usage rg "useDispatch" "packages/desktop-client/src/components/modals/manager/ImportModal.tsx" # Verify the action type for pushModal ast-grep --pattern 'pushModal($$$)' "packages/desktop-client/src/components/modals/manager/ImportModal.tsx"Length of output: 549
packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx (2)
11-11: LGTM! Hook changes align with Redux Toolkit best practices.The changes from
useDispatchtouseAppDispatchfollow the Redux Toolkit's TypeScript recommendations, providing better type safety for dispatched actions.Also applies to: 34-34
Line range hint
1-1: Consider enabling TypeScript strict mode.The
@ts-strict-ignorecomment suggests TypeScript strict checks are disabled. Consider addressing any type issues to enable strict mode, which would provide better type safety alongside the newly typed Redux hooks.Let's check if other files in the codebase have strict mode enabled:
packages/desktop-client/src/components/modals/manager/ImportYNAB4Modal.tsx (3)
8-8: LGTM! Import statement updated correctly.The change aligns with the PR objective to use typed Redux hooks as recommended by Redux Toolkit.
28-28: LGTM! Hook usage updated consistently.The
useAppDispatchhook is correctly used to replaceuseDispatch, maintaining consistency with the import change.
Line range hint
1-1: Consider removing@ts-strict-ignorecomment.Since we're improving TypeScript support by using typed Redux hooks, this might be a good opportunity to address any TypeScript strict mode issues in this file.
Let's check if there are any patterns of strict mode ignores across the codebase:
packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx (2)
7-7: LGTM! Import change aligns with Redux Toolkit recommendations.The change from
useDispatchtouseAppDispatchfollows the Redux Toolkit's TypeScript usage guidelines.
25-25: LGTM! Consistent usage of the renamed hook.The hook usage is correctly updated to match the new import name while maintaining the same functionality.
Let's verify that all dispatch usages in this file are using the new hook:
✅ Verification successful
Verification successful: All dispatch-related code is using the new hook correctly
- Import statement uses
useAppDispatch- Hook initialization is correct
- Both dispatch calls in the component use the properly initialized dispatch
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify no instances of the old useDispatch hook remain rg "useDispatch" "packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx"Length of output: 90
Script:
#!/bin/bash # Get the full content of the file cat "packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx" # Also search for any dispatch-related patterns to be thorough rg -i "dispatch" "packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx"Length of output: 5549
packages/desktop-client/src/components/modals/BudgetListModal.tsx (2)
5-5: LGTM! Import statement updated correctly.The change from
useSelectortouseAppSelectoraligns with the PR objectives to use typed Redux hooks as recommended by Redux Toolkit.
14-16: LGTM! Hook usage updated correctly.The selector logic remains unchanged while benefiting from improved type safety through
useAppSelector.Let's verify that all Redux hook imports in this file have been updated consistently:
✅ Verification successful
Verification confirms proper Redux hook migration
- ✓ No untyped Redux hooks from 'react-redux'
- ✓ Using typed
useAppSelectorfrom local redux module- ✓ Consistent hook usage throughout the file
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining untyped Redux hooks # Expected: No instances of useSelector or useDispatch from react-redux rg --type typescript "import.*{.*use(Selector|Dispatch).*}.*from ['\"]react-redux['\"]" "packages/desktop-client/src/components/modals/BudgetListModal.tsx"Length of output: 313
Script:
#!/bin/bash # Check imports and usage patterns in the file # 1. Check for any react-redux hook imports echo "=== Checking react-redux hook imports ===" rg "from 'react-redux'" "packages/desktop-client/src/components/modals/BudgetListModal.tsx" # 2. Check for typed hook imports and all hook usages echo -e "\n=== Checking hook imports and usage patterns ===" rg "use(App)?(Selector|Dispatch)" "packages/desktop-client/src/components/modals/BudgetListModal.tsx"Length of output: 497
packages/desktop-client/src/components/modals/CoverModal.tsx (2)
8-8: LGTM! Import change aligns with Redux Toolkit recommendations.The change from
useDispatchtouseAppDispatchfollows the Redux Toolkit's TypeScript usage guidelines, improving type safety.
52-52: LGTM! Verify typed dispatch implementation.The change to
useAppDispatchensures type safety for dispatched actions. This is particularly beneficial for the modal actions being dispatched inonCategoryClick.Let's verify the typing implementation:
✅ Verification successful
Type-safe dispatch implementation verified ✓
The
useAppDispatchhook is properly typed using TypeScript generics withAppDispatch, ensuring compile-time type safety for all dispatched actions, including modal operations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the typing implementation for useAppDispatch and modal actions # Check the Redux store setup and typing ast-grep --pattern 'export const useAppDispatch = $$$' # Check modal action types ast-grep --pattern 'pushModal($$$)'Length of output: 64229
packages/desktop-client/src/components/AppBackground.tsx (2)
7-7: LGTM! Import change aligns with Redux Toolkit recommendations.The change from importing
useSelectorto importing the typeduseAppSelectorfrom the local Redux implementation follows the Redux Toolkit's TypeScript best practices.
19-19: LGTM! Hook usage updated correctly.The
useSelectorhook has been properly replaced withuseAppSelectorwhile maintaining the same selector logic.Let's verify the typed hook implementation:
✅ Verification successful
Verified: useAppSelector is properly implemented and typed
The hook is correctly implemented with proper TypeScript typing using RootState, and there's only one implementation ensuring consistency across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of useAppSelector in the Redux folder # and ensure it's properly typed with RootState # Check the implementation of useAppSelector ast-grep --pattern 'export const useAppSelector = $$$' # Check if RootState type is used in the implementation rg -l "RootState.*useAppSelector" packages/desktop-client/src/redux/Length of output: 3225
Script:
#!/bin/bash # Verify there's only one implementation of useAppSelector rg -l "useAppSelector.*=.*useSelector" # Check the RootState type definition ast-grep --pattern 'export type RootState = $$$'Length of output: 241
packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (4)
33-33: LGTM: Import of typed Redux hook.The change to import
useAppDispatchaligns with the PR objective to use typed Redux hooks as recommended by Redux Toolkit.
233-233: LGTM: Proper usage of typed dispatch in BudgetCell.The
useAppDispatchhook is correctly used for dispatching modal-related actions.
411-411: LGTM: Proper usage of typed dispatch in ExpenseCategory.The
useAppDispatchhook is correctly used for dispatching budget-related actions.
Line range hint
1-1347: Verify complete migration of Redux hooks.The file appears to have completely migrated to typed Redux hooks. Let's verify there are no remaining untyped hooks.
✅ Verification successful
Migration to typed Redux hooks is complete
The file has been fully migrated to use typed Redux hooks:
- No untyped
useDispatchimports from 'react-redux'- All dispatch instances use
useAppDispatchfrom the local Redux setup- Implementation is consistent throughout the file
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining untyped Redux hooks # Test: Search for useDispatch imports or usage. Expect: No matches. echo "Checking for untyped useDispatch..." rg "useDispatch(?!.*App)" "packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx" # Note: The negative lookahead (?!.*App) ensures we don't match useAppDispatchLength of output: 412
Script:
#!/bin/bash # Check for useDispatch imports and usage with simpler patterns echo "Checking for useDispatch imports..." rg "import.*useDispatch.*from 'react-redux'" "packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx" echo -e "\nChecking for raw useDispatch usage..." rg "const dispatch = useDispatch\(" "packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx" echo -e "\nVerifying useAppDispatch usage..." rg "useAppDispatch" "packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx"Length of output: 695
packages/desktop-client/src/hooks/useAccounts.ts (1)
5-5: LGTM! Consistent hook renamingThe Redux hooks have been correctly renamed while maintaining the same functionality. The changes align with the PR objectives and Redux Toolkit's TypeScript recommendations.
Also applies to: 8-9, 17-17
packages/desktop-client/src/auth/AuthProvider.tsx (1)
4-4: LGTM! Clean hook replacementThe Redux selector hook has been properly renamed while preserving the existing TypeScript types and selector logic.
Also applies to: 19-19
packages/desktop-client/src/components/BankSyncStatus.tsx (1)
5-5: LGTM! Clean hook replacement with improved formattingThe Redux selector hook has been properly renamed, and the selector formatting has been improved for better readability while maintaining the same functionality.
Also applies to: 13-15
packages/desktop-client/src/components/settings/Reset.tsx (1)
8-8: LGTM! Consistent hook renamingThe Redux dispatch hook has been correctly renamed while maintaining the same dispatch functionality.
Also applies to: 47-47
packages/desktop-client/src/components/HelpMenu.tsx (1)
13-13: LGTM! Changes align with Redux Toolkit recommendations.The replacement of
useDispatchwithuseAppDispatchis implemented correctly and follows the PR objectives.Also applies to: 55-55
packages/desktop-client/src/components/manager/WelcomeScreen.tsx (1)
6-6: LGTM! Changes align with Redux Toolkit recommendations.The replacement of
useDispatchwithuseAppDispatchis implemented correctly and follows the PR objectives.Also applies to: 16-16
packages/desktop-client/src/components/manager/subscribe/Bootstrap.tsx (1)
9-9: LGTM! Changes align with Redux Toolkit recommendations.The replacement of
useDispatchwithuseAppDispatchis implemented correctly and follows the PR objectives.Also applies to: 23-23
packages/desktop-client/src/components/schedules/index.tsx (1)
10-10: LGTM! Changes align with Redux Toolkit recommendations.The replacement of
useDispatchwithuseAppDispatchis implemented correctly and follows the PR objectives. The implementation also maintains the project's pattern of includingdispatchin dependency arrays of hooks, as indicated in the retrieved learnings.Also applies to: 23-23
packages/desktop-client/src/components/sidebar/Sidebar.tsx (1)
14-14: LGTM! Changes align with Redux Toolkit recommendations.The migration from
useDispatchtouseAppDispatchis implemented correctly, maintaining the expected functionality while providing better TypeScript support.Also applies to: 30-30
packages/desktop-client/src/components/settings/Encryption.tsx (1)
7-7: LGTM! Consistent implementation of typed dispatch.The migration to
useAppDispatchis implemented correctly, maintaining the functionality while adding type safety for modal actions.Also applies to: 17-17
packages/desktop-client/src/components/transactions/TransactionMenu.tsx (1)
8-8: LGTM! Proper implementation of typed dispatch.The migration to
useAppDispatchis implemented correctly, maintaining the functionality while adding type safety for schedule-related actions.Also applies to: 39-39
packages/desktop-client/src/components/schedules/ScheduleLink.tsx (1)
15-15: LGTM! Proper implementation of typed dispatch.The migration to
useAppDispatchis implemented correctly, maintaining the functionality while adding type safety for modal actions.Also applies to: 38-38
packages/desktop-client/src/components/mobile/budget/CategoryTransactions.tsx (1)
20-20: LGTM! Consistent implementation ofuseAppDispatchThe changes align with the Redux Toolkit recommendations for TypeScript usage.
Also applies to: 37-37
packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx (1)
11-11: LGTM! Consistent implementation ofuseAppDispatchThe changes align with the Redux Toolkit recommendations for TypeScript usage, particularly beneficial in this component which makes extensive use of dispatch for modal management.
Also applies to: 28-28
packages/desktop-client/src/components/payees/ManagePayeesWithData.tsx (1)
15-15: LGTM! Consistent implementation ofuseAppDispatchThe changes align with the Redux Toolkit recommendations for TypeScript usage, particularly valuable in this data-heavy component with multiple dispatch calls.
Also applies to: 27-27
packages/desktop-client/src/components/reports/reports/CustomReportListCards.tsx (1)
15-15: LGTM! Consistent implementation ofuseAppDispatchThe changes align with the Redux Toolkit recommendations for TypeScript usage.
Also applies to: 67-67
packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx (1)
12-12: LGTM! Consistent with TypeScript best practices.The change from
useDispatchtouseAppDispatchaligns with Redux Toolkit's TypeScript recommendations and maintains type safety for dispatched actions.Also applies to: 87-87
packages/desktop-client/src/components/sidebar/Accounts.tsx (1)
15-15: LGTM! Type-safe Redux hooks implementation.The changes correctly implement both
useAppDispatchanduseAppSelector, maintaining type safety for the Redux state and dispatch actions. The selector properly types the account syncing state.Also applies to: 26-26, 34-36
packages/desktop-client/src/components/modals/manager/ConfirmChangeDocumentDir.tsx (1)
7-7: LGTM! Proper typed dispatch implementation.The change to
useAppDispatchmaintains type safety for notification dispatching while keeping the component's functionality intact.Also applies to: 51-51
packages/desktop-client/src/components/settings/index.tsx (1)
13-13: LGTM! Consistent type-safe dispatch implementation.The change to
useAppDispatchproperly maintains type safety for both thecloseBudgetandloadPrefsactions while preserving the component's functionality.Also applies to: 146-146
packages/desktop-client/src/components/LoggedInUser.tsx (1)
13-13: LGTM! Consistent implementation of typed Redux hooks.The changes align with Redux Toolkit's TypeScript recommendations, enhancing type safety without altering functionality.
Also applies to: 36-36, 38-38, 48-48, 53-53
packages/desktop-client/src/components/App.tsx (1)
31-31: LGTM! Verify dispatch usage in useEffect dependencies.The typed hooks are correctly implemented. Since
dispatchis used in theuseEffectdependency array, verify that the newuseAppDispatchmaintains referential stability across renders.Also applies to: 52-52, 53-53, 161-161
✅ Verification successful
Dispatch usage in useEffect dependencies is safe
The implementation follows Redux's best practices, and useAppDispatch maintains referential stability across renders. All usages properly clean up event listeners and follow consistent patterns throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential issues with dispatch in useEffect dependencies # Look for patterns where dispatch might cause unnecessary re-renders ast-grep --pattern 'useEffect($_, [$$$, dispatch, $$$])'Length of output: 11060
packages/desktop-client/src/components/modals/TransferOwnership.tsx (1)
13-13: LGTM! Enhanced type safety for ownership transfer operations.The implementation of typed hooks improves type safety for critical operations involving user data and file ownership management.
Also applies to: 32-32, 38-38, 43-43
packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx (1)
12-12: LGTM! The Redux hook changes look good.The changes align with the PR objective of using typed Redux hooks, which will improve type safety when dispatching actions.
Also applies to: 52-52
packages/desktop-client/src/components/reports/reports/NetWorth.tsx (1)
17-17: LGTM! The Redux hook changes look good.The changes align with the PR objective of using typed Redux hooks, which will improve type safety when dispatching actions.
Also applies to: 55-55
packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx (1)
12-12: LGTM! The Redux hook changes look good.The changes align with the PR objective of using typed Redux hooks, which will improve type safety when dispatching actions.
Also applies to: 44-44
packages/desktop-client/src/components/Notifications.tsx (1)
18-18: LGTM! The Redux hook changes look good.The changes align with the PR objective of using typed Redux hooks, which will improve type safety when dispatching actions and selecting state.
Also applies to: 265-270
packages/desktop-client/src/components/Titlebar.tsx (1)
28-28: LGTM! The Redux hook has been updated correctly.The change from
useDispatchtouseAppDispatchaligns with the PR objective to enhance type safety in Redux usage.Also applies to: 113-113
packages/desktop-client/src/components/FinancesApp.tsx (1)
21-21: LGTM! The Redux hooks have been updated correctly.The changes from
useDispatchtouseAppDispatchanduseSelectortouseAppSelectoralign with the PR objective to enhance type safety in Redux usage.Also applies to: 88-88, 92-92
packages/desktop-client/src/components/mobile/accounts/Accounts.tsx (1)
15-15: LGTM! The Redux hooks have been updated correctly.The changes from
useDispatchtouseAppDispatchanduseSelectortouseAppSelectoralign with the PR objective to enhance type safety in Redux usage.Also applies to: 229-231, 307-307, 309-311
packages/desktop-client/src/components/budget/index.tsx (1)
28-28: LGTM! The Redux hook has been updated correctly.The change from
useDispatchtouseAppDispatchaligns with the PR objective to enhance type safety in Redux usage.Also applies to: 70-70
packages/desktop-client/src/components/reports/reports/Summary.tsx (1)
24-24: LGTM! Clean and straightforward migration.The migration to
useAppDispatchis correctly implemented. The change improves type safety for notification dispatches.Also applies to: 179-179
packages/desktop-client/src/components/reports/Overview.tsx (1)
24-24: LGTM! Clean migration to typed dispatch.The migration to
useAppDispatchis correctly implemented, enhancing type safety for notification and modal actions.Also applies to: 54-54
packages/desktop-client/src/components/manager/BudgetList.tsx (1)
45-45: LGTM! Clean import of typed hooks.The import statement correctly includes both
useAppDispatchanduseAppSelectorhooks.packages/desktop-client/src/components/Modals.tsx (1)
12-12: LGTM! The change touseAppDispatchenhances type safety.The migration from
useDispatchtouseAppDispatchaligns with Redux Toolkit's recommendations for TypeScript usage.packages/desktop-client/src/components/reports/reports/Calendar.tsx (1)
50-50: LGTM! The change touseAppDispatchenhances type safety.The migration from
useDispatchtouseAppDispatchaligns with Redux Toolkit's recommendations for TypeScript usage.packages/desktop-client/src/components/schedules/ScheduleDetails.tsx (1)
24-24: LGTM! The change touseAppDispatchenhances type safety.The migration from
useDispatchtouseAppDispatchaligns with Redux Toolkit's recommendations for TypeScript usage.packages/desktop-client/src/components/accounts/Account.tsx (1)
Line range hint
1863-1892: LGTM! The changes touseAppSelectorenhance type safety.The migration from
useSelectortouseAppSelectoraligns with Redux Toolkit's recommendations for TypeScript usage.packages/desktop-client/src/auth/ProtectedRoute.tsx (1)
7-7: LGTM! Changes align with Redux Toolkit recommendations.The replacement of
useSelectorwithuseAppSelectoris correctly implemented for both state selections, maintaining type safety while following Redux Toolkit best practices.Also applies to: 26-26, 32-32
packages/desktop-client/src/components/modals/manager/ImportActualModal.tsx (1)
8-8: LGTM! Proper implementation of typed dispatch.The change from
useDispatchtouseAppDispatchis correctly implemented, ensuring type safety for dispatched actions.Also applies to: 36-36
packages/desktop-client/src/components/sidebar/Account.tsx (1)
16-16: LGTM! Hook replacement is independent of upcoming changes.The change from
useDispatchtouseAppDispatchis correctly implemented. Note: This component is being rewritten in a separate PR, but these changes are independent and don't interfere with that work.Also applies to: 105-105
packages/desktop-client/src/components/modals/CloseAccountModal.tsx (1)
17-17: LGTM! Consistent implementation of typed dispatch.The change from
useDispatchtouseAppDispatchis correctly implemented, maintaining type safety across all dispatched actions in the modal.Also applies to: 65-65
packages/desktop-client/src/components/manager/subscribe/Login.tsx (2)
13-13: LGTM! Import statement updated correctly.The change from
useDispatchtouseAppDispatchaligns with the Redux Toolkit's TypeScript recommendations and the PR objectives.
205-205: LGTM! Hook usage updated correctly.The
useAppDispatchhook is properly used to create the dispatch instance, maintaining functionality while improving type safety.Let's verify that no untyped Redux hooks remain in this file:
✅ Verification successful
Verification complete: Redux hooks are properly typed
The file correctly uses the typed
useAppDispatchhook from the local Redux setup and contains no untyped Redux hooks from 'react-redux'. The migration is successfully completed in this file.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining untyped Redux hooks rg -U "import.*\{.*use(Dispatch|Selector).*\}.*from 'react-redux'" "packages/desktop-client/src/components/manager/subscribe/Login.tsx"Length of output: 141
Script:
#!/bin/bash # Check for all Redux-related imports and hook usages rg -U "import.*from '(react-redux|@/store)'" "packages/desktop-client/src/components/manager/subscribe/Login.tsx" # Search for any remaining useDispatch or useSelector usage in the file rg -U "use(Dispatch|Selector)\(" "packages/desktop-client/src/components/manager/subscribe/Login.tsx"Length of output: 221
Script:
#!/bin/bash # First verify the file exists if [ -f "packages/desktop-client/src/components/manager/subscribe/Login.tsx" ]; then echo "File exists. Checking content:" # Show imports and surrounding context rg -C 2 "import" "packages/desktop-client/src/components/manager/subscribe/Login.tsx" else echo "File not found" # Search for the file in case the path is different fd -e tsx -e ts "Login" fiLength of output: 1439
packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx (2)
11-11: LGTM! Import change aligns with Redux Toolkit best practicesThe change from
useDispatchtouseAppDispatchfollows Redux Toolkit's recommendations for TypeScript usage, providing better type inference for dispatch actions.
60-60: Verify type inference for dispatch actionsThe change to
useAppDispatchlooks good. Let's verify that the action types are properly inferred.✅ Verification successful
Type safety is properly maintained with useAppDispatch
The change to
useAppDispatchensures proper type inference for modal actions and maintains type safety throughout the dispatch chain.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if pushModal action is properly typed in the Redux store # Look for the action type definition and its usage with AppDispatch # Search for pushModal action type definition echo "Searching for pushModal action type definition..." ast-grep --pattern 'export const pushModal = $_' # Search for AppDispatch type definition echo "Searching for AppDispatch type definition..." ast-grep --pattern 'export type AppDispatch = $_'Length of output: 3394
Script:
#!/bin/bash # Search for modal-related action definitions and usage echo "Searching for modal action definitions..." rg "pushModal|modal" --type ts -A 2 # Search for the specific component's modal dispatch usage echo "Searching for modal dispatch in SelectedTransactionsButton..." rg "pushModal" packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx -A 2Length of output: 65318
packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx (1)
14-14: LGTM! Import change aligns with Redux Toolkit recommendations.The change from
useDispatchtouseAppDispatchfollows Redux Toolkit's TypeScript best practices.packages/desktop-client/src/components/manager/subscribe/OpenIdCallback.ts (1)
6-6: LGTM! Changes align with TypeScript best practices.The switch to
useAppDispatchfollows Redux Toolkit's recommendations for TypeScript usage, providing better type safety for dispatch operations.Also applies to: 9-9
packages/desktop-client/src/components/modals/CreateAccountModal.tsx (2)
15-15: LGTM! Import change aligns with Redux Toolkit's TypeScript recommendations.The change from
useDispatchtouseAppDispatchfollows the Redux Toolkit's best practices for TypeScript integration.
37-37: LGTM! Hook usage is consistent throughout the component.The
useAppDispatchhook is correctly initialized and used consistently across all dispatch calls in the component, including:
- GoCardless bank authorization
- SimpleFin account linking
- Modal state management
- Local account creation
Let's verify that all dispatch calls in this component use properly typed actions:
✅ Verification successful
All dispatch calls in CreateAccountModal are properly typed and safe
The
pushModalaction creator is properly typed with multiple overloads that ensure type safety for both modal names and their options. All dispatch calls in CreateAccountModal use valid modal names that are registered in the modal system with proper type validation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all actions dispatched in this component are properly typed # by checking their imports and usage # Check action imports and their types rg "pushModal.*from.*actions" packages/desktop-client/src/ # Verify dispatch usage patterns ast-grep --pattern 'dispatch(pushModal($$$))' packages/desktop-client/src/components/modals/CreateAccountModal.tsxLength of output: 5574
Script:
#!/bin/bash # Check the type definitions for pushModal and modal names rg "type.*Modal.*=.*{" packages/desktop-client/src/ -A 5 rg "pushModal.*=.*" packages/desktop-client/src/ -A 2 ast-grep --pattern 'type $_ = { type: "pushModal"; payload: $_ }'Length of output: 280
Script:
#!/bin/bash # Search for modal and action type definitions rg "type.*Modal" packages/desktop-client/src/ rg "type.*Modal" packages/desktop-client/src/types/ rg "export.*pushModal" packages/desktop-client/src/ -A 2 # Check the actions directory specifically fd "modals.ts" --exec cat {}Length of output: 31042
packages/desktop-client/src/components/modals/manager/FilesSettingsModal.tsx (3)
8-8: LGTM! Import change aligns with Redux Toolkit best practices.The change from
useDispatchtouseAppDispatchfollows Redux Toolkit's TypeScript recommendations for better type safety.
145-145: LGTM! Verify action type inference.The change to
useAppDispatchensures better type safety. The dispatch usage withloadAllFilesaction should now have proper type inference.Let's verify the action type definitions are properly set up:
✅ Verification successful
Action types are properly set up and used correctly ✅
The
loadAllFilesaction creator is properly typed withAppDispatchandGetRootStateinpackages/loot-core/src/client/actions/budgets.ts, and it's correctly imported and used withuseAppDispatchin the FilesSettingsModal component.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the action types are properly defined ast-grep --pattern 'export const loadAllFiles = $_' 'loot-core/client/actions'Length of output: 150
Script:
#!/bin/bash # Find potential action files fd -e ts -e js "actions|store" # Search for loadAllFiles definitions rg "loadAllFiles" -A 2Length of output: 4543
19-19: LGTM! Verify action type inference.The change to
useAppDispatchensures better type safety. The dispatch usage withpushModalaction should now have proper type inference.Let's verify the action type definitions are properly set up:
✅ Verification successful
Type safety is maintained through TypeScript and modal architecture
The modal management system in the codebase is properly typed through TypeScript's type system and the centralized modal architecture. The
useAppDispatchusage is safe in this context.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the action types are properly defined ast-grep --pattern 'export const pushModal = $_' 'loot-core/client/actions'Length of output: 147
Script:
#!/bin/bash # Find potential action files fd -t f actions # Search for pushModal definition rg "export const pushModal" -A 2Length of output: 9963
Script:
#!/bin/bash # Check modal-related actions and store setup rg "modal" packages/desktop-client/src/hooks/useActions.ts -A 2 rg "modal" packages/desktop-client/src/store -A 2Length of output: 187
Script:
#!/bin/bash # Broader search for modal and store related files fd -t f store packages/desktop-client/src rg -g '!*.test.*' -g '!*.snap' "modal" packages/desktop-client/src -lLength of output: 1840
packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx (2)
27-27: LGTM! Import change aligns with Redux Toolkit recommendations.The change from
useSelectortouseAppSelectorfollows the Redux Toolkit's TypeScript usage guidelines.
62-64: LGTM! Hook usage updated correctly.The selector implementation maintains the same functionality while benefiting from improved type safety through
useAppSelector.Let's verify the Redux store type definitions are properly set up:
✅ Verification successful
Redux store setup verified successfully
The
useAppSelectorusage is correct and type-safe:
- RootState is properly defined in the store
- Multiple components consistently access
state.queries.newTransactions- Store is correctly configured with @reduxjs/toolkit
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Redux store type definitions # Look for store type definitions that should be used by useAppSelector ast-grep --pattern 'export type RootState = ReturnType<$_>'Length of output: 168
Script:
#!/bin/bash # Search for queries slice definition and newTransactions state echo "=== Searching for queries slice ===" ast-grep --pattern 'queries: { $$$ }' echo -e "\n=== Searching for newTransactions in state definitions ===" rg "newTransactions" --type typescript -C 2 packages/loot-core/src/client/store/Length of output: 371
Script:
#!/bin/bash echo "=== Searching for queries in store/reducer definitions ===" rg "queries" -t ts -t tsx -C 3 packages/loot-core/src/client/store/ echo -e "\n=== Looking for newTransactions state references ===" rg "newTransactions" -t ts -t tsx -C 3 packages/loot-core/src/client/store/ echo -e "\n=== Searching for state interface definitions ===" ast-grep --pattern 'interface $_ { queries?: $_ }' ast-grep --pattern 'type $_ = { queries?: $_ }'Length of output: 669
Script:
#!/bin/bash echo "=== Searching for queries state definitions ===" rg "queries.*newTransactions" -C 3 echo -e "\n=== Looking for store setup with queries ===" rg "createStore|configureStore" -C 3 echo -e "\n=== Looking for queries reducer ===" fd queries --exec grep -l "reducer" {}Length of output: 3721
Script:
#!/bin/bash echo "=== Checking queries reducer implementation ===" rg -C 5 "queries.*reducer" packages/loot-core/src/client/reducers/ echo -e "\n=== Looking for queries initial state ===" rg -C 3 "initialState.*queries" packages/loot-core/src/client/reducers/Length of output: 346
| @@ -24,7 +24,7 @@ import { useAccounts } from '../hooks/useAccounts'; | |||
| import { useCategories } from '../hooks/useCategories'; | |||
| import { usePayees } from '../hooks/usePayees'; | |||
| import { useSelected, SelectedProvider } from '../hooks/useSelected'; | |||
| import { useDispatch } from '../redux'; | |||
| import { useAppDispatch } from '../redux'; | |||
There was a problem hiding this comment.
💡 Codebase verification
Migration to typed Redux hooks is incomplete
UserAccess.tsxandUserDirectory.tsxin the admin components still use untypeduseDispatch- The ESLint rule to prevent untyped Redux hooks is not implemented yet
🔗 Analysis chain
LGTM! Changes align with Redux Toolkit's TypeScript recommendations.
The migration from useDispatch to useAppDispatch is implemented correctly and consistently.
Let's verify the ESLint rule mentioned in the PR objectives:
Also applies to: 116-116
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify ESLint rule preventing untyped Redux hooks
# Expected: Find ESLint configuration that bans untyped hooks
# Look for ESLint config files
fd -e js -e json -e yaml -e yml '\.eslint' -x cat {} \; | grep -A 5 -B 5 'react-redux'
# Check if there are any remaining untyped hooks
rg "import.*\{.*useDispatch.*\}.*from\s+['\"]react-redux['\"]"
Length of output: 491
Script:
#!/bin/bash
# Check redux/index.ts implementation
cat packages/desktop-client/src/redux/index.ts
# Look for ESLint config in package.json
cat packages/desktop-client/package.json | jq '.eslintConfig'
Length of output: 382
| @@ -87,7 +87,7 @@ export function GoCardlessExternalMsgModal({ | |||
| }: GoCardlessExternalMsgProps) { | |||
| const { t } = useTranslation(); | |||
|
|
|||
| const dispatch = useDispatch(); | |||
| const dispatch = useAppDispatch(); | |||
There was a problem hiding this comment.
💡 Codebase verification
Migration to typed dispatch is mostly complete but needs attention in admin components
The implementation in GoCardlessExternalMsgModal.tsx correctly uses the typed useAppDispatch hook. However, there are still two components using the untyped version:
- packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx
- packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx
🔗 Analysis chain
LGTM! Consistent usage of typed dispatch hook.
The implementation correctly uses the newly imported useAppDispatch hook.
Let's verify the consistency of this change across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining untyped useDispatch imports/usage
# that might have been missed during the migration.
echo "Checking for untyped useDispatch imports..."
rg "import.*\{.*useDispatch.*\}.*from 'react-redux'" packages/desktop-client/src
echo "Checking for untyped useDispatch usage..."
rg "const.*dispatch.*=.*useDispatch\(\)" packages/desktop-client/src
Length of output: 898
Script:
#!/bin/bash
# Check the redux/index.ts content and look for useAppDispatch usage
echo "Checking redux/index.ts content..."
cat packages/desktop-client/src/redux/index.ts
echo -e "\nChecking for typed useAppDispatch usage..."
rg "import.*\{.*useAppDispatch.*\}" packages/desktop-client/src
echo -e "\nChecking for typed dispatch usage..."
rg "const.*dispatch.*=.*useAppDispatch\(\)" packages/desktop-client/src
Length of output: 19734
| @@ -26,7 +26,7 @@ import { | |||
| import { useAccounts } from '../../hooks/useAccounts'; | |||
| import { useCommonPayees, usePayees } from '../../hooks/usePayees'; | |||
| import { SvgAdd, SvgBookmark } from '../../icons/v1'; | |||
| import { useDispatch } from '../../redux'; | |||
| import { useAppDispatch } from '../../redux'; | |||
There was a problem hiding this comment.
💡 Codebase verification
Migration needed: Two admin components still use untyped useDispatch
The following files should be updated to use useAppDispatch for consistent type safety across the codebase:
packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsxpackages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx
🔗 Analysis chain
LGTM! The Redux hook change aligns with TypeScript best practices.
The replacement of useDispatch with useAppDispatch follows the Redux Toolkit recommendations for TypeScript usage, providing better type inference for the dispatch function.
Let's verify the consistency of this change across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining untyped useDispatch imports and verify the consistency of useAppDispatch usage
# Check for any remaining untyped useDispatch imports from react-redux
echo "Checking for untyped useDispatch imports..."
rg "import.*useDispatch.*from 'react-redux'" -l
# Check the pattern of useAppDispatch usage
echo "Verifying useAppDispatch usage pattern..."
ast-grep --pattern 'import { useAppDispatch } from "../../redux"'
Length of output: 504
Script:
#!/bin/bash
# Check the implementation of useAppDispatch and related hooks
echo "Checking redux/index.ts content..."
cat packages/desktop-client/src/redux/index.ts
# Look for all variations of useAppDispatch imports
echo -e "\nChecking all useAppDispatch import patterns..."
rg "import.*useAppDispatch" -g "*.ts*"
# Check how useDispatch is used in the components
echo -e "\nChecking useDispatch usage in UserAccess.tsx..."
rg "useDispatch" -A 3 packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx
echo -e "\nChecking useDispatch usage in UserDirectory.tsx..."
rg "useDispatch" -A 3 packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx
Length of output: 10244
Rename redux useDispatch and useSelector to useAppDispatch and useAppSelector, respectively.
https://redux.js.org/usage/usage-with-typescript