test(query-devtools/contexts/QueryDevtoolsContext): add test for 'Provider' value reactivity via getter properties#10793
Merged
sukvvon merged 4 commits intoMay 25, 2026
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Vitest test that mounts a consumer under QueryDevtoolsContext.Provider and asserts useQueryDevtoolsContext() observes reactive updates to the provider’s ChangesQueryDevtoolsContext Hook Test
🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs:
Suggested labels:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 57e6e74
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview2 package(s) bumped directly, 23 bumped as dependents. 🟩 Patch bumps
|
sukvvon
force-pushed
the
test/query-devtools-querydevtoolscontext-unit-tests
branch
2 times, most recently
from
80ec018 to
9922895
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/query-devtools/src/__tests__/contexts/QueryDevtoolsContext.test.tsx (3)
80-83: 💤 Low valueOptional: Strengthen assertions to verify the effect ran exactly twice.
While the current assertions correctly verify the observed client values, you could make the test more robust by confirming that
createEffectran exactly twice (once on mount, once after the signal update).💡 Optional assertion improvements
- expect(observed.at(-1)).toBe(initialClient) + expect(observed).toHaveLength(1) + expect(observed[0]).toBe(initialClient) setClient(nextClient) - expect(observed.at(-1)).toBe(nextClient) + expect(observed).toHaveLength(2) + expect(observed).toEqual([initialClient, nextClient])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-devtools/src/__tests__/contexts/QueryDevtoolsContext.test.tsx` around lines 80 - 83, The test should assert that the effect ran exactly twice by checking the observed array length in addition to the last values; update the assertions around observed, setClient, and the signal update so you assert observed.length (or use toHaveLength) equals 2 and that observed[0] is initialClient and observed[1] (or at(-1)) is nextClient to ensure createEffect executed once on mount and once after setClient.
53-54: ⚡ Quick winConsider disposing QueryClient instances after use.
Both
initialClientandnextClientshould be disposed after the test completes to prevent potential resource leaks.♻️ Suggested cleanup pattern
expect(observed.at(-1)).toBe(initialClient) setClient(nextClient) expect(observed.at(-1)).toBe(nextClient) + + initialClient.clear() + nextClient.clear() })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-devtools/src/__tests__/contexts/QueryDevtoolsContext.test.tsx` around lines 53 - 54, The test creates two QueryClient instances (initialClient and nextClient) but never disposes them; add cleanup to call their disposal/cleanup methods (e.g., clear/destroy/cancel pending queries) after the test completes—either in an afterEach block or a finally block inside the test—so that initialClient and nextClient are cleaned up and no resources/observers are leaked (locate the variables initialClient and nextClient in QueryDevtoolsContext.test.tsx and invoke the appropriate disposal method on each).
32-32: ⚡ Quick winConsider disposing QueryClient instances after use.
QueryClient instances may hold resources such as caches and timers. While test instances are garbage-collected, explicitly disposing them is a best practice to prevent potential resource leaks in the test suite.
♻️ Suggested cleanup pattern
it('should resolve to the value provided by the "Provider"', () => { - const value: QueryDevtoolsProps = { - client: new QueryClient(), + const client = new QueryClient() + const value: QueryDevtoolsProps = { + client, queryFlavor: 'TanStack Query', version: '5', onlineManager, } let resolved: QueryDevtoolsProps | undefined function ContextProbe() { resolved = useQueryDevtoolsContext() return null } render(() => ( <QueryDevtoolsContext.Provider value={value}> <ContextProbe /> </QueryDevtoolsContext.Provider> )) expect(resolved).toBe(value) + client.clear() })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-devtools/src/__tests__/contexts/QueryDevtoolsContext.test.tsx` at line 32, The test creates a QueryClient instance (new QueryClient()) but doesn't dispose it; update the test to store the instance in a variable and add a teardown (e.g., afterEach) that properly cleans it up—call QueryClient methods to cancel outstanding queries and clear caches (for example use client.cancelQueries(), client.getQueryCache().clear(), and client.getMutationCache().clear() or client.clear() if available) to ensure QueryClient resources are released in the QueryDevtoolsContext.test.tsx tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/query-devtools/src/__tests__/contexts/QueryDevtoolsContext.test.tsx`:
- Around line 80-83: The test should assert that the effect ran exactly twice by
checking the observed array length in addition to the last values; update the
assertions around observed, setClient, and the signal update so you assert
observed.length (or use toHaveLength) equals 2 and that observed[0] is
initialClient and observed[1] (or at(-1)) is nextClient to ensure createEffect
executed once on mount and once after setClient.
- Around line 53-54: The test creates two QueryClient instances (initialClient
and nextClient) but never disposes them; add cleanup to call their
disposal/cleanup methods (e.g., clear/destroy/cancel pending queries) after the
test completes—either in an afterEach block or a finally block inside the
test—so that initialClient and nextClient are cleaned up and no
resources/observers are leaked (locate the variables initialClient and
nextClient in QueryDevtoolsContext.test.tsx and invoke the appropriate disposal
method on each).
- Line 32: The test creates a QueryClient instance (new QueryClient()) but
doesn't dispose it; update the test to store the instance in a variable and add
a teardown (e.g., afterEach) that properly cleans it up—call QueryClient methods
to cancel outstanding queries and clear caches (for example use
client.cancelQueries(), client.getQueryCache().clear(), and
client.getMutationCache().clear() or client.clear() if available) to ensure
QueryClient resources are released in the QueryDevtoolsContext.test.tsx tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 813ea494-6c04-4bc6-bc8f-d4114c367483
📒 Files selected for processing (1)
packages/query-devtools/src/__tests__/contexts/QueryDevtoolsContext.test.tsx
…vider' value reactivity via getter properties
sukvvon
force-pushed
the
test/query-devtools-querydevtoolscontext-unit-tests
branch
from
b698ddd to
79e0b84
Compare
sukvvon
changed the title
Contributor
size-limit report 📦
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/query-devtools/src/__tests__/contexts/QueryDevtoolsContext.test.tsx`:
- Around line 9-10: Add cleanup to avoid leaking QueryClient instances: in
QueryDevtoolsContext.test.tsx add an afterEach that calls initialClient.clear()
and nextClient.clear() (or the appropriate QueryClient disposal method if using
a different version) to dispose both QueryClient instances created as
initialClient and nextClient so they don't accumulate across tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1bf27ca6-ab2a-4768-a2fd-f1c8cfe6b9ca
📒 Files selected for processing (1)
packages/query-devtools/src/__tests__/contexts/QueryDevtoolsContext.test.tsx
…t' instances after the reactive getter test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Add a dedicated
src/__tests__/contexts/QueryDevtoolsContext.test.tsxmirroring thesrc/contextsdirectory layout (matching theThemeContext.test.tsxsplit from #10787).The file locks down the one behavior that the module adds on top of
solid-js's context primitives: aProvidervalue whose fields are reactive getters must propagate signal updates to consumers — the pattern thatTanstackQueryDevtoolsuses (get client() { return client() }) to flowsetClient/setTheme/setErrorTypesetc. into the renderedDevtools.A bare value pass-through case and a default-value case were considered but dropped because they would only exercise
solid-jsitself, not anything the module contributes.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit