Resolved FIXME #2147
Resolved FIXME #2147
Conversation
…-logic-and-add-validation-test
…sing-logic-in-zodmessagehandler
|
WalkthroughThe changes introduce new unit tests for the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)packages/core/src/v3/zodNamespace.ts (1)
🔇 Additional comments (4)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core/src/v3/zodMessageHandler.test.ts(1 hunks)packages/core/src/v3/zodMessageHandler.ts(1 hunks)packages/core/src/v3/zodNamespace.ts(3 hunks)packages/core/test/zodNamespace.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/core/src/v3/zodMessageHandler.test.ts (1)
packages/core/src/v3/zodMessageHandler.ts (1)
ZodMessageHandler(58-210)
packages/core/src/v3/zodNamespace.ts (1)
packages/core/src/v3/zodSocket.ts (2)
ZodSocketMessageCatalogSchema(8-17)GetSocketMessagesWithCallback(244-248)
🪛 Biome (1.9.4)
packages/core/src/v3/zodNamespace.ts
[error] 18-18: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🔇 Additional comments (9)
packages/core/src/v3/zodNamespace.ts (3)
8-8: LGTM: Import addition supports the new validation logic.The import of
GetSocketMessagesWithCallbackis correctly added to support the compile-time validation.
42-42: LGTM: Compile-time constraint properly applied.The type constraint is correctly applied to the interface, providing compile-time safety against callback schemas in server messages.
118-126: LGTM: Runtime validation logic is robust and well-implemented.The runtime validation correctly:
- Filters server message entries for those with callback properties
- Uses truthiness check to identify actual callback schemas (not just undefined properties)
- Provides a clear, descriptive error message listing the problematic message keys
This complements the compile-time validation and ensures runtime safety.
packages/core/test/zodNamespace.test.ts (2)
6-14: LGTM: Clean stub server implementation.The stub server provides the minimal interface needed for testing without unnecessary complexity. The chainable pattern for the namespace methods is well-implemented.
16-38: LGTM: Comprehensive test coverage for callback validation.The test correctly:
- Sets up a server message schema with a callback
- Verifies that the ZodNamespace constructor throws an error
- Uses appropriate regex matching for the error message
This test provides good coverage for the runtime validation added in
zodNamespace.ts.packages/core/src/v3/zodMessageHandler.test.ts (2)
6-21: LGTM: Comprehensive test for payload-wrapped messages.The test correctly verifies the handling of messages with explicit
payloadfields, ensuring:
- The handler receives the correct payload
- The acknowledgment is properly returned
- The message structure matches expected format
23-35: LGTM: Essential test for direct payload messages.This test covers the important scenario where messages don't have a
payloadwrapper, ensuring backward compatibility and proper handling of messages from sources other than ZodMessageSender.packages/core/src/v3/zodMessageHandler.ts (2)
182-186: LGTM: Enhanced payload detection using Zod validation.The improvement from a simple property check to Zod schema validation is excellent. This approach:
- Validates that the message is actually an object with a
payloadproperty- Uses
passthrough()to allow additional properties- Provides more reliable detection than
"payload" in messageThis addresses potential edge cases where a message might have a
payloadproperty but not be a valid message structure.
188-195: LGTM: Robust message handling for both scenarios.The conditional logic correctly handles both message types:
- Messages with payload: spreads the entire message with the event type
- Messages without payload: safely extracts version and constructs payload from remaining properties
The defensive programming with the
messageObjinitialization ensures safety even for non-object messages.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
PR Draft Summary
The recent work includes two key tasks:
Validate namespace server messages
zodNamespaceto reject server messages that define callbacks.zodNamespace.test.tsensuring unsupported callbacks produce errors.Improve message parsing logic
zodMessageHandlerto reliably detect payloads using runtime validation.zodMessageHandler.test.tscovering messages with and without explicitpayloadfields.These updates strengthen server message validation and enhance message parsing reliability.