Skip to content

fix(client-config): set isLoaded to false on API status update #12371

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 30, 2025

Conversation

AMoreaux
Copy link
Contributor

@AMoreaux AMoreaux commented May 28, 2025

Attempt at #12289 (edit Félix: removed fix keyword since I don't think it fixes it)

@AMoreaux AMoreaux requested a review from charlesBochet May 28, 2025 20:20
@AMoreaux AMoreaux self-assigned this May 28, 2025
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 modifies the isLoaded flag in the client configuration loading logic, introducing a potential bug where successful data fetches incorrectly set isLoaded to false.

  • In /packages/twenty-front/src/modules/client-config/hooks/useClientConfig.ts, the isLoaded flag is incorrectly set to false after successful data fetch, which could trigger infinite loading loops
  • The ClientConfigProviderEffect component relies on isLoaded to determine when to fetch data, making this change particularly problematic
  • This change contradicts the logic in ClientConfigProvider which uses this state to handle error displays

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

github-actions bot commented May 28, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:20509

This environment will automatically shut down when the PR is closed or after 5 hours.

@FelixMalfait FelixMalfait self-assigned this May 29, 2025
@FelixMalfait
Copy link
Member

Hey @AMoreaux thanks

  1. While this might be a real bug I don't think it's the cause of the issue you linked since this new client-config mechanism is still on main and hasn't been deployed to prod
  2. We cannot remove the line like you did because other elements rely on isLoaded being set to true. I can take over your PR and try to do something that works

@FelixMalfait FelixMalfait merged commit b747337 into main May 30, 2025
52 checks passed
@FelixMalfait FelixMalfait deleted the fix/12289 branch May 30, 2025 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants