-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Fix Client Config async loading #12308
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
Conversation
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.
PR Summary
This PR fixes async loading issues in client configuration by ensuring proper loading state management and preventing race conditions in data fetching.
- Fixed race condition in
ClientConfigProviderEffect.tsx
by movingisLoaded
state update after all client config data is loaded - Added loading check in
useGetPublicWorkspaceDataByDomain.ts
to prevent premature API calls before client config is ready - Improved error handling in domain management to redirect to default domain on failures
- Reordered imports across multiple files for better code organization and maintainability
5 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
Changes for performance improvement. The primary improvements include replacing GraphQL queries with REST-based client configuration fetching and making the client config non render-blocking
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:21365 This environment will automatically shut down when the PR is closed or after 5 hours. |
@@ -0,0 +1,15 @@ | |||
import { ClientConfig } from '~/generated/graphql'; | |||
|
|||
let cachedClientConfig: ClientConfig | null = null; |
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.
still need to fix this :)
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.
why don't we use a recoilState here? we actually already have it
Fix ClientConfig async loading --------- Co-authored-by: Félix Malfait <[email protected]> Co-authored-by: Félix Malfait <[email protected]>
Fix ClientConfig async loading --------- Co-authored-by: Félix Malfait <[email protected]> Co-authored-by: Félix Malfait <[email protected]>
Fix ClientConfig async loading --------- Co-authored-by: Félix Malfait <[email protected]> Co-authored-by: Félix Malfait <[email protected]>
Fix ClientConfig async loading