#1127 fix github verification #1183
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@adrianvrj is attempting to deploy a commit to the LFG Labs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis change introduces a new identity refresh mechanism using React context and hooks. It adds an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant IdentityRefreshProvider
participant AvailableIdentities
participant useNotificationManager
User->>App: Initiates verification (e.g., GitHub)
App->>useNotificationManager: Handles transaction notification
useNotificationManager->>IdentityRefreshProvider: Calls refreshIdentity(tokenId) after success
IdentityRefreshProvider->>AvailableIdentities: Triggers registered refresh callback for tokenId
AvailableIdentities->>AvailableIdentities: Updates identity data (refreshData)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/identities/availableIdentities.tsx(2 hunks)components/providers/IdentityRefreshProvider.tsx(1 hunks)hooks/useIdentityRefresh.ts(1 hunks)hooks/useNotificationManager.ts(4 hunks)pages/_app.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the `registerv3` component (`components/domains/registerv3.tsx`), the `setdomain` prop is kept de...
Learnt from: Viktohblake
PR: lfglabs-dev/app.starknet.id#995
File: components/domains/registerV3.tsx:19-21
Timestamp: 2025-01-26T19:18:53.841Z
Learning: In the `RegisterV3` component (`components/domains/registerV3.tsx`), the `setDomain` prop is kept despite being unused because removing it would require changes to dependent files. This prop is marked with a comment indicating it's not used since the domain search bar was removed.
Applied to files:
components/identities/availableIdentities.tsx
🧬 Code Graph Analysis (4)
pages/_app.tsx (1)
components/providers/IdentityRefreshProvider.tsx (1)
IdentityRefreshProvider(8-37)
components/providers/IdentityRefreshProvider.tsx (1)
hooks/useIdentityRefresh.ts (1)
IdentityRefreshContext(9-9)
components/identities/availableIdentities.tsx (1)
hooks/useIdentityRefresh.ts (1)
useIdentityRefresh(11-17)
hooks/useNotificationManager.ts (1)
hooks/useIdentityRefresh.ts (1)
useIdentityRefresh(11-17)
🪛 Biome (2.1.2)
components/identities/availableIdentities.tsx
[error] 41-41: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
hooks/useNotificationManager.ts
[error] 25-25: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 ESLint
hooks/useNotificationManager.ts
[error] 27-29: Empty block statement.
(no-empty)
🔇 Additional comments (7)
hooks/useIdentityRefresh.ts (1)
1-17: Well-structured React context implementation.The hook follows standard React context patterns with proper error handling. The interface clearly defines the contract and the error message provides helpful guidance for developers.
pages/_app.tsx (2)
20-20: Good integration of the provider import.The import is properly placed and follows the existing import structure.
56-72: Proper provider hierarchy and placement.The IdentityRefreshProvider is correctly positioned within the component tree, ensuring the identity refresh context is available throughout the application while maintaining logical provider nesting.
components/providers/IdentityRefreshProvider.tsx (1)
1-37: Excellent provider implementation.The use of
useReffor the callbacks Map is appropriate since callback changes shouldn't trigger re-renders. All methods are properly memoized withuseCallbackfor stable references, and the implementation efficiently manages the callback registry.components/identities/availableIdentities.tsx (1)
164-175: Update effect dependencies and cleanup logic.The registration and cleanup logic is well-structured, but ensure the effect dependencies are updated after fixing the conditional hook usage.
hooks/useNotificationManager.ts (2)
71-84: Good verification transaction handling logic.The logic to trigger identity refreshes after successful verification transactions is well-implemented:
- Correctly identifies verification transaction types
- Properly extracts token ID from notification subtext using regex
- Uses appropriate delay to ensure backend processing completes
94-94: Update effect dependencies after fixing hook usage.Remember to update the effect dependencies after fixing the conditional hook usage above.
| let registerRefreshCallback: ((tokenId: string, callback: () => void) => void) | null = null; | ||
| let unregisterRefreshCallback: ((tokenId: string) => void) | null = null; | ||
| try { | ||
| const context = useIdentityRefresh(); | ||
| registerRefreshCallback = context.registerRefreshCallback; | ||
| unregisterRefreshCallback = context.unregisterRefreshCallback; | ||
| } catch { | ||
| console.error("Unable to refresh user identity."); | ||
| } |
There was a problem hiding this comment.
Critical: Fix conditional hook usage violation.
The useIdentityRefresh hook is being called conditionally inside a try-catch block, which violates React's Rules of Hooks. This can cause unpredictable behavior and state corruption.
Apply this diff to fix the hooks violation:
- let registerRefreshCallback: ((tokenId: string, callback: () => void) => void) | null = null;
- let unregisterRefreshCallback: ((tokenId: string) => void) | null = null;
- try {
- const context = useIdentityRefresh();
- registerRefreshCallback = context.registerRefreshCallback;
- unregisterRefreshCallback = context.unregisterRefreshCallback;
- } catch {
- console.error("Unable to refresh user identity.");
- }
+ const { registerRefreshCallback, unregisterRefreshCallback } = useIdentityRefresh();If you need to handle the case where the provider might not be available, consider using a default context value or ensuring the provider is always present in the component tree.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let registerRefreshCallback: ((tokenId: string, callback: () => void) => void) | null = null; | |
| let unregisterRefreshCallback: ((tokenId: string) => void) | null = null; | |
| try { | |
| const context = useIdentityRefresh(); | |
| registerRefreshCallback = context.registerRefreshCallback; | |
| unregisterRefreshCallback = context.unregisterRefreshCallback; | |
| } catch { | |
| console.error("Unable to refresh user identity."); | |
| } | |
| const { registerRefreshCallback, unregisterRefreshCallback } = useIdentityRefresh(); |
🧰 Tools
🪛 Biome (2.1.2)
[error] 41-41: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In components/identities/availableIdentities.tsx around lines 38 to 46, the
useIdentityRefresh hook is called inside a try-catch block, causing a
conditional hook usage violation. To fix this, move the useIdentityRefresh call
outside of any conditional or try-catch blocks so it is always called
unconditionally at the top level of the component. Then handle the case where
the context might be undefined by providing a default context value or checking
the context after the hook call, rather than catching errors from the hook call
itself.
| let refreshIdentity: ((tokenId: string) => void) | null = null; | ||
| try { | ||
| const context = useIdentityRefresh(); | ||
| refreshIdentity = context.refreshIdentity; | ||
| } catch { | ||
|
|
||
| } |
There was a problem hiding this comment.
Critical: Fix conditional hook usage and empty catch block.
Two issues here:
- The
useIdentityRefreshhook is called conditionally, violating React's Rules of Hooks - Empty catch block provides no error handling
Apply this diff to fix both issues:
- let refreshIdentity: ((tokenId: string) => void) | null = null;
- try {
- const context = useIdentityRefresh();
- refreshIdentity = context.refreshIdentity;
- } catch {
-
- }
+ const { refreshIdentity } = useIdentityRefresh();If the provider might not be available, consider using a default context value or ensuring the provider is always present.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let refreshIdentity: ((tokenId: string) => void) | null = null; | |
| try { | |
| const context = useIdentityRefresh(); | |
| refreshIdentity = context.refreshIdentity; | |
| } catch { | |
| } | |
| const { refreshIdentity } = useIdentityRefresh(); |
🧰 Tools
🪛 Biome (2.1.2)
[error] 25-25: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 ESLint
[error] 27-29: Empty block statement.
(no-empty)
🤖 Prompt for AI Agents
In hooks/useNotificationManager.ts around lines 23 to 29, the useIdentityRefresh
hook is called inside a try-catch block, causing conditional hook usage which
violates React's Rules of Hooks, and the catch block is empty providing no error
handling. To fix this, remove the try-catch and call useIdentityRefresh
unconditionally. Handle the case where the provider might not be available by
using a default context value in the provider or by ensuring the provider is
always present, so the hook never throws. This eliminates conditional hook calls
and removes the need for an empty catch block.
1 participant
Summary
This PR fixes GitHub verification by implementing an identity refresh system that automatically
updates user identities after successful verification transactions.
Changes
(IdentityRefreshProvider) and hook (useIdentityRefresh) to manage identity refresh callbacks
across components
callbacks and handle identity updates when verification transactions complete
when verification transactions (GitHub, Twitter, Discord, POP) are successful
_app.tsx to enable the refresh system
Technical Details
The fix works by:
successfully
Files Modified
triggering
Files Added
components/providers/IdentityRefreshProvider.tsx - Context provider for refresh system
hooks/useIdentityRefresh.ts - Hook for accessing refresh functionality
Closes Github verification bug #1127
Summary by CodeRabbit
New Features
Chores