Skip to content

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

Merged
merged 11 commits into from
May 28, 2025
Merged

Fix Client Config async loading #12308

merged 11 commits into from
May 28, 2025

Conversation

charlesBochet
Copy link
Member

@charlesBochet charlesBochet commented May 27, 2025

Fix ClientConfig async loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a 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 moving isLoaded 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
Copy link
Contributor

github-actions bot commented May 27, 2025

🚀 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.

@FelixMalfait
Copy link
Member

Discussed the enum issue with @Weiko - created a followup issue as there seems to be many inconsistencies across the codebase and this would be a PR larger than the current one #12320

@@ -0,0 +1,15 @@
import { ClientConfig } from '~/generated/graphql';

let cachedClientConfig: ClientConfig | null = null;
Copy link
Member Author

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 :)

Copy link
Member Author

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

@FelixMalfait FelixMalfait merged commit d133055 into main May 28, 2025
52 checks passed
@FelixMalfait FelixMalfait deleted the fix-client-config branch May 28, 2025 08:40
jordan-chalupka pushed a commit to InsurOS/twenty that referenced this pull request May 28, 2025
Fix ClientConfig async loading

---------

Co-authored-by: Félix Malfait <[email protected]>
Co-authored-by: Félix Malfait <[email protected]>
abdulrahmancodes pushed a commit to abdulrahmancodes/twenty that referenced this pull request Jun 2, 2025
Fix ClientConfig async loading

---------

Co-authored-by: Félix Malfait <[email protected]>
Co-authored-by: Félix Malfait <[email protected]>
abdulrahmancodes pushed a commit to abdulrahmancodes/twenty that referenced this pull request Jun 2, 2025
Fix ClientConfig async loading

---------

Co-authored-by: Félix Malfait <[email protected]>
Co-authored-by: Félix Malfait <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants