fix: correct useEducationStatus query cache configuration#30416
Conversation
- Replace ineffective gcTime: 0 with proper cache controls - Add staleTime: 0, refetchOnMount, and refetchOnWindowFocus - Use isFetchedAfterMount to prevent stale data display - Follows TanStack Query best practices for provider-level queries
Summary of ChangesHello @lyzno1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a caching issue with the useEducationStatus query by updating the TanStack Query configuration to ensure data is always fresh. The use of isFetchedAfterMount to prevent displaying stale data is a good improvement. I've found one potential bug related to the use of the logical OR operator || instead of the nullish coalescing operator ??, which could lead to incorrect data handling for falsy values. My suggestion addresses this.
There was a problem hiding this comment.
Pull request overview
This PR fixes the query cache configuration for useEducationStatus by replacing the ineffective gcTime: 0 with proper cache controls. The change addresses an issue where gcTime (garbage collection time) was being used to prevent stale data, but this setting only affects inactive queries and was therefore ineffective for the provider-level query that remains active.
Key Changes:
- Replaced
gcTime: 0withstaleTime: 0to ensure data is immediately marked as stale - Added explicit refetch configurations to handle component mount and window focus scenarios
- Modified the provider context to use
isFetchedAfterMountflag to prevent displaying cached data before fresh fetch completes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
web/service/use-education.ts | Updated query configuration to use staleTime: 0 and added explicit refetch options instead of ineffective gcTime: 0 |
web/context/provider-context.tsx | Added conditional logic using isFetchedAfterMount to prevent displaying cached education data before fresh fetch completes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Replace || with ?? to prevent issues with falsy values - Fixes potential bug where expire_at: 0 would be incorrectly converted to null - Improves code clarity and follows best practices
- Remove refetchOnMount (default: true) - Remove refetchOnWindowFocus (default: true) - Keep only non-default configuration: staleTime: 0
Part of #30414
Fixes #30415
Summary
Replaced ineffective
gcTime: 0with proper cache controls for provider-level query.Changes
Hook Configuration
gcTime: 0(ineffective for active queries)staleTime: 0(ensures data marked stale immediately)Component Usage
isFetchedAfterMountto prevent displaying cached data before fresh fetchWhy
Per TanStack Query docs,
gcTimeonly applies to inactive queries. Provider-level queries remain active, makinggcTime: 0ineffective.Testing