feature: add subscription.find #2735
Conversation
|
🚀 Thanks for opening this pull request! |
|
Warning Rate limit exceeded@dblythy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds QUERY op and RESULT event handling to LiveQueryClient, passes client reference into LiveQuerySubscription, implements subscription.find() to send a QUERY via WebSocket, maps RESULT payloads to ParseObject instances (setting className when missing) and emits them on the subscription, and updates types and tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App as Application
participant Sub as LiveQuerySubscription
participant LQC as LiveQueryClient
participant WS as WebSocket
participant PS as ParseServer
App->>Sub: subscription.find()
alt client attached
Sub->>LQC: await connectPromise
LQC->>WS: send OP_TYPES.QUERY { requestId: sub.id, query, sessionToken }
WS->>PS: forward QUERY
PS-->>WS: RESULT { requestId, results:[...] }
WS-->>LQC: message(OP_EVENTS.RESULT, { requestId, results })
LQC->>LQC: map results -> ParseObject (set className if missing)
LQC->>Sub: emit('result', [ParseObject,...])
Sub-->>App: 'result' event with mapped results
else no client
Sub-->>App: find() is no-op
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Potential focus areas:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #2735 +/- ##
===========================================
- Coverage 100.00% 99.88% -0.12%
===========================================
Files 63 64 +1
Lines 6185 6233 +48
Branches 1456 1492 +36
===========================================
+ Hits 6185 6226 +41
- Misses 0 7 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
types/LiveQuerySubscription.d.ts (2)
92-92: Consider stronger typing for theclientfield.The
clientfield is currently typed asany, which loses type safety. While a directLiveQueryClienttype would create a circular dependency, consider defining a minimal interface or usingimport typeto preserve type information without runtime circularity.Example approach:
+import type LiveQueryClient from './LiveQueryClient'; + declare class LiveQuerySubscription { id: string | number; query: ParseQuery; sessionToken?: string; subscribePromise: any; unsubscribePromise: any; subscribed: boolean; - client: any; + client?: LiveQueryClient; emitter: EventEmitter; on: EventEmitter['on']; emit: EventEmitter['emit']; - constructor(id: string | number, query: ParseQuery, sessionToken?: string, client?: any); + constructor(id: string | number, query: ParseQuery, sessionToken?: string, client?: LiveQueryClient);Also applies to: 96-96
103-107: Enhance JSDoc to document preconditions.The documentation for
find()should clarify what happens whenclientis not set, and whether callers need to wait for connection before calling./** * Execute a query on this subscription. * The results will be delivered via the 'result' event. + * Note: This method requires a client to be set and will only send the query + * after the client's connection promise resolves. */ find(): void;src/__tests__/LiveQueryClient-test.js (1)
1155-1184: LGTM! Consider adding edge case test.The test correctly verifies that
subscription.find()sends a properly formatted query message with the correctopandrequestId. The test appropriately waits for the connection promise before callingfind().Consider adding a test for the edge case where
find()is called whenclientis not set or before connection is established:it('find() does nothing when client is not set', () => { const subscription = new LiveQuerySubscription(1, new ParseQuery('Test')); expect(() => subscription.find()).not.toThrow(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/LiveQueryClient.ts(5 hunks)src/LiveQuerySubscription.ts(2 hunks)src/__tests__/LiveQueryClient-test.js(3 hunks)types/LiveQuerySubscription.d.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/__tests__/LiveQueryClient-test.js (2)
src/__tests__/ParseQuery-test.js (2)
LiveQuerySubscription(35-35)ParseQuery(34-34)src/__tests__/ParseLiveQuery-test.js (2)
LiveQuerySubscription(16-16)ParseQuery(15-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build (Node 20, 20.15.1)
- GitHub Check: build (Node 18, 18.20.4)
🔇 Additional comments (8)
src/LiveQueryClient.ts (4)
24-24: LGTM!The addition of the
QUERYoperation type follows the existing pattern and is correctly placed.
38-38: LGTM!The
RESULTevent type addition is correctly placed and follows the existing pattern.
58-58: LGTM!The
RESULTemitter type addition is correctly placed and follows the existing pattern.
218-218: LGTM!Passing the client instance to the subscription constructor enables
subscription.find()to send messages over the WebSocket. This is essential for the new query execution feature.src/__tests__/LiveQueryClient-test.js (3)
1-1: LGTM!Unmocking
LiveQuerySubscriptionand importing it directly is necessary to test the newfind()method and verify prototype behavior. This follows the same pattern used in other test files.Also applies to: 41-41
1096-1132: LGTM!The test comprehensively verifies that the RESULT event handler correctly maps multiple JSON objects to ParseObject instances and emits them via the subscription. Test structure follows existing patterns.
1134-1136: LGTM!Both tests appropriately verify that
find()exists on theLiveQuerySubscriptionprototype and on subscription instances. The instance test also confirms the correct type.Also applies to: 1138-1153
src/LiveQuerySubscription.ts (1)
95-95: LGTM!The client field and constructor parameter additions are correctly implemented. The client reference is properly stored for use by the
find()method.Also applies to: 103-109
2 participants
Pull Request
Issue
parse-community/parse-server#9864
Closes: #2114
Approach
Tasks
Summary by CodeRabbit
New Features
Tests
Documentation