fix: provider list not refreshing after adding new provider#546
fix: provider list not refreshing after adding new provider#546ding113 merged 1 commit intoding113:devfrom
Conversation
Move AddProviderDialog creation from Server Component (page.tsx) to Client Component (ProviderManagerLoaderContent) to ensure useQueryClient() correctly accesses the QueryClient instance within QueryClientProvider. This fixes the issue where invalidateQueries() calls did not trigger React Query to refetch data after adding a new provider.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 Walkthrough漫步本次变更删除了 AddProviderDialog 组件在多个页面中的导入和使用,并调整了 ProviderManagerLoader 的属性接口,将 addDialogSlot 属性从可配置变更为内联固定实现。 变化
代码审查工作量估计🎯 3 (Moderate) | ⏱️ ~20 分钟 Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @NieiR, 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 a critical bug where newly added providers were not immediately reflected in the provider list. The root cause was identified as Highlights
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 the issue of the provider list not refreshing by moving the AddProviderDialog instantiation into a client component that is a child of QueryClientProvider. This ensures it can access the correct QueryClient context. The changes are logical and well-explained. The removal of a duplicate 'Add Provider' button also cleans up the UI.
I have one critical piece of feedback regarding the instantiation of QueryClient in provider-manager-loader.tsx. It's currently created at the module level, which can lead to data leakage between users during server-side rendering. I've left a detailed comment with a suggested fix to align with React Query's best practices for Next.js.
src/app/[locale]/settings/providers/_components/provider-manager-loader.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The fix correctly moves AddProviderDialog instantiation from Server Components into the Client Component (ProviderManagerLoaderContent) where the QueryClientProvider context is available. This ensures useQueryClient() can properly access the QueryClient instance for invalidateQueries() to work correctly.
PR Size: XS
- Lines changed: 10 (2 additions, 8 deletions)
- Files changed: 3
Technical Assessment
The root cause analysis in the PR description is accurate: when AddProviderDialog was instantiated in Server Components and passed as a slot prop, the component tree boundary prevented useQueryClient() from accessing the QueryClient instance created in ProviderManagerLoader. By moving the instantiation inside ProviderManagerLoaderContent (which is wrapped by QueryClientProvider), the hook can now correctly access the context.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - N/A (no doc changes)
- Test coverage - Adequate (manual test plan provided)
- Code clarity - Good
Automated review by Claude AI
Summary
AddProviderDialogcreation from Server Component to Client Component (ProviderManagerLoaderContent)useQueryClient()correctly accesses theQueryClientinstance withinQueryClientProviderRoot Cause
AddProviderDialogwas previously created in Server Components (page.tsx) and passed as a slot prop. Due to React Server/Client Component boundary handling,useQueryClient()could not correctly access the samequeryClientinstance, causinginvalidateQueries()calls to fail to trigger data refetch.Changes
provider-manager-loader.tsxAddProviderDialoginternally instead of receiving via propsettings/providers/page.tsxaddDialogSlotpropdashboard/providers/page.tsxAddProviderDialogfromSection.actionsTest plan
/settings/providerspage/dashboard/providerspageGreptile Summary
This PR fixes the provider list not refreshing after adding a new provider by correctly managing React Server/Client Component boundaries. The root cause was that
AddProviderDialogwas being instantiated in Server Components and passed as a prop, which broke theQueryClientProvidercontext needed foruseQueryClient()to work correctly.Changes Made:
AddProviderDialoginstantiation from Server Components (page.tsxfiles) into the Client ComponentProviderManagerLoaderContent, which is wrapped byQueryClientProvideraddDialogSlotprop fromProviderManagerLoadersince the dialog is now created internallyAddProviderDialoginstantiation in/dashboard/providers, ensuring a single source of truth for dialog managementqueryClient.invalidateQueries()on successful provider creation, triggering the expected data refetchAll changes follow proper Next.js App Router patterns by keeping query client operations within the Client Component boundary where
QueryClientProvideris available.Confidence Score: 5/5
ProviderManagerLoaderis simplified but functional; (3) The fix directly addresses the root cause -useQueryClient()can now access the sameQueryClientinstance within the provider boundary; (4) No new dependencies or complex logic was introduced; (5) The pattern aligns with React and Next.js best practices for handling Client Components within Server Components.Important Files Changed
AddProviderDialoginstantiation from Server Components into this Client Component. Now the dialog is created insideQueryClientProvider, ensuringuseQueryClient()has access to the correct context instance. RemovedaddDialogSlotprop and imports. This fixes the core issue where query invalidation wasn't working.AddProviderDialogimport andaddDialogSlotprop fromProviderManagerLoader. This Server Component no longer needs to instantiate the dialog. Changes align with the fix to move dialog creation to the Client Component boundary.AddProviderDialogimport and instantiation fromSection.actions. The dialog is now consistently managed only byProviderManagerLoader, preventing duplicate buttons and ensuring a single source of truth for dialog state and query invalidation.Sequence Diagram
sequenceDiagram participant SC as Server Component<br/>(page.tsx) participant QCP as QueryClientProvider participant CC as ProviderManagerLoaderContent<br/>(Client Component) participant APD as AddProviderDialog<br/>(Client Component) participant QC as QueryClient rect rgb(200, 220, 255) Note over SC,QC: BEFORE FIX: AddProviderDialog created in Server Component SC->>SC: Create AddProviderDialog instance SC->>QCP: Pass addDialogSlot prop Note over APD: useQueryClient() fails - outside QueryClientProvider context end rect rgb(200, 255, 220) Note over SC,QC: AFTER FIX: AddProviderDialog created inside Client Component SC->>QCP: Render QueryClientProvider QCP->>CC: Render ProviderManagerLoaderContent CC->>APD: Create AddProviderDialog (inside QueryClientProvider context) APD->>QC: useQueryClient() now has access APD->>APD: onSuccess: invalidateQueries(['providers']) QC->>CC: Refetch data end