chore: Fix all the react lint errors #277
Conversation
|
Warning Rate limit exceeded@hansl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 5 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces several dependency array updates in memoization and effects across multiple components, ensuring dynamic recalculations upon prop and state changes. The changes also include replacing standard Changes
Sequence Diagram(s)sequenceDiagram
participant IbcSendForm
participant State
IbcSendForm->>State: useEffect triggers when [ibcChains, isGroup] change
State-->>IbcSendForm: Provide updated chain info
IbcSendForm->>IbcSendForm: Recompute filteredBalances & initialSelectedToken via useMemo
sequenceDiagram
participant CountdownTimer
participant Timer
CountdownTimer->>Timer: setInterval (1-sec updates)
Timer-->>CountdownTimer: Update current time ("now")
CountdownTimer->>CountdownTimer: Recompute timeLeft using useMemo
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #277 +/- ##
==========================================
+ Coverage 53.51% 53.53% +0.01%
==========================================
Files 210 209 -1
Lines 17228 17233 +5
==========================================
+ Hits 9220 9226 +6
+ Misses 8008 8007 -1 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
components/wallet.tsx (1)
181-184: Enhance error handling inonClickConnect.The error handling could be improved by providing more context and potentially showing an error notification to the user.
const onClickConnect: MouseEventHandler = e => { e.preventDefault(); - connect().catch(console.error); + connect().catch(error => { + console.error('Failed to connect wallet:', error); + // Show error notification to user + }); };🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 181-181: components/wallet.tsx#L181
Added line #L181 was not covered by tests
[warning] 183-183: components/wallet.tsx#L183
Added line #L183 was not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
components/3js/animatedMesh.tsx(1 hunks)components/admins/modals/validatorModal.tsx(3 hunks)components/bank/components/tokenList.tsx(1 hunks)components/bank/forms/ibcSendForm.tsx(1 hunks)components/bank/forms/sendForm.tsx(2 hunks)components/factory/components/__tests__/DenomImage.test.tsx(1 hunks)components/groups/components/CountdownTimer.tsx(2 hunks)components/react/authSignerModal.tsx(3 hunks)components/react/chain-card.tsx(0 hunks)components/react/index.ts(0 hunks)components/react/modal.tsx(2 hunks)components/types.tsx(0 hunks)components/wallet.tsx(3 hunks)
💤 Files with no reviewable changes (3)
- components/types.tsx
- components/react/index.ts
- components/react/chain-card.tsx
🧰 Additional context used
🧠 Learnings (1)
components/react/authSignerModal.tsx (1)
Learnt from: fmorency
PR: liftedinit/manifest-app#257
File: components/react/authSignerModal.tsx:313-313
Timestamp: 2025-02-12T14:01:23.163Z
Learning: The `SignModal` component in `components/react/authSignerModal.tsx` should use native `<img>` tags instead of `next/image` to maintain framework agnosticism and reusability across different React applications.
🪛 Biome (1.9.4)
components/react/modal.tsx
[error] 457-457: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 458-458: Forbidden extra non-null assertion.
Safe fix: Remove extra non-null assertion.
(lint/suspicious/noExtraNonNullAssertion)
[error] 462-462: Forbidden extra non-null assertion.
Safe fix: Remove extra non-null assertion.
(lint/suspicious/noExtraNonNullAssertion)
🪛 GitHub Check: codecov/patch
components/react/modal.tsx
[warning] 398-417: components/react/modal.tsx#L398-L417
Added lines #L398 - L417 were not covered by tests
[warning] 420-420: components/react/modal.tsx#L420
Added line #L420 was not covered by tests
[warning] 423-430: components/react/modal.tsx#L423-L430
Added lines #L423 - L430 were not covered by tests
[warning] 433-436: components/react/modal.tsx#L433-L436
Added lines #L433 - L436 were not covered by tests
[warning] 439-439: components/react/modal.tsx#L439
Added line #L439 was not covered by tests
[warning] 442-452: components/react/modal.tsx#L442-L452
Added lines #L442 - L452 were not covered by tests
[warning] 455-455: components/react/modal.tsx#L455
Added line #L455 was not covered by tests
[warning] 457-462: components/react/modal.tsx#L457-L462
Added lines #L457 - L462 were not covered by tests
[warning] 465-473: components/react/modal.tsx#L465-L473
Added lines #L465 - L473 were not covered by tests
[warning] 476-478: components/react/modal.tsx#L476-L478
Added lines #L476 - L478 were not covered by tests
[warning] 481-489: components/react/modal.tsx#L481-L489
Added lines #L481 - L489 were not covered by tests
[warning] 492-499: components/react/modal.tsx#L492-L499
Added lines #L492 - L499 were not covered by tests
[warning] 501-503: components/react/modal.tsx#L501-L503
Added lines #L501 - L503 were not covered by tests
[warning] 506-515: components/react/modal.tsx#L506-L515
Added lines #L506 - L515 were not covered by tests
[warning] 520-522: components/react/modal.tsx#L520-L522
Added lines #L520 - L522 were not covered by tests
[warning] 524-525: components/react/modal.tsx#L524-L525
Added lines #L524 - L525 were not covered by tests
components/wallet.tsx
[warning] 116-116: components/wallet.tsx#L116
Added line #L116 was not covered by tests
[warning] 181-181: components/wallet.tsx#L181
Added line #L181 was not covered by tests
[warning] 183-183: components/wallet.tsx#L183
Added line #L183 was not covered by tests
[warning] 186-186: components/wallet.tsx#L186
Added line #L186 was not covered by tests
[warning] 188-193: components/wallet.tsx#L188-L193
Added lines #L188 - L193 were not covered by tests
[warning] 195-209: components/wallet.tsx#L195-L209
Added lines #L195 - L209 were not covered by tests
[warning] 211-224: components/wallet.tsx#L211-L224
Added lines #L211 - L224 were not covered by tests
[warning] 226-264: components/wallet.tsx#L226-L264
Added lines #L226 - L264 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (12)
components/factory/components/__tests__/DenomImage.test.tsx (1)
18-18: LGTM! ESLint suppressions are appropriate for test mocks.The ESLint disable comments are correctly used to suppress warnings about image elements in the test environment where we're mocking Next.js' Image component.
components/groups/components/CountdownTimer.tsx (1)
4-20: Great optimization of the countdown timer logic!The refactoring improves the component by:
- Using a single source of truth for current time
- Properly cleaning up the interval on unmount
- Memoizing time calculations with correct dependencies
components/3js/animatedMesh.tsx (1)
60-60: Correct fix for geometry recalculation!Adding
shapeGeoto the dependency array ensures that the geometry is properly recreated when the shape type changes, preventing stale renders.components/bank/components/tokenList.tsx (1)
85-85: Good optimization of skeleton items memoization!Removing
totalPagesfrom the dependency array is correct since skeleton items only depend onpageSize. This prevents unnecessary recalculations whentotalPageschanges.components/wallet.tsx (1)
116-116:❓ Verification inconclusive
Verify the dependency array change.
The dependency array for
_renderWalletContenthas been simplified by removingusernameandaddress. While this may improve performance by reducing re-renders, verify that the component still updates correctly when these values change.
🏁 Script executed:
#!/bin/bash # Search for usages of username or address within the component's render logic rg -A 5 'username|address' components/wallet.tsxLength of output: 2471
Dependency Array Verification for Memoized Render Logic
The dependency array for
_renderWalletContentnow only includes[localStatus, connect, openView]. However, sinceusernameandaddressare still used elsewhere in the render logic (as verified by the grep output), please ensure that omitting them from the dependency array does not lead to stale renders when these values change. Verify that any UI elements relying onusernameoraddressupdate as expected when these values are updated.
- Confirm that excluding
usernameandaddressfrom the_renderWalletContentdependencies does not affect the component's re-rendering behavior.- Consider revisiting whether these values should be added back to the dependency array if they directly affect the memoized output.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 116-116: components/wallet.tsx#L116
Added line #L116 was not covered by testscomponents/react/authSignerModal.tsx (1)
343-343: Approve the simplified walletIconString assignment.Good simplification of the walletIconString assignment by removing the unnecessary nullish coalescing operator.
components/bank/forms/sendForm.tsx (1)
53-55: LGTM! Improved memoization dependencies.Good improvement to include
balancesin the dependency array. This ensuresinitialSelectedTokenupdates correctly when balances change.components/react/modal.tsx (3)
51-68: LGTM! Enhanced error handling.The addition of new error types and the updated error checking function improve the robustness of the wallet connection flow.
156-156: LGTM! Fixed hook dependencies.Adding
setQrMessageandsetQRStateto the dependency array ensures proper state updates and aligns with React's rules of hooks.
386-550:❓ Verification inconclusive
Add test coverage for modal views.
The refactored
_renderfunction and its various modal views lack test coverage. Consider adding tests for:
- Email/SMS input flows
- Connected/Connecting states
- QR code view
- Error handling
- Contact selection
Run the following script to check current test coverage:
🏁 Script executed:
#!/bin/bash # Description: Check test coverage for modal views # Search for existing tests rg -A 5 "describe.*TailwindModal|test.*TailwindModal|it.*TailwindModal" --type testLength of output: 114
Please Add or Verify Test Coverage for Modal Views
The refactored
_renderfunction still lacks any automated test coverage for the various modal states. The initial test search usingrg --type testreturned an error instead of a clear result, so it’s unclear if tests exist under a different naming convention. Please verify manually that tests are in place or add them for:
- Email/SMS Input Flows: Validate that the email and SMS flows set the login hint and handle connection errors appropriately.
- Connected/Connecting States: Ensure that the transitions for connected and connecting states behave correctly, especially with tailored messages.
- QR Code View: Confirm that the QR code modal renders and responds to user actions as expected.
- Error Handling: Check that error conditions trigger the proper UI responses and callbacks.
- Contact Selection: Test that the contacts view supports both selection modes, including the message edit modal trigger.
🧰 Tools
🪛 Biome (1.9.4)
[error] 457-457: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 458-458: Forbidden extra non-null assertion.
Safe fix: Remove extra non-null assertion.
(lint/suspicious/noExtraNonNullAssertion)
[error] 462-462: Forbidden extra non-null assertion.
Safe fix: Remove extra non-null assertion.
(lint/suspicious/noExtraNonNullAssertion)
🪛 GitHub Check: codecov/patch
[warning] 398-417: components/react/modal.tsx#L398-L417
Added lines #L398 - L417 were not covered by tests
[warning] 420-420: components/react/modal.tsx#L420
Added line #L420 was not covered by tests
[warning] 423-430: components/react/modal.tsx#L423-L430
Added lines #L423 - L430 were not covered by tests
[warning] 433-436: components/react/modal.tsx#L433-L436
Added lines #L433 - L436 were not covered by tests
[warning] 439-439: components/react/modal.tsx#L439
Added line #L439 was not covered by tests
[warning] 442-452: components/react/modal.tsx#L442-L452
Added lines #L442 - L452 were not covered by tests
[warning] 455-455: components/react/modal.tsx#L455
Added line #L455 was not covered by tests
[warning] 457-462: components/react/modal.tsx#L457-L462
Added lines #L457 - L462 were not covered by tests
[warning] 465-473: components/react/modal.tsx#L465-L473
Added lines #L465 - L473 were not covered by tests
[warning] 476-478: components/react/modal.tsx#L476-L478
Added lines #L476 - L478 were not covered by tests
[warning] 481-489: components/react/modal.tsx#L481-L489
Added lines #L481 - L489 were not covered by tests
[warning] 492-499: components/react/modal.tsx#L492-L499
Added lines #L492 - L499 were not covered by tests
[warning] 501-503: components/react/modal.tsx#L501-L503
Added lines #L501 - L503 were not covered by tests
[warning] 506-515: components/react/modal.tsx#L506-L515
Added lines #L506 - L515 were not covered by tests
[warning] 520-522: components/react/modal.tsx#L520-L522
Added lines #L520 - L522 were not covered by tests
[warning] 524-525: components/react/modal.tsx#L524-L525
Added lines #L524 - L525 were not covered by testscomponents/bank/forms/ibcSendForm.tsx (2)
91-91: LGTM! Fixed hook dependencies.Adding
ibcChainsto the dependency array ensures that the initial chain is correctly set when the available chains change.
99-99: Optimize memoization dependencies.The changes to the dependency arrays in
filteredBalancesandinitialSelectedTokenhave different impacts:
- Removing
selectedFromChainfromfilteredBalancesis correct as it's not used in the calculation.- Adding
balancestoinitialSelectedTokenensures proper updates when balances change.Also applies to: 104-104
Now
bun run builddoes not show any lint errors, and also fixed some re-rendering of components in some cases.This should have no visual or functional effect on the application.
Summary by CodeRabbit
New Features
Refactor
Tests