fix: sync query state in broadcast added events#10835
Conversation
📝 WalkthroughWalkthroughThis PR enhances cross-tab query synchronization in the experimental broadcast client. The implementation now includes the ChangesCross-tab query state synchronization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/query-broadcast-client-experimental/src/__tests__/index.test.ts (1)
32-76: ⚡ Quick winConsider adding test coverage for backward compatibility.
The test verifies that state propagates when present in the "added" message (the happy path). However, the defensive guard added in
index.ts(lines 96-98) suggests concern for backward compatibility with older messages that lack state.Consider adding a test case that verifies the behavior when an "added" message arrives without state:
- For existing queries: should not call
setState(undefined)- For new queries: should build with default/empty state
🤖 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-broadcast-client-experimental/src/__tests__/index.test.ts` around lines 32 - 76, Add a new test in index.test.ts that simulates receiving an "added" broadcast message without a state payload and asserts backward-compatible behavior: for an existing query (create a seeded query via receiverClient.getQueryCache().build) ensure the receiver's state is not replaced with undefined (use receiverClient.getQueryCache().find(...).state to assert it stays intact), and for a new query ensure the receiver builds a default/empty state (assert find(...)?.state is defined and matches default shape). Use the same setup helpers (broadcastQueryClient, senderClient, receiverClient, broadcastChannel) and send an "added" message without a state field through the broadcastChannel to trigger the code paths guarded in index.ts.
🤖 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-broadcast-client-experimental/src/__tests__/index.test.ts`:
- Around line 32-76: Add a new test in index.test.ts that simulates receiving an
"added" broadcast message without a state payload and asserts
backward-compatible behavior: for an existing query (create a seeded query via
receiverClient.getQueryCache().build) ensure the receiver's state is not
replaced with undefined (use receiverClient.getQueryCache().find(...).state to
assert it stays intact), and for a new query ensure the receiver builds a
default/empty state (assert find(...)?.state is defined and matches default
shape). Use the same setup helpers (broadcastQueryClient, senderClient,
receiverClient, broadcastChannel) and send an "added" message without a state
field through the broadcastChannel to trigger the code paths guarded in
index.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1efdc65a-63a0-4a67-bdf8-79f771188a8a
📒 Files selected for processing (2)
packages/query-broadcast-client-experimental/src/__tests__/index.test.tspackages/query-broadcast-client-experimental/src/index.ts
Summary
addedevents so new clients can hydrate initial query state immediately.setStateupdates foraddedmessages without state so older payloads remain compatible.Summary by CodeRabbit
Bug Fixes
Tests