test(query-core/queryObserver): add tests for 'notifyOnChangeProps' as a function controlling listener notification#10900
Conversation
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds two tests to queryObserver.test.tsx exercising notifyOnChangeProps when it's a function: one asserts the listener is called after refetch when data changes, the other asserts the listener is not called when data remains identical. ChangesnotifyOnChangeProps function test coverage
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 fc9e160
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ 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. |
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-core/src/__tests__/queryObserver.test.tsx`:
- Around line 750-764: The test is asserting on the listener's total call
history which can be polluted by earlier notifications; after creating the mock
listener (vi.fn()) and subscribing with observer.subscribe(listener), call
listener.mockClear() (or listener.mockReset()) before triggering
observer.refetch() so the assertion checks only notifications caused by refetch;
update both tests that use QueryObserver.subscribe(listener) +
observer.refetch() to clear the mock after subscribe and before the
refetch/assertion.
🪄 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: 5ce13e5d-fd83-4a93-8f2c-024d04417725
📒 Files selected for processing (1)
packages/query-core/src/__tests__/queryObserver.test.tsx
450047c to
5bdc383
Compare
…s a function controlling listener notification
5bdc383 to
03f6585
Compare
size-limit report 📦
|
…nge-props-function # Conflicts: # packages/query-core/src/__tests__/queryObserver.test.tsx
🎯 Changes
Adds two tests in
queryObserver.test.tsxcovering the function form ofnotifyOnChangeProps.queryObserver.test.tsxpreviously had nonotifyOnChangePropstests at all. These verify that whennotifyOnChangePropsis a function, its returned value controls whether listeners are notified:data) changesdata) does not changeThis covers the previously uncovered branch in
shouldNotifyListenerswherenotifyOnChangePropsis called as a function.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit