fix: solid-query-devtools and panel query client #9763
Conversation
🦋 Changeset detectedLatest commit: 1c060af The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughUpdates Solid Query Devtools to accept an optional Changes
Sequence Diagram(s)sequenceDiagram autonumber actor App participant Devtools as SolidQueryDevtools participant Hook as useQueryClient participant Ctx as QueryClientProvider participant Panel as TanStack Devtools Panel App->>Devtools: render({ client? }) Devtools->>Hook: useQueryClient(props.client) alt client prop provided Hook-->>Devtools: return props.client else context provides client Hook->>Ctx: read context Ctx-->>Hook: return context client Hook-->>Devtools: client from context else none available Hook-->>Devtools: throw "No QueryClient" Devtools-->>App: render error end Devtools->>Panel: render({ client }) Panel-->>App: mounted Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/solid-query-devtools/src/devtoolsPanel.tsx (1)
45-46: Fix correctly addresses issue #9762.The changes properly forward
props.clienttouseQueryClient, enabling the devtools to work outside a Query context when an explicit client is provided.One minor observation:
queryClientfromuseQueryClientis a constant value, so wrapping it increateMemomay not provide reactivity benefits. However, this pattern could be intentional for consistency with Solid's reactive patterns or defensive coding for future changes.Consider simplifying to use
queryClientdirectly if the memo serves no reactive purpose:- const queryClient = useQueryClient(props.client) - const client = createMemo(() => queryClient) + const client = useQueryClient(props.client)Then update references from
client()toclient:const devtools = new TanstackQueryDevtoolsPanel({ - client: client(), + client: client,createEffect(() => { - devtools.setClient(client()) + devtools.setClient(client) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/solid-query-devtools/package.json(2 hunks)packages/solid-query-devtools/src/__tests__/devtools.test.tsx(1 hunks)packages/solid-query-devtools/src/__tests__/devtoolsPanel.test.tsx(1 hunks)packages/solid-query-devtools/src/devtools.tsx(1 hunks)packages/solid-query-devtools/src/devtoolsPanel.tsx(1 hunks)packages/solid-query-devtools/test-setup.ts(1 hunks)packages/solid-query-devtools/vite.config.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T17:57:33.184Z
Learnt from: TkDodo PR: TanStack/query#9612 File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0 Timestamp: 2025-09-02T17:57:33.184Z Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json. Applied to files:
packages/solid-query-devtools/package.json
🧬 Code graph analysis (3)
packages/solid-query-devtools/src/__tests__/devtools.test.tsx (1)
packages/solid-query-devtools/src/devtools.tsx (1)
SolidQueryDevtools(50-100)
packages/solid-query-devtools/vite.config.ts (1)
packages/angular-query-experimental/scripts/prepack.js (1)
packageJson(67-67)
packages/solid-query-devtools/src/__tests__/devtoolsPanel.test.tsx (1)
packages/solid-query-devtools/src/devtoolsPanel.tsx (1)
SolidQueryDevtoolsPanel(44-86)
🔇 Additional comments (1)
packages/solid-query-devtools/src/__tests__/devtoolsPanel.test.tsx (1)
7-31: Excellent test coverage for the fix.The tests validate all three key scenarios:
- Error when no QueryClient is available
- Success when QueryClient is provided via context
- Success when QueryClient is provided via props (addresses issue #9762)
The test structure is clear and the coverage is appropriate for validating the devtools component behavior.
packages/solid-query-devtools/src/__tests__/devtoolsPanel.test.tsx Outdated Show resolved Hide resolved
| Hey this is my first time contributing to this repo so let me know if I've done anything wrong. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/solid-query-devtools/src/__tests__/devtoolsPanel.test.tsx (1)
6-31: LGTM! Test coverage appropriately validates the client prop forwarding fix.The test suite correctly covers the three key scenarios for QueryClient provisioning:
- Error when no client is available
- Successful rendering with context-provided client
- Successful rendering with prop-provided client (the fix for issue #9762)
The suite name is now correctly singular to match the component name.
Optional improvements for test robustness:
Consider these refinements to strengthen the test suite:
- Add cleanup to prevent state leakage:
it('should not throw an error if query client is provided via context', () => { const queryClient = new QueryClient() + onCleanup(() => queryClient.clear()) expect(() => render(() => ( <QueryClientProvider client={queryClient}> <SolidQueryDevtoolsPanel /> </QueryClientProvider> )), ).not.toThrow() })
- Assert the specific error message in line 8-10:
it('should throw an error if no query client has been set', () => { expect(() => render(() => <SolidQueryDevtoolsPanel />)).toThrow( - 'No QueryClient set, use QueryClientProvider to set one', + /No QueryClient set/, ) })
- Add edge-case test for precedence when both context and prop are provided:
it('should prioritize prop client over context client', () => { const contextClient = new QueryClient() const propClient = new QueryClient() expect(() => render(() => ( <QueryClientProvider client={contextClient}> <SolidQueryDevtoolsPanel client={propClient} /> </QueryClientProvider> )), ).not.toThrow() })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
.changeset/breezy-cobras-sniff.md(1 hunks)packages/solid-query-devtools/src/__tests__/devtoolsPanel.test.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/breezy-cobras-sniff.md
🧰 Additional context used
🧬 Code graph analysis (1)
packages/solid-query-devtools/src/__tests__/devtoolsPanel.test.tsx (1)
packages/solid-query-devtools/src/devtoolsPanel.tsx (1)
SolidQueryDevtoolsPanel(44-86)
| View your CI Pipeline Execution ↗ for commit 1c060af
☁️ Nx Cloud last updated this comment at |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
@@ Coverage Diff @@ ## main #9763 +/- ## ========================================== + Coverage 45.56% 45.68% +0.12% ========================================== Files 196 200 +4 Lines 8327 8390 +63 Branches 1895 1912 +17 ========================================== + Hits 3794 3833 +39 - Misses 4091 4110 +19 - Partials 442 447 +5 🚀 New features to boost your workflow:
|
2 participants
🎯 Changes
This PR fixes #9762. The fix was simple, the queryClient passed as a prop should be forwarded to the useQueryClient hooks. I've also added simple rendering tests for SolidQueryClient and SolidQueryClientPanel
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests
Chores