test(query-devtools/utils): add tests for 'getPreferredColorScheme'#10654
test(query-devtools/utils): add tests for 'getPreferredColorScheme'#10654
Conversation
📝 WalkthroughWalkthroughNew test suite validates that the ChangesgetPreferredColorScheme Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
|
View your CI Pipeline Execution ↗ for commit acdaf2c
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/query-devtools/src/__tests__/utils.test.ts (1)
1187-1219: 💤 Low valueLGTM — shared-listener design is intentional and correct.
Each
window.matchMedia()call returns a newMediaQueryListobject, but sinceaddEventListenerandremoveEventListeneron every returned object close over the samelistenersSet, both registration and cleanup work correctly even if the implementation stores theMediaQueryListreference and calls both methods on the same object.One minor precision gap worth noting: the
addEventListenermock ignores its first argument (_event: string), solistenerscounts callbacks registered for any event type, not just'change'. ThelistenerCount() === 1assertions would still pass if the implementation incorrectly registered two listeners for two different event types whose counts happened to cancel out after disposal. In practice this is a non-issue given the implementation only registers for'change', but filtering on_event === 'change'would makelistenerCountsemantically precise without extra complexity.🔧 Optional: filter by event type in the mock
addEventListener: vi.fn( - (_event: string, listener: MatchMediaListener) => { - listeners.add(listener) + (event: string, listener: MatchMediaListener) => { + if (event === 'change') listeners.add(listener) }, ) as MediaQueryList['addEventListener'], removeEventListener: vi.fn( - (_event: string, listener: MatchMediaListener) => { - listeners.delete(listener) + (event: string, listener: MatchMediaListener) => { + if (event === 'change') listeners.delete(listener) }, ) as MediaQueryList['removeEventListener'],🤖 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__/utils.test.ts` around lines 1187 - 1219, The mock in setupMatchMediaMock currently ignores the event name so listeners Set counts callbacks for any event; change the addEventListener/removeEventListener handlers in setupMatchMediaMock to only add/remove the listener when the first argument equals 'change' (i.e., check the _event parameter) so the Set only tracks 'change' listeners, and ensure listenerCount() continues to return listeners.size (now representing only 'change' listeners).
🤖 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__/utils.test.ts`:
- Around line 1187-1219: The mock in setupMatchMediaMock currently ignores the
event name so listeners Set counts callbacks for any event; change the
addEventListener/removeEventListener handlers in setupMatchMediaMock to only
add/remove the listener when the first argument equals 'change' (i.e., check the
_event parameter) so the Set only tracks 'change' listeners, and ensure
listenerCount() continues to return listeners.size (now representing only
'change' listeners).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e3e6b97d-af28-4bdd-8f39-c96dc897b750
📒 Files selected for processing (1)
packages/query-devtools/src/__tests__/utils.test.ts
1 participant
🎯 Changes
Add unit tests for the
getPreferredColorSchemehelper inquery-devtools/utils.tsx.The helper returns a Solid signal that defaults to
'dark', then on mount readswindow.matchMedia('(prefers-color-scheme: dark)')and reflects its value, listening tochangeevents for live updates.Cases:
'dark'beforeonMountruns (default signal value)onMount, callsmatchMediawith'(prefers-color-scheme: dark)', registers a listener, and reflectsmatches: trueas'dark'matches: falseas'light'changeevent fireschangelistener on cleanupwindow.matchMediais stubbed viavi.stubGlobal(auto-cleaned byvi.unstubAllGlobalsinafterEach). Solid'sonMountruns as a microtask, so each test uses fake timers andawait vi.advanceTimersByTimeAsync(0)to flush — matching the pattern insolid-query-persist-client.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit