-
-
Couldn't load subscription status.
- Fork 10.3k
fix(studio): fallback to count estimate if exact count query timeout #35774
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
5 Skipped Deployments
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
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.
LGTM!
- Tested with a partitioned table on preview. Note I couldn't trigger a timeout, so just testing the happy path.
One small code note:
| useEffect(() => { | ||
| // If the count query encountered a timeout error with exact count | ||
| // turn off the exact count to rely on approximate | ||
| if (isError && snap.enforceExactCount && error.code === 408) { |
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.
If you don't want to include snap.enforceExactCount and error.code in the dependency array, you should wrap them in useLatest() outside the useEffect so we don't accidentally read stale values here!
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.
Oh ! I was wondering since snap.setPage wasn't a deps here: https://github.com/supabase/supabase/pull/35774/files#diff-357ec6b1c5b04e3a99a116ebe222a9682dc4cbb1ab1bc175202db3a6fb6193c6R124-R132
I wondered if maybe there was some case where we don't wanna mention all dependencies (methods, derived properties, ...).
But I don't mind declaring them all in the array of dependencies. Good spot.
I have read the CONTRIBUTING.md file.
YES/NO
What kind of change does this PR introduce?
Bug fix, feature, docs update, ...
What is the current behavior?
Please link any relevant issues here.
What is the new behavior?
Feel free to include screenshots if it includes visual changes.
Additional context
Add any other context or screenshots.