Fix race condition while loading metadata on sign in #9027
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces state management to handle race conditions during metadata loading on sign-in, with key changes focused on synchronizing the authentication and metadata loading processes.
- Added new
ObjectMetadataItemsGatercomponent that gates content rendering until metadata is loaded, displaying a loader in the interim - Introduced
isAppWaitingForFreshObjectMetadataStateRecoil state to track metadata loading status across components - Modified
VerifyEffectto set metadata loading state before token verification - Added debounced state setter in
PageChangeEffectwith 100ms delay to prevent rapid state updates during navigation - Changed default auth modal visibility to true and updated modal display logic for consistent behavior across auth routes
9 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
| setIsAppWaitingForFreshObjectMetadata(true); | ||
| await verify(loginToken); | ||
| } |
There was a problem hiding this comment.
logic: No error handling for verify() call. Could leave app in permanent loading state if verification fails.
| setIsAppWaitingForFreshObjectMetadata(true); | ||
| await verify(loginToken); |
There was a problem hiding this comment.
logic: isAppWaitingForFreshObjectMetadata is never set back to false after verification completes.
| const setIsAppWaitingForFreshObjectMetadataDebounced = useDebouncedCallback( | ||
| () => { | ||
| setIsAppWaitingForFreshObjectMetadata(false); | ||
| }, | ||
| 100, | ||
| ); |
There was a problem hiding this comment.
style: 100ms debounce may be too short for slower connections or heavy metadata loads. Consider increasing to 250-500ms for better reliability.
| if (isDefined(pageChangeEffectNavigateLocation)) { | ||
| navigate(pageChangeEffectNavigateLocation); | ||
| setIsAppWaitingForFreshObjectMetadataDebounced(); | ||
| } |
There was a problem hiding this comment.
logic: Setting metadata state after navigation could lead to flash of incorrect content. Consider setting state before navigation.
| isAppWaitingForFreshObjectMetadataState, | ||
| ); | ||
|
|
||
| const shouldDisplayChildren = !isAppWaitingForFreshObjectMetadata; |
There was a problem hiding this comment.
style: variable name could be more descriptive like isMetadataReady or shouldRenderContent
No description provided.