-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: fetch data on initial render of app #1413
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes involve significant refactoring across multiple components and hooks in the frontend application. Key modifications include the removal of various data-fetching logic and hooks, particularly those related to fetching validators, balances, and delegations. A new custom hook, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Hook
participant Redux
User->>App: Open application
App->>Hook: Initialize app state
Hook->>Redux: Fetch user wallet and chain data
Redux-->>Hook: Return data
Hook->>App: Update state with fetched data
App->>User: Render updated UI
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 review |
Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (3)
frontend/src/custom-hooks/useInitAllValidator.ts (1)
Line range hint
1-34
: Consider making the hook more generic and handle errors.The hook is fetching validator data for specific networks and validators, which might not be scalable for adding new networks or validators. Please consider the following suggestions:
Make the hook more generic to support fetching validator data for any network or validator. This can be achieved by accepting the network and validator details as parameters to the hook.
Handle errors that might occur while fetching the data. You can use a try-catch block or the
unwrapResult
function from Redux Toolkit to handle errors.Return the fetched data from the hook so that it can be used in the application. You can use the
useSelector
hook to select the data from the Redux store.Here's an example of how the hook can be refactored to incorporate these suggestions:
import { useEffect, useState } from 'react'; import { useAppDispatch, useAppSelector } from './StateHooks'; import { getWitvalValidator, getWitvalDelegations, getWitvalDelegatorsCount } from '@/store/features/staking/stakeSlice'; import { RootState } from '@/store/store'; interface ValidatorData { validator: any; delegations: any; delegatorsCount: number; } const useInitValidator = (network: string, validatorId: number, operatorAddress: string) => { const dispatch = useAppDispatch(); const [error, setError] = useState<string | null>(null); const { validator, delegations, delegatorsCount } = useAppSelector((state: RootState) => state.stake); useEffect(() => { const fetchData = async () => { try { await dispatch(getWitvalValidator({ baseURL: network, id: validatorId })).unwrap(); await dispatch(getWitvalDelegations({ baseURL: network, operatorAddress })).unwrap(); await dispatch(getWitvalDelegatorsCount({ baseURL: network, id: validatorId })).unwrap(); } catch (error) { setError(error.message); } }; fetchData(); }, [dispatch, network, validatorId, operatorAddress]); return { validator, delegations, delegatorsCount, error }; }; export default useInitValidator;In this refactored version:
- The hook accepts the
network
,validatorId
, andoperatorAddress
as parameters, making it more generic.- It uses a try-catch block to handle errors and sets the error message in the component state.
- It returns the fetched data and the error state from the hook, which can be used in the component.
You can then use this hook in your component like this:
const { validator, delegations, delegatorsCount, error } = useInitValidator(POLYGON_CONFIG.baseURL, 50, '');This way, the hook is more reusable and handles errors gracefully.
frontend/src/app/(routes)/(overview)/overview-components/OverviewDashboard.tsx (1)
Line range hint
8-28
: Consider lifting the state up or using a context ifchainIDs
is used by many child components.The
OverviewDashboard
component passes thechainIDs
prop down to multiple child components. If this prop is used by many child components, it might be worth considering the following suggestions to avoid prop drilling:
- Lift the state up to a higher-level component that can pass the
chainIDs
directly to the child components that need it.- Use the Context API to provide the
chainIDs
value to the child components without passing it through intermediary components.frontend/src/app/(routes)/multiops/components/Messages/Redelegate.tsx (1)
48-48
: Reintroduce thebaseURLs
prop in theRedelegate
component.The
baseURLs
prop is still used within theRedelegate.tsx
file, specifically in a function call togetDelegations
. Removing it from the destructured props without addressing its usage elsewhere could lead to runtime errors or broken functionality. Please ensure that thebaseURLs
prop is correctly handled in the component.
baseURLs
is used in a function call:dispatch(getDelegations({ address, chainID, baseURLs: restURLs }));
Analysis chain
Verify the impact of removing the
baseURLs
prop.Ensure that the removal of the
baseURLs
prop does not break any functionality in theRedelegate
component.Run the following script to verify the usage of
baseURLs
in the component:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `baseURLs` in the `Redelegate` component. # Test: Search for the usage of `baseURLs` in the component. Expect: No occurrences. rg --type typescript -A 5 $'baseURLs' frontend/src/app/(routes)/multiops/components/Messages/Redelegate.tsxLength of output: 244
Script:
#!/bin/bash # Description: Verify the usage of `baseURLs` in the `Redelegate` component. # Test: Search for the usage of `baseURLs` in the component. Expect: No occurrences. rg --type typescript -A 5 $'baseURLs' frontend/src/app/\(routes\)/multiops/components/Messages/Redelegate.tsxLength of output: 143
Script:
#!/bin/bash # Description: Verify the usage of `baseURLs` in the `Redelegate` component. # Test: Search for the usage of `baseURLs` in the component. Expect: No occurrences. rg -A 5 $'baseURLs' frontend/src/app/\(routes\)/multiops/components/Messages/Redelegate.tsxLength of output: 329
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (20)
- frontend/src/app/(routes)/(overview)/overview-components/OverviewDashboard.tsx (1 hunks)
- frontend/src/app/(routes)/governance/popups/DialogDeposit.tsx (4 hunks)
- frontend/src/app/(routes)/multiops/components/Messages/Delegate.tsx (5 hunks)
- frontend/src/app/(routes)/multiops/components/Messages/Redelegate.tsx (4 hunks)
- frontend/src/app/(routes)/multiops/components/Messages/Undelegate.tsx (2 hunks)
- frontend/src/app/(routes)/multisig/components/multisig-account/MultisigAccount.tsx (2 hunks)
- frontend/src/app/(routes)/settings/authz/new-authz/components/ValidatorAutoComplete.tsx (4 hunks)
- frontend/src/app/(routes)/staking/[network]/SingleChainDashboard.tsx (1 hunks)
- frontend/src/app/(routes)/staking/components/ReDelegatePopup.tsx (4 hunks)
- frontend/src/app/(routes)/transfers/components/TransfersPage.tsx (2 hunks)
- frontend/src/components/main-layout/FixedLayout.tsx (3 hunks)
- frontend/src/components/txn-builder/messages/DelegateForm.tsx (3 hunks)
- frontend/src/components/txn-builder/messages/RedelegateForm.tsx (2 hunks)
- frontend/src/components/txn-builder/messages/UndelegateForm.tsx (2 hunks)
- frontend/src/custom-hooks/common/useInitApp.ts (1 hunks)
- frontend/src/custom-hooks/useGetValidatorInfo.ts (4 hunks)
- frontend/src/custom-hooks/useInitAllValidator.ts (1 hunks)
- frontend/src/custom-hooks/useInitStaking.ts (1 hunks)
- frontend/src/custom-hooks/useStaking.ts (5 hunks)
- frontend/src/custom-hooks/useValidator.ts (1 hunks)
Files skipped from review due to trivial changes (6)
- frontend/src/app/(routes)/governance/popups/DialogDeposit.tsx
- frontend/src/app/(routes)/multisig/components/multisig-account/MultisigAccount.tsx
- frontend/src/app/(routes)/staking/components/ReDelegatePopup.tsx
- frontend/src/components/txn-builder/messages/DelegateForm.tsx
- frontend/src/components/txn-builder/messages/RedelegateForm.tsx
- frontend/src/components/txn-builder/messages/UndelegateForm.tsx
Additional context used
Biome
frontend/src/custom-hooks/useValidator.ts
[error] 26-26: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 27-27: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
frontend/src/custom-hooks/useStaking.ts
[error] 25-25: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
Additional comments not posted (24)
frontend/src/custom-hooks/useInitStaking.ts (1)
1-1
: Justify the removal of the entire logic and provide more context.The changes effectively disable all previous functionalities associated with staking initialization. This is a significant change that requires careful consideration and justification.
Please provide more context on the motivation behind this change and its impact on the application's functionality. In particular, address the following questions:
- Is the removal of this logic intentional, and does it align with the PR objectives?
- How does this change affect the user experience and the application's behavior?
- Are there any dependencies or other parts of the codebase that rely on the removed functionality?
- Is there a plan to replace or refactor the removed logic in a different way?
To verify the impact of this change, run the following script to search for usages of
useInitStaking
in the codebase:If there are usages of
useInitStaking
in other parts of the codebase, ensure that the removal of the logic does not break the application or introduce unintended side effects.Verification successful
No external usage of
useInitStaking
found, but justification needed.The
useInitStaking
function is not used elsewhere in the codebase, which means its removal does not directly impact other parts of the application. However, the removal of its logic still requires justification to ensure it aligns with the overall objectives of the PR and does not affect future development plans.
- Please provide more context on the motivation behind this change and its impact on the application's functionality.
- Is the removal of this logic intentional, and does it align with the PR objectives?
- Are there plans to replace or refactor the removed logic in a different way?
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usages of `useInitStaking` in the codebase. # Test: Search for the function usage. Expect: Occurrences of the function usage. rg --type typescript -A 5 $'useInitStaking'Length of output: 77
Script:
#!/bin/bash # Description: Search for usages of `useInitStaking` in the codebase. # Test: Search for the function usage. Expect: Occurrences of the function usage. rg --type ts -A 5 $'useInitStaking'Length of output: 231
frontend/src/app/(routes)/(overview)/overview-components/OverviewDashboard.tsx (1)
Line range hint
1-37
: No inconsistencies between the code and the AI-generated summary.The AI-generated summary provides additional context about the removed code, which is not visible in the provided code. The code reflects the final state after the changes mentioned in the summary.
frontend/src/custom-hooks/useValidator.ts (4)
16-38
: LGTM: Additional checks improve the robustness of the validator fetching process.The additional checks in the
fetchValidator
function ensure that thegetValidator
action is dispatched only when thevaloperAddress
is not present in both active and inactive validators. This improves the efficiency and robustness of the validator fetching process.Tools
Biome
[error] 26-26: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 27-27: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
40-49
: LGTM: Comprehensive retrieval of validator details.The updated
getValidatorDetails
function provides a more comprehensive retrieval mechanism by considering both active and inactive validators. This ensures that the function returns the most up-to-date validator details.
1-5
: LGTM: Import statements are correctly formatted and necessary.The import statements in this code segment are correctly formatted and necessary for the functionality of the custom hook. They ensure that the required dependencies are available for use within the hook.
10-12
: LGTM: Correct usage of theuseAppSelector
hook.The code segment correctly uses the
useAppSelector
hook to retrieve the wallet connection status from the Redux store. This follows the expected pattern for accessing data from the store within a custom hook.frontend/src/app/(routes)/transfers/components/TransfersPage.tsx (1)
1-1
: Verify the removal of theuseInitBalances
hook and ensure balance data is handled correctly.The
useInitBalances
hook import statement and its invocation have been removed from theTransfersPage
component. This change indicates that the component no longer initializes balances based on the providedchainIDs
when it mounts.Please ensure that:
- The removal of the
useInitBalances
hook is intentional and aligns with the desired behavior.- The balance data is being handled correctly after this change, and there are no unintended consequences on the user experience or data consistency.
To verify the impact of this change, run the following script:
Also applies to: 1-1
Verification successful
Removal of
useInitBalances
hook is verified and balance data is handled correctly.The
useInitBalances
hook is not used elsewhere in the codebase, confirming that its removal from theTransfersPage.tsx
component does not affect other parts of the application. Balance data is being managed through various other components and hooks, ensuring that the application's functionality related to balance data remains intact. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of balance data in the codebase after the removal of the `useInitBalances` hook. # Test 1: Search for the usage of the `useInitBalances` hook. Expect: No occurrences. rg --type typescript $'useInitBalances' # Test 2: Search for the usage of balance data. Expect: Balance data is being fetched and handled correctly. rg --type typescript -A 5 $'balance'Length of output: 144
Script:
#!/bin/bash # Description: Verify the usage of balance data in the codebase after the removal of the `useInitBalances` hook. # Test 1: Search for the usage of the `useInitBalances` hook. Expect: No occurrences. rg --type ts $'useInitBalances' # Test 2: Search for the usage of balance data. Expect: Balance data is being fetched and handled correctly. rg --type ts -A 5 $'balance'Length of output: 105164
frontend/src/components/main-layout/FixedLayout.tsx (3)
30-30
: LGTM!The new import for
useInitApp
is approved.
96-97
: LGTM!The usage of
useInitApp
to initialize the application state is approved.
7-7
: Verify the usage of the removed hooks in the codebase.Ensure that the removed hooks
useFetchPriceInfo
,useInitFeegrant
, anduseInitAuthz
are no longer used in the codebase.Run the following script to verify the usage of the removed hooks:
Verification successful
The removed hooks are still in use elsewhere in the codebase.
The hooks
useFetchPriceInfo
,useInitFeegrant
, anduseInitAuthz
are still being utilized in other parts of the codebase, including within theuseInitApp
hook and various components. Their removal fromFixedLayout.tsx
appears to be part of a refactoring effort rather than an indication of obsolescence.
useFetchPriceInfo
is used infrontend/src/custom-hooks/common/useInitApp.ts
.useInitFeegrant
is used infrontend/src/custom-hooks/common/useInitApp.ts
,frontend/src/components/common/DialogFeegrants.tsx
, andfrontend/src/app/(routes)/settings/feegrant/FeegrantPage.tsx
.useInitAuthz
is used infrontend/src/custom-hooks/common/useInitApp.ts
,frontend/src/components/common/DialogAuthzGrants.tsx
, andfrontend/src/app/(routes)/settings/authz/AuthzPage.tsx
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the removed hooks in the codebase. # Test 1: Search for the usage of `useFetchPriceInfo`. Expect: No occurrences. rg --type typescript -g '!node_modules' $'useFetchPriceInfo' # Test 2: Search for the usage of `useInitFeegrant`. Expect: No occurrences. rg --type typescript -g '!node_modules' $'useInitFeegrant' # Test 3: Search for the usage of `useInitAuthz`. Expect: No occurrences. rg --type typescript -g '!node_modules' $'useInitAuthz'Length of output: 277
Script:
#!/bin/bash # Description: Verify the usage of the removed hooks in the codebase. # Test 1: Search for the usage of `useFetchPriceInfo`. Expect: No occurrences. rg -g '*.ts' -g '*.tsx' -g '!node_modules' $'useFetchPriceInfo' # Test 2: Search for the usage of `useInitFeegrant`. Expect: No occurrences. rg -g '*.ts' -g '*.tsx' -g '!node_modules' $'useInitFeegrant' # Test 3: Search for the usage of `useInitAuthz`. Expect: No occurrences. rg -g '*.ts' -g '*.tsx' -g '!node_modules' $'useInitAuthz'Length of output: 2269
frontend/src/custom-hooks/common/useInitApp.ts (4)
1-21
: LGTM!The imports are correctly used and there are no unused imports.
23-42
: LGTM!The custom hook is correctly defined and the state values are properly retrieved. The
useGetChainInfo
hook is correctly used to get chain and denom info.
43-94
: LGTM!The data fetching logic in the
useEffect
hook is correctly implemented. It fetches all the necessary data conditionally based on theisAuthzMode
value. The hook has the correct dependencies to trigger the data fetching when the wallet connection or authz mode changes.
96-101
: LGTM!The custom hooks
useFetchPriceInfo
,useInitFeegrant
, anduseInitAuthz
are correctly used based on the appropriate conditions.frontend/src/app/(routes)/settings/authz/new-authz/components/ValidatorAutoComplete.tsx (2)
2-2
: LGTM!The code changes are approved.
16-16
: LGTM!The code changes are approved.
frontend/src/app/(routes)/multiops/components/Messages/Delegate.tsx (1)
219-219
: LGTM!The code changes are approved.
frontend/src/custom-hooks/useGetValidatorInfo.ts (4)
11-11
: LGTM!The code changes are approved.
54-64
: LGTM!The code changes are approved. The updated logic improves the specificity of the validator matching process.
137-138
: LGTM!The code changes are approved. The updated conditions ensure that properties are set more robustly using optional chaining and length checks.
Also applies to: 141-142, 145-146
Line range hint
201-224
: LGTM!The code changes are approved. The updated condition allows for a more flexible and comprehensive validation process by checking against the
VITWIT_VALIDATOR_NAMES
array. The additional logic for retrieving and updating validator information for Polygon and Oasis networks is implemented correctly.frontend/src/custom-hooks/useStaking.ts (1)
Line range hint
26-358
: Verify the removal of data-fetching logic and dispatch calls.The AI-generated summary indicates that several data-fetching logic and dispatch calls have been removed from the
useStaking
hook. This is a significant change that might affect the functionality of the hook.Please ensure that the removal of these calls is intentional and that the hook is still functioning as expected in the application.
If the changes are correct, no further action is required. If the changes are incorrect, please revert them and provide more context on how the hook should be refactored.
frontend/src/app/(routes)/multiops/components/Messages/Redelegate.tsx (2)
13-13
: LGTM!The removal of the unused import statement for
getAllValidators
is approved.
390-390
: LGTM!The reformatting of the button is approved.
Summary by CodeRabbit
New Features
useInitApp
, for centralized application state initialization related to blockchain interactions.Improvements
useGetValidatorInfo
hook for improved validator identification and property assignment.Bug Fixes
Refactor