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 (largest 100 files by percent change)
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
WalkthroughThis pull request performs a systematic refactoring of Redux hook usage throughout the desktop client. In nearly every file—from components and modals to custom hooks—the generic Redux hooks Possibly Related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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
joel-jeremy
force-pushed
the
rename-redux-hooks
branch
from
9c40c53 to
92835e6
Compare
There was a problem hiding this comment.
The code itself looks fine, though CodeRabbit had some comments that looked like potential issues. Copying over my comment from Discord:
I can see it both ways, though personally I lean a little towards
useDispatchinstead ofuseAppDispatch—it's marginally easier to understand when reading the code for newcomers, and you should still get linter errors in your IDE if you use the wrong one. But I'd love to hear from others, I was involved in the PR this is split from
Fixing this on #4123 |
|
I would lean more towards existing |
joel-jeremy
force-pushed
the
rename-redux-hooks
branch
from
4917edd to
818d031
Compare
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/desktop-client/src/redux/index.ts (1)
1-6: Consider enhancing the implementation with additional type utilities and documentation.While the current implementation is solid, consider these enhancements:
- Add JSDoc comments explaining the purpose of these typed hooks
- Consider creating additional type utilities for common Redux patterns (e.g., type-safe action creators, selector factories)
Example documentation:
+/** + * Type-safe version of useDispatch + * @see https://redux-toolkit.js.org/usage/usage-with-typescript#getting-the-dispatch-type + */ export const useAppDispatch = useDispatch.withTypes<AppDispatch>(); + +/** + * Type-safe version of useSelector + * @see https://redux-toolkit.js.org/usage/usage-with-typescript#getting-the-state-type + */ export const useAppSelector = useSelector.withTypes<RootState>();packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx (1)
1-1:❓ Verification inconclusive
Consider removing
@ts-strict-ignoreto enhance type safety.Since this PR aims to improve TypeScript usage by adopting typed Redux hooks, consider removing the
@ts-strict-ignoredirective to enforce stricter type checking throughout the file.Let's check if other files in the codebase have already enabled strict mode:
🏁 Script executed:
#!/bin/bash # Search for files with and without ts-strict-ignore echo "Files with @ts-strict-ignore:" rg "@ts-strict-ignore" -l echo "\nFiles with @ts-strict:" rg "@ts-strict" -lLength of output: 172
Action Required: Remove
@ts-strict-ignoreDirective for Enhanced Type Safety
- The file
packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsxcurrently begins with the@ts-strict-ignoredirective, which undermines the PR’s goal of enforcing stricter TypeScript checks.- Removing this directive will help ensure that type issues are identified and resolved, aligning the file with an overall stricter TypeScript configuration.
- Note that the initial repository-wide search for similar directives produced inconclusive results. Please manually verify that removing this directive won’t lead to unexpected type errors in this file or elsewhere.
📜 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 (102)
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(4 hunks)packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx(2 hunks)packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx(2 hunks)packages/desktop-client/src/components/admin/UserAccess/UserAccessRow.tsx(2 hunks)packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.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.tsx(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/EditAccess.tsx(2 hunks)packages/desktop-client/src/components/modals/EditRuleModal.jsx(2 hunks)packages/desktop-client/src/components/modals/EditUser.tsx(3 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/OpenIDEnableModal.tsx(2 hunks)packages/desktop-client/src/components/modals/OutOfSyncMigrationsModal.tsx(2 hunks)packages/desktop-client/src/components/modals/PasswordEnableModal.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(4 hunks)packages/desktop-client/src/components/util/GenericInput.jsx(2 hunks)packages/desktop-client/src/hooks/useAccounts.ts(1 hunks)packages/desktop-client/src/hooks/useCategories.ts(1 hunks)packages/desktop-client/src/hooks/useFailedAccounts.ts(1 hunks)packages/desktop-client/src/hooks/useGlobalPref.ts(3 hunks)packages/desktop-client/src/hooks/useMetadataPref.ts(2 hunks)packages/desktop-client/src/hooks/useModalState.ts(2 hunks)packages/desktop-client/src/hooks/usePayees.ts(2 hunks)packages/desktop-client/src/hooks/useSyncServerStatus.ts(1 hunks)packages/desktop-client/src/hooks/useSyncedPref.ts(2 hunks)packages/desktop-client/src/hooks/useSyncedPrefs.ts(1 hunks)packages/desktop-client/src/hooks/useTransactionBatchActions.ts(2 hunks)packages/desktop-client/src/hooks/useUndo.ts(2 hunks)packages/desktop-client/src/hooks/useUpdatedAccounts.ts(1 hunks)packages/desktop-client/src/redux/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (82)
- packages/desktop-client/src/components/schedules/PostsOfflineNotification.tsx
- packages/desktop-client/src/components/modals/manager/ImportYNAB5Modal.tsx
- packages/desktop-client/src/components/accounts/AccountSyncCheck.tsx
- packages/desktop-client/src/components/ManageRules.tsx
- packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx
- packages/desktop-client/src/components/modals/MergeUnusedPayeesModal.tsx
- packages/desktop-client/src/components/modals/OutOfSyncMigrationsModal.tsx
- packages/desktop-client/src/components/modals/GoCardlessExternalMsgModal.tsx
- packages/desktop-client/src/components/HelpMenu.tsx
- packages/desktop-client/src/components/manager/ManagementApp.tsx
- packages/desktop-client/src/components/sidebar/Sidebar.tsx
- packages/desktop-client/src/components/modals/manager/ImportModal.tsx
- packages/desktop-client/src/components/modals/CloseAccountModal.tsx
- packages/desktop-client/src/components/modals/TransferModal.tsx
- packages/desktop-client/src/components/mobile/budget/index.tsx
- packages/desktop-client/src/components/modals/manager/ImportActualModal.tsx
- packages/desktop-client/src/components/reports/reports/Spending.tsx
- packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx
- packages/desktop-client/src/components/reports/reports/Calendar.tsx
- packages/desktop-client/src/components/Titlebar.tsx
- packages/desktop-client/src/components/modals/CreateAccountModal.tsx
- packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx
- packages/desktop-client/src/components/LoggedInUser.tsx
- packages/desktop-client/src/components/modals/manager/ImportYNAB4Modal.tsx
- packages/desktop-client/src/components/transactions/TransactionMenu.tsx
- packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx
- packages/desktop-client/src/auth/ProtectedRoute.tsx
- packages/desktop-client/src/components/modals/manager/ConfirmChangeDocumentDir.tsx
- packages/desktop-client/src/components/util/GenericInput.jsx
- packages/desktop-client/src/components/schedules/ScheduleLink.tsx
- packages/desktop-client/src/components/Notifications.tsx
- packages/desktop-client/src/components/AppBackground.tsx
- packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx
- packages/desktop-client/src/components/payees/ManagePayeesWithData.tsx
- packages/desktop-client/src/components/settings/Encryption.tsx
- packages/desktop-client/src/components/settings/AuthSettings.tsx
- packages/desktop-client/src/components/reports/reports/CashFlow.tsx
- packages/desktop-client/src/components/reports/reports/NetWorth.tsx
- packages/desktop-client/src/components/modals/EditRuleModal.jsx
- packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx
- packages/desktop-client/src/components/budget/index.tsx
- packages/desktop-client/src/components/sidebar/Account.tsx
- packages/desktop-client/src/components/modals/TransferOwnership.tsx
- packages/desktop-client/src/components/modals/BudgetListModal.tsx
- packages/desktop-client/src/components/sidebar/BudgetName.tsx
- packages/desktop-client/src/components/reports/reports/Summary.tsx
- packages/desktop-client/src/components/modals/EnvelopeBudgetSummaryModal.tsx
- packages/desktop-client/src/auth/AuthProvider.tsx
- packages/desktop-client/src/components/modals/manager/FilesSettingsModal.tsx
- packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.jsx
- packages/desktop-client/src/components/manager/subscribe/Bootstrap.tsx
- packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx
- packages/desktop-client/src/components/schedules/index.tsx
- packages/desktop-client/src/components/FinancesApp.tsx
- packages/desktop-client/src/components/modals/CoverModal.tsx
- packages/desktop-client/src/components/manager/subscribe/OpenIdCallback.ts
- packages/desktop-client/src/components/transactions/TransactionList.jsx
- packages/desktop-client/src/components/UpdateNotification.tsx
- packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx
- packages/desktop-client/src/components/schedules/ScheduleDetails.tsx
- packages/desktop-client/src/components/manager/ConfigServer.tsx
- packages/desktop-client/src/components/mobile/budget/CategoryTransactions.tsx
- packages/desktop-client/src/components/settings/Reset.tsx
- packages/desktop-client/src/components/Modals.tsx
- packages/desktop-client/src/components/manager/WelcomeScreen.tsx
- packages/desktop-client/src/components/reports/Overview.tsx
- packages/desktop-client/src/components/settings/index.tsx
- packages/desktop-client/src/components/mobile/accounts/Accounts.tsx
- packages/desktop-client/src/components/manager/subscribe/Login.tsx
- packages/desktop-client/src/components/sidebar/Accounts.tsx
- packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
- packages/desktop-client/src/components/BankSyncStatus.tsx
- packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx
- packages/desktop-client/src/components/manager/BudgetList.tsx
- packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx
- packages/desktop-client/src/components/App.tsx
- packages/desktop-client/src/components/reports/reports/CustomReportListCards.tsx
- packages/desktop-client/src/components/modals/LoadBackupModal.tsx
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx
- packages/desktop-client/src/components/accounts/Account.tsx
- packages/desktop-client/src/hooks/useAccounts.ts
- packages/desktop-client/src/components/transactions/TransactionsTable.jsx
🔇 Additional comments (40)
packages/desktop-client/src/hooks/useSyncedPref.ts (2)
6-6: LGTM! Import of typed Redux hooks.The change aligns with Redux Toolkit's TypeScript recommendations by importing the typed hooks from the local Redux setup.
15-15: LGTM! Migration to typed Redux hooks.The replacement of
useDispatchanduseSelectorwith their typed counterpartsuseAppDispatchanduseAppSelectorenhances type safety while maintaining the same functionality.This change:
- Improves TypeScript integration
- Follows Redux Toolkit best practices
- Maintains consistent hook naming across the codebase
Also applies to: 22-22
packages/desktop-client/src/hooks/useMetadataPref.ts (2)
6-6: LGTM! Import changes align with Redux Toolkit's recommendations.The change from React Redux's untyped hooks to app-specific typed hooks follows Redux Toolkit's best practices for TypeScript usage.
15-15: LGTM! Hook usage changes improve type safety.The replacement of
useDispatchanduseSelectorwith their app-specific counterpartsuseAppDispatchanduseAppSelectorenhances type safety while maintaining the same functionality.Also applies to: 22-22
packages/desktop-client/src/hooks/useTransactionBatchActions.ts (1)
20-20: LGTM! The change to typed Redux hooks enhances type safety.The switch to
useAppDispatchaligns with Redux Toolkit's TypeScript recommendations and provides better type inference for the complex transaction batch operations in this hook.Also applies to: 58-58
packages/desktop-client/src/components/mobile/transactions/TransactionList.tsx (1)
31-31: LGTM! The Redux hook changes enhance type safety.The replacement of
useDispatchwithuseAppDispatchaligns with Redux Toolkit's TypeScript recommendations, providing better type inference for the dispatch function.Also applies to: 279-279
packages/desktop-client/src/components/modals/PasswordEnableModal.tsx (2)
8-8: LGTM! Import statement updated to use typed Redux hook.The change from
useDispatchtouseAppDispatchaligns with Redux Toolkit's TypeScript recommendations and enhances type safety.
34-34: LGTM! Hook usage updated consistently.The dispatch hook usage is updated to use
useAppDispatch, maintaining consistency with the import change and providing better TypeScript type inference for dispatched actions.packages/desktop-client/src/hooks/useUpdatedAccounts.ts (2)
1-1: LGTM! Import change aligns with Redux Toolkit best practices.The change from
useSelectortouseAppSelectorfollows the recommended pattern for TypeScript usage with Redux Toolkit.
4-4: LGTM! Hook usage updated consistently.The selector implementation remains unchanged while benefiting from improved type safety through
useAppSelector.packages/desktop-client/src/components/modals/EditAccess.tsx (2)
10-10: LGTM! Import change aligns with Redux Toolkit best practices.The change from
useDispatchtouseAppDispatchenhances type safety and follows Redux Toolkit's TypeScript recommendations.
30-30: LGTM! Hook usage properly updated.The change to
useAppDispatchensures proper typing for all dispatched actions while maintaining the same functionality.packages/desktop-client/src/hooks/useModalState.ts (2)
6-6: LGTM! Import of typed Redux hooks.The change to use
useAppSelectoranduseAppDispatchaligns with Redux Toolkit's recommendations for TypeScript usage.
17-19: LGTM! Consistent usage of typed Redux hooks.The changes properly implement the typed Redux hooks while maintaining the same functionality. The state selectors benefit from improved type inference through
useAppSelector.packages/desktop-client/src/hooks/useUndo.ts (2)
8-8: LGTM! Import statement updated to use typed Redux hook.The change aligns with Redux Toolkit recommendations for TypeScript usage.
20-20: LGTM! Dispatch instantiation updated to use typed Redux hook.The change maintains the same functionality while providing better type safety. The hook is correctly used within the component and its dependencies.
packages/desktop-client/src/hooks/usePayees.ts (3)
8-8: LGTM! Import statement aligns with Redux Toolkit recommendations.The change to import typed Redux hooks from the application's Redux folder follows the Redux Toolkit's recommendations for TypeScript usage.
10-23: LGTM! Consistent usage of typed Redux hooks.The changes in
useCommonPayeescorrectly implement the typed Redux hooks while maintaining the existing functionality. The selector functions are properly memoized, and the hook's logic remains unchanged.
25-36: LGTM! Consistent usage of typed Redux hooks.The changes in
usePayeescorrectly implement the typed Redux hooks while maintaining the existing functionality. The implementation follows the same pattern asuseCommonPayees, ensuring consistency across the codebase.packages/desktop-client/src/hooks/useFailedAccounts.ts (1)
3-3: LGTM! Changes align with Redux Toolkit best practices.The replacement of
useSelectorwithuseAppSelectorfollows the Redux Toolkit recommendations for TypeScript usage, ensuring type safety and consistency across the codebase.Also applies to: 6-6
packages/desktop-client/src/hooks/useCategories.ts (1)
5-5: LGTM! Consistent application of Redux Toolkit patterns.The changes correctly implement the Redux Toolkit recommendations by:
- Using
useAppDispatchfor action dispatching- Using
useAppSelectorfor state selection- Maintaining consistent typing across Redux operations
This enhances type safety and follows the established patterns for TypeScript usage.
Also applies to: 8-11, 19-19
packages/desktop-client/src/components/modals/EditUser.tsx (4)
11-11: LGTM! Import change aligns with Redux Toolkit recommendations.The change from
useDispatchtouseAppDispatchfollows the recommended pattern for TypeScript usage.
78-78: LGTM! Hook usage updated in custom hook.The change to
useAppDispatchinuseSaveUsermaintains functionality while improving type safety for action dispatching.
160-160: LGTM! Hook usage updated in component.The change to
useAppDispatchinEditUsercomponent maintains functionality while improving type safety for modal actions.
11-11:✅ Verification successful
Verify consistent hook usage across the codebase.
Let's ensure the migration from
useDispatchtouseAppDispatchis consistent across all files.Also applies to: 78-78, 160-160
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining usages of the untyped hooks # Search for any remaining useDispatch imports echo "Checking for remaining useDispatch imports..." rg "import.*useDispatch.*from 'react-redux'" || echo "No untyped useDispatch imports found." # Search for any remaining useDispatch declarations echo -e "\nChecking for remaining useDispatch usage..." rg "const dispatch = useDispatch\(\)" || echo "No untyped useDispatch usage found."Length of output: 462
Consistent hook usage verified across the codebase.
The migration from
useDispatchtouseAppDispatchhas been successfully completed. Verification using the shell scripts confirms that there are no remaining imports or usages of the untypeduseDispatchhook. Specifically:
- File:
packages/desktop-client/src/components/modals/EditUser.tsxnow correctly importsuseAppDispatchon line 11.- Other referenced lines (78 and 160) have been audited, and no untyped
useDispatchusage was found throughout the codebase.No further action is needed.
packages/desktop-client/src/redux/index.ts (1)
1-6:❓ Verification inconclusive
LGTM! The implementation follows Redux Toolkit's best practices.
The changes align perfectly with Redux Toolkit's recommendations for TypeScript usage:
- Clear distinction between untyped and typed hooks through naming
- Proper type safety using
.withTypes<>()- Centralized hook definitions for consistent usage across the application
Let's verify that the new hook names are being used consistently across the codebase:
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining usage of untyped hooks # and verify adoption of typed hooks echo "Checking for any remaining untyped useDispatch/useSelector usage..." rg "useDispatch(?!\.|\w)" --type ts --type tsx echo "Checking for any remaining untyped useSelector usage..." rg "useSelector(?!\.|\w)" --type ts --type tsx echo "Verifying adoption of typed hooks..." rg "useAppDispatch|useAppSelector" --type ts --type tsxLength of output: 1002
Ensure all hook usages are consistently typed and verify regex commands
The implementation in the file aligns perfectly with Redux Toolkit's recommendations:
- Proper imports and naming conventions help distinguish between untyped and typed hooks.
- The use of
.withTypes<>()enforces type safety and follows best practices.However, our automated verification using the provided regex commands encountered errors (due to unsupported lookahead and an unrecognized file type for tsx). Please manually re-run the following commands with the
--pcre2flag (and adjust the file type for tsx if needed) to ensure no untypeduseDispatchoruseSelectorusages remain:#!/bin/bash # Verify untyped useDispatch rg --pcre2 'useDispatch(?!\.|\w)' --type ts --type tsx # Verify untyped useSelector rg --pcre2 'useSelector(?!\.|\w)' --type ts --type tsx # Confirm adoption of typed hooks rg 'useAppDispatch|useAppSelector' --type ts --type tsxOnce these adjustments are made and verification is confirmed, the hook usage across the codebase can be confidently validated.
packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx (2)
10-10: LGTM! Import statement updated correctly.The change from
useDispatchtouseAppDispatchaligns with the PR objectives to use typed Redux hooks as recommended by Redux Toolkit.
28-28:❓ Verification inconclusive
LGTM! Hook usage updated correctly.
The dispatch variable is now properly typed using
useAppDispatch, which will provide better TypeScript support for action dispatching throughout the component.Let's verify that all dispatch usages in this file are properly typed:
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining untyped useDispatch imports or usages rg -i "useDispatch" packages/desktop-client/src/components/modals/OpenIDEnableModal.tsxLength of output: 195
Action Required: Verify File Location and Hook Implementation
The use of
useAppDispatchappears correct, ensuring properly typed dispatch calls. However, our check for anyuseDispatchusage in
packages/desktop-client/src/components/modals/OpenIDEnableModal.tsxreturned an error indicating that the file could not be found. Please verify that the file exists at the expected path or whether it has been moved/renamed. Once the file location is confirmed and there are no untypeduseDispatchusages, the changes can be fully approved.
- Confirm the file’s existence and location (e.g., using a command like
fd -HI "OpenIDEnableModal\.tsx").- Ensure all dispatch usages in the component are properly typed with
useAppDispatch.packages/desktop-client/src/components/admin/UserAccess/UserAccessRow.tsx (1)
11-11:❓ Verification inconclusive
LGTM! The Redux hook changes align with TypeScript best practices.
The changes correctly implement the typed Redux hooks as recommended by Redux Toolkit, which will improve type safety in the application.
Let's verify that all Redux hook imports in this directory follow the same pattern:
Also applies to: 26-26
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining untyped Redux hooks in the admin directory # Search for untyped hooks echo "Checking for untyped useDispatch/useSelector hooks..." rg "import.*\{.*use(Dispatch|Selector).*\}.*from.*['\"]react-redux['\"]" "packages/desktop-client/src/components/admin" # Search for typed hooks to confirm consistent usage echo "Checking for typed useAppDispatch/useAppSelector hooks..." rg "import.*\{.*useApp(Dispatch|Selector).*\}.*from.*['\"].*redux['\"]" "packages/desktop-client/src/components/admin"Length of output: 658
LGTM! The Redux hook changes are implemented correctly and align with the migration to typed hooks for improved type safety.
- The import change at line 11 now correctly uses
useAppDispatchfrom'../../../redux'.- The hook usage update at line 26 is consistent with the new typed approach.
- Note: The automated search script initially pointed to a non-existent directory (
packages/desktop-client/src/components/admin). Please manually verify that the affected file and any related admin components reside in the correct folder (e.g.,packages/desktop-client/src/components/admin/UserAccess) and that the new hook usage is applied uniformly.packages/desktop-client/src/hooks/useSyncServerStatus.ts (2)
8-8: LGTM! The hook usage change enhances type safety.The replacement of
useSelectorwithuseAppSelectormaintains the same functionality while adding TypeScript type inference for the Redux state.
2-2:❓ Verification inconclusive
LGTM! The import change aligns with Redux Toolkit's recommendations.
The change to import
useAppSelectorfollows best practices for TypeScript usage with Redux Toolkit.Let's verify the centralized export of typed hooks:
🏁 Script executed:
#!/bin/bash # Description: Verify the centralized export of typed Redux hooks # Expected: Find the export of useAppSelector in the redux index file # Check the redux index file for the typed hook export rg -A 5 "export.*useAppSelector" "packages/desktop-client/src/redux/"Length of output: 144
Attention: Verify the Centralized Hook Export
The change to import
useAppSelectorinpackages/desktop-client/src/hooks/useSyncServerStatus.tsis consistent with migrating toward a more type-safe Redux Toolkit usage. However, our initial check for the centralized export at the expected path (packages/desktop-client/src/redux) yielded an error ("No such file or directory"). This suggests one of the following:
- The centralized export file (which should export
useAppSelector) might be missing or located in a different directory.- The import path may need adjustment to reflect the correct location of the exported hook.
Action Items:
Verify the Export Location:
Confirm thatuseAppSelectoris exported from a centralized module. If its export exists in another file or directory, update the import path accordingly.Double-Check the Repository Structure:
Review the repository structure manually or run a repository-wide search (e.g., usingrg -n "export.*useAppSelector" .) to locate the export ofuseAppSelectorto ensure consistency with the intended architecture.Once you verify the location and adjust the path if needed, the hook change itself aligns well with Redux Toolkit’s type safety recommendations.
packages/desktop-client/src/hooks/useSyncedPrefs.ts (4)
6-6: LGTM! Import changes align with Redux Toolkit recommendations.The change to import typed Redux hooks from a local Redux configuration follows the recommended pattern for TypeScript usage.
12-12: LGTM! Dispatch hook change enhances type safety.The switch to
useAppDispatchprovides better type inference for dispatched actions while maintaining the existing functionality.
19-19: LGTM! Selector hook change improves type inference.The switch to
useAppSelectorenhances type safety for state selection while preserving the existing selector logic.
10-10:❓ Verification inconclusive
Verify migration status to
useSyncedPref.Since this hook is marked as deprecated in favor of
useSyncedPref, ensure that:
- The singular version exists and is properly implemented
- A migration plan is in place for existing usages
- Documentation is updated to guide developers to the new hook
🏁 Script executed:
#!/bin/bash # Search for the singular version implementation and its usages echo "Searching for useSyncedPref implementation..." rg "useSyncedPref[^s]" -A 5 echo "Checking remaining usages of deprecated hook..." rg "useSyncedPrefs" -A 5Length of output: 260
Action Required: Verify migration to
useSyncedPrefThe deprecation notice in
packages/desktop-client/src/hooks/useSyncedPrefs.tssuggests replacing the plural hook with a singular version. However, our initial script did not find any references touseSyncedPref, which means we cannot confirm its existence or usage automatically. Please ensure the following:
- Confirm that the new hook (
useSyncedPref) is implemented (for example, verify that a file likepackages/desktop-client/src/hooks/useSyncedPref.tsexists).- Check that all usages of
useSyncedPrefshave been migrated to the new hook.- Verify that there is a migration plan in place and that the relevant documentation is updated to guide developers.
packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx (2)
23-23: LGTM! Import change aligns with Redux Toolkit best practices.The change from
useDispatchtouseAppDispatchfollows Redux Toolkit's TypeScript recommendations for improved type safety.
88-88: LGTM! Consistent usage of the typed dispatch hook.The
useAppDispatchhook is used correctly and consistently throughout the component for dispatching various actions.packages/desktop-client/src/hooks/useGlobalPref.ts (3)
6-6: LGTM! Import changes align with Redux Toolkit recommendations.The change to import typed Redux hooks follows the recommended pattern for TypeScript usage, enhancing type safety in the application.
16-16: LGTM! Proper implementation of typed Redux hooks.The changes correctly implement the typed Redux hooks while maintaining the existing functionality and type safety. The selector properly accesses the state with appropriate type inference.
Also applies to: 30-32
6-6:❓ Verification inconclusive
Verify ESLint rule configuration.
The changes look good, but let's verify that the ESLint rule is properly configured to prevent untyped hook usage.
Also applies to: 16-16, 30-32
🏁 Script executed:
#!/bin/bash # Description: Check ESLint configuration for Redux hook rules # Search for ESLint configuration files echo "Searching for ESLint configuration files..." fd -e js -e json -e yaml -e yml '.eslintrc' # Search for any remaining untyped hook usage echo "Checking for any remaining untyped hook usage..." rg "useDispatch|useSelector" --type tsLength of output: 437
The previous verification script didn’t search any files due to an overly restrictive filter. Let's run an updated script that:
• Lists all ESLint configuration files (to ensure our rules are in effect)
• Searches across all *.ts and *.tsx files for any usage of “useDispatch” or “useSelector”Please run the script below and confirm the output manually:
Once you’ve verified that:
- The ESLint configuration files are present and correct
- The search returns no unexpected untyped hook usage
we can be confident that the ESLint rule is effectively catching untyped hooks as part of the migration.
Next Steps:
- Review the ESLint configuration files to ensure the rule covers both .ts and .tsx files.
- Confirm no untyped hook usage is reported by the updated script.
🏁 Script executed:
[run_scripts] #!/bin/bash echo "Listing ESLint configuration files in the repository:" find . -type f \( -iname '.eslintrc' -o -iname '.eslintrc.js' -o -iname '.eslintrc.json' -o -iname '.eslintrc.yaml' -o -iname '.eslintrc.yml' \) echo "Searching for untyped Redux hook usage in .ts and .tsx files..." rg -g "*.ts" -g "*.tsx" "useDispatch|useSelector" .Length of output: 568
Verify ESLint configuration for Redux hooks usage
The changes look good overall, but please manually verify the following:
- ESLint Configuration: Confirm that the ESLint rules are set up to enforce typed Redux hooks. Note that some projects define their ESLint settings in the
package.jsonrather than in standalone.eslintrc*files.- Untyped Hook Usage: Verify that searches for
useDispatchanduseSelectorin your TypeScript files (including.tsand.tsx) return no untyped usages—especially in areas noted at lines 16-16 and 30-32.Once you confirm the ESLint configuration (whether in
package.jsonor an external config file) properly guards against untyped hook usage, this review point can be considered addressed.
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
|
This PR was closed because it has been stalled for 5 days with no activity. |
Rename redux useDispatch and useSelector to useAppDispatch and useAppSelector, respectively.
https://redux.js.org/usage/usage-with-typescript