feat(client, server): custom RPC JSON Serializer#241
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request updates the project's documentation and serialization logic. The sidebar configuration in the VitePress site was updated to remove an outdated item, add a new "RPC JSON Serializer" link, and correct text casing for an existing item. A new documentation file has been added to explain how to extend and override the RPC JSON serializer. Several documentation files had their metadata keys standardized from uppercase to lowercase. Additionally, the RPC JSON serializer source was refactored to use new type constants and a custom serializer interface, and comprehensive tests were introduced for serialization logic and error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Handler as RPCHandler
participant Serializer as StandardRPCJsonSerializer
participant Custom as StandardRPCCustomJsonSerializer
participant Client as RPCClient
Handler->>Serializer: serialize(value)
alt Custom serializer available
Serializer->>Custom: use custom serialize
Custom-->>Serializer: serialized value & meta
else Default handling
Serializer->>Serializer: process value using type constants
end
Serializer-->>Handler: return serialized JSON & meta
Handler->>Serializer: deserialize(json, meta)
alt Metadata indicates custom type
Serializer->>Custom: use custom deserialize
Custom-->>Serializer: deserialized value
else Default handling
Serializer->>Serializer: process deserialization using type constants
end
Serializer-->>Handler: return deserialized value
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
ERR_PNPM_OPTIONAL_DEPS_REQUIRE_PROD_DEPS Optional dependencies cannot be installed without production dependencies ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
More templates
@orpc/client
@orpc/contract
@orpc/openapi-client
@orpc/openapi
@orpc/react-query
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/svelte-query
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/client/src/adapters/standard/rpc-json-serializer.test.ts (2)
4-16: Consider adding negative or edge-case tests forPersonobjects.
ThePersonclass and itstoJSONmethod look fine. However, you might want to test edge scenarios:
nameas an empty string or undefineddateas an invalid date
This ensures robust coverage for real-world usage.
18-30: ValidatePerson2in-depth with more complex data structures.
Person2supports a potentially unboundedanytype fordata. Consider including nested arrays/objects deeper than the single-level example, plus invalid or unusual data.apps/content/docs/advanced/rpc-json-serializer.md (1)
45-47: Highlight potential version mismatches or conflicts.
In the warning note, you mention ensuringtypeis unique and greater than 20. Also consider clarifying that conflicts can arise if future releases introduce new built-in types occupying a given numeric range.packages/client/src/adapters/standard/rpc-json-serializer.ts (2)
3-11: Use anenumor a single object for type constants.
DefiningTYPE_*as separate constants is fine, but grouping them into anenumor a single object can improve maintainability and clarity—especially if you add more types in future.-const TYPE_BIGINT = 0 -const TYPE_DATE = 1 -... +enum SerializerType { + BIGINT = 0, + DATE = 1, + // ... +}
37-46: Factor out repeated serialization steps for clarity.
Manymeta.push()calls follow a similar pattern. Extracting a helper function (e.g.,pushMeta(type, segments, meta)) might DRY up the code and make it easier to maintain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/content/.vitepress/config.ts(1 hunks)apps/content/docs/advanced/rpc-json-serializer.md(1 hunks)apps/content/docs/best-practices/dedupe-middleware.md(1 hunks)apps/content/docs/lifecycle.md(1 hunks)apps/content/docs/metadata.md(1 hunks)packages/client/src/adapters/standard/rpc-json-serializer.test.ts(1 hunks)packages/client/src/adapters/standard/rpc-json-serializer.ts(4 hunks)
🔇 Additional comments (13)
apps/content/docs/best-practices/dedupe-middleware.md (3)
2-3: Standardize Metadata KeysThe metadata keys have been updated from uppercase to lowercase (“title” and “description”), which aligns with the standardization applied across the documentation. This improves consistency and readability.
24-37: Clear and Consistent Code ExampleThe TypeScript example demonstrating the dedupe middleware mechanism is clear and well-documented. The use of
os.$contextalong with the middleware pattern effectively illustrates how to leverage context to prevent redundant middleware execution.
82-104: Detailed Deduplication ExamplesThe code examples provided under “Built-in Dedupe Middleware” comprehensively demonstrate scenarios where middleware deduplication occurs (and where it does not). The inline comments (e.g., “⛔ Deduplication does not occur”) add clarity.
apps/content/docs/metadata.md (2)
2-3: Standardize Metadata KeysThe metadata section has been updated to use lowercase keys (“title” and “description”) which is consistent with similar documentation files. This improves the overall uniformity of our documentation metadata.
20-45: Good Example of Metadata Usage in CodeThe TypeScript example provided clearly demonstrates how to initialize and extend metadata using
. $metaand.meta(). The comments (including the code highlight markers) help clarify the concept for the reader.apps/content/docs/lifecycle.md (2)
2-3: Standardize Metadata KeysThe changes on lines 2-3 update the metadata keys to lowercase (“title” and “description”), aligning this document with the standard format used throughout the documentation suite.
10-42: Well-Structured Lifecycle DocumentationThe document provides a comprehensive overview of the oRPC lifecycle. The included diagram reference, the middleware order table, and the configuration example effectively communicate the intended lifecycle process. The content is both informative and consistent with our documentation standards.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: A comma might be missing here.
Context: ...fter input validation and before output validation apply the following configuration: ```...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
packages/client/src/adapters/standard/rpc-json-serializer.test.ts (2)
122-143: Excellent test for detecting duplicate serializer types.
The error handling test correctly ensures that each custom serializer has a uniquetype. This promotes maintainability and prevents hidden conflicts.
32-43:Details
❓ Verification inconclusive
Ensure custom type uniqueness across the broader codebase.
customSupportedDataTypesdefine custom types with type IDs (like 20 or 21). This is consistent with your main code, but confirm no collisions occur beyond this test file.Could you scan the repo to confirm that type
20or21is not used by other custom serializers?
🏁 Script executed:
#!/bin/bash rg -A 5 $'type: 20\\b' rg -A 5 $'type: 21\\b'Length of output: 2937
Action: Confirm Intentional Use of Type ID 21
- Our scan confirms that type ID 20 appears only in the test file.
- However, type ID 21 is used both in the test file (for
Person2) and in the documentation file atapps/content/docs/advanced/rpc-json-serializer.md(forUser). Please verify that reusing type ID 21 in these contexts is intentional and won’t lead to runtime collisions.- If this dual use is by design, consider adding a clarifying comment in the docs or tests. Otherwise, update one of the type IDs to ensure they are unique across the codebase.
apps/content/.vitepress/config.ts (1)
148-149: Sidebar addition is clear and well-placed.
Adding "RPC JSON Serializer" to the "Advanced" section logically highlights where the concept belongs. Including "Exceeds the maximum length ..." with corrected casing ensures consistent naming conventions.apps/content/docs/advanced/rpc-json-serializer.md (1)
1-16: Nicely structured introduction and usage summary.
The step-by-step explanation of the RPC JSON serializer extension is clear. Including a quick reference to the built-in supported types is helpful for new users.packages/client/src/adapters/standard/rpc-json-serializer.ts (2)
27-35: Sensible check for duplicate custom serializer types.
Throwing an error on duplicate custom types ensures consistent serialization logic without conflicts. This aligns well with the test coverage inrpc-json-serializer.test.ts.
122-201: Clean approach to merging blob references and applying rehydration indeserialize.
The layered approach of reconstructing objects from metadata and optionally injectingBlobinstances is well-structured. The final switch-case for known/built-in types plus custom deserialization ensures coverage for all scenarios. Great job.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/client/src/adapters/standard/rpc-json-serializer.ts (3)
34-36: Improve error message for duplicate serializer typesWhile the validation for unique serializer types is important, the error message could be more descriptive to help developers debug issues more quickly.
- throw new Error('Custom serializer type must be unique.') + throw new Error('Custom serializer types must be unique. Please ensure each serializer has a distinct type value.')
153-159: Add error handling for custom deserializersThe custom deserialization logic doesn't include any error handling. If a custom deserializer throws an exception, it could potentially break the entire deserialization process. Consider adding try-catch blocks to handle errors gracefully.
for (const custom of this.customSerializers) { if (custom.type === type) { - currentRef[preSegment] = custom.deserialize(currentRef[preSegment]) + try { + currentRef[preSegment] = custom.deserialize(currentRef[preSegment]) + } catch (error) { + console.error(`Error in custom deserializer (type ${custom.type}):`, error); + // Could either throw a more descriptive error or continue with original value + // depending on desired behavior + } break } }
20-21: Consider improving type safetyThe methods in the
StandardRPCJsonCustomSerializerinterface useanyfor the data parameters. While this provides flexibility, it might be beneficial to use more specific types or generics to improve type safety.interface StandardRPCJsonCustomSerializer { type: number condition(data: unknown): boolean - serialize(data: any): unknown - deserialize(serialized: any): unknown + serialize<T>(data: T): unknown + deserialize<T>(serialized: unknown): T }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/content/docs/advanced/rpc-json-serializer.md(1 hunks)packages/client/src/adapters/standard/rpc-json-serializer.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/content/docs/advanced/rpc-json-serializer.md
🔇 Additional comments (4)
packages/client/src/adapters/standard/rpc-json-serializer.ts (4)
3-12: Good use of constants for type identificationUsing a constant object to replace magic numbers is a great improvement for code readability and maintainability. This makes the code more self-documenting and reduces the risk of errors when referring to specific serializer types.
17-22: Well-designed custom serializer interfaceThe
StandardRPCJsonCustomSerializerinterface provides a clean and flexible way to implement custom serialization logic. The separation of concerns between condition checking, serialization, and deserialization is well thought out.
39-122: Well-structured serialization logicThe serialization method is well-structured with custom serializers checked first before falling back to built-in types. The code maintains a consistent pattern throughout and correctly uses the type constants.
124-201: Robust deserialization implementationThe deserialize method handles both custom and built-in types appropriately. The method signatures provide flexibility for different use cases, which is a good design choice.
| for (const custom of this.customSerializers) { | ||
| if (custom.condition(data)) { | ||
| const result = this.serialize(custom.serialize(data), segments, meta, maps, blobs) | ||
|
|
||
| meta.push([custom.type, segments]) | ||
|
|
||
| return result | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding type validation for custom serializers
The code correctly prioritizes custom serializers, but there's no validation to ensure that custom serializer types don't conflict with built-in types (0-7). This could lead to unexpected behavior if a custom serializer uses the same type as a built-in one.
constructor(options: StandardRPCJsonSerializerOptions = {}) {
this.customSerializers = options.customJsonSerializers ?? []
if (this.customSerializers.length !== new Set(this.customSerializers.map(custom => custom.type)).size) {
throw new Error('Custom serializer type must be unique.')
}
+
+ // Ensure custom serializers don't conflict with built-in types
+ const builtInTypeValues = Object.values(STANDARD_RPC_JSON_SERIALIZER_BUILT_IN_TYPES);
+ for (const serializer of this.customSerializers) {
+ if (builtInTypeValues.includes(serializer.type)) {
+ throw new Error(`Custom serializer type ${serializer.type} conflicts with a built-in type. Please use a different value.`);
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const custom of this.customSerializers) { | |
| if (custom.condition(data)) { | |
| const result = this.serialize(custom.serialize(data), segments, meta, maps, blobs) | |
| meta.push([custom.type, segments]) | |
| return result | |
| } | |
| } | |
| constructor(options: StandardRPCJsonSerializerOptions = {}) { | |
| this.customSerializers = options.customJsonSerializers ?? [] | |
| if (this.customSerializers.length !== new Set(this.customSerializers.map(custom => custom.type)).size) { | |
| throw new Error('Custom serializer type must be unique.') | |
| } | |
| // Ensure custom serializers don't conflict with built-in types | |
| const builtInTypeValues = Object.values(STANDARD_RPC_JSON_SERIALIZER_BUILT_IN_TYPES); | |
| for (const serializer of this.customSerializers) { | |
| if (builtInTypeValues.includes(serializer.type)) { | |
| throw new Error(`Custom serializer type ${serializer.type} conflicts with a built-in type. Please use a different value.`); | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/client/src/adapters/standard/rpc-json-serializer.ts (1)
31-37: 🛠️ Refactor suggestionConsider adding validation for custom serializer type conflicts.
The code currently checks that custom serializer types are unique among themselves, but it doesn't validate that they don't conflict with built-in types (0-7). This could lead to unexpected behavior if a custom serializer uses the same type as a built-in one.
constructor(options: StandardRPCJsonSerializerOptions = {}) { this.customSerializers = options.customJsonSerializers ?? [] if (this.customSerializers.length !== new Set(this.customSerializers.map(custom => custom.type)).size) { throw new Error('Custom serializer type must be unique.') } + + // Ensure custom serializers don't conflict with built-in types + const builtInTypeValues = Object.values(STANDARD_RPC_JSON_SERIALIZER_BUILT_IN_TYPES); + for (const serializer of this.customSerializers) { + if (builtInTypeValues.includes(serializer.type)) { + throw new Error(`Custom serializer type ${serializer.type} conflicts with a built-in type. Please use a different value.`); + } + } }
🧹 Nitpick comments (1)
apps/content/docs/advanced/rpc-json-serializer.md (1)
45-47: Consider adding a recommendation for type value ranges.While the warning correctly states that the type should be greater than 20, it would be helpful to also provide a recommended range or upper limit to further reduce the risk of future conflicts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/content/docs/advanced/rpc-json-serializer.md(1 hunks)packages/client/src/adapters/standard/rpc-json-serializer.ts(4 hunks)
🔇 Additional comments (8)
apps/content/docs/advanced/rpc-json-serializer.md (2)
1-84: Well-structured documentation with comprehensive examples.This new documentation file provides clear instructions on how to extend or override the standard RPC JSON serializer. The examples are comprehensive and cover both custom serializer implementation and integration with RPCHandler and RPCLink.
77-82:Details
✅ Verification successful
Verify that the built-in undefined serializer works as described.
Let's confirm that the serialization behavior described here works as expected, particularly the statement about how oRPC represents undefined only in array items.
🏁 Script executed:
#!/bin/bash # Find tests that verify the undefined serializer behavior rg -A 5 "undefined.*serializer|serializer.*undefined" --type tsLength of output: 1187
Resolved: Test cases confirm the undefined serializer works as expected.
The tests in
packages/openapi/src/adapters/standard/openapi-codec.test.tsconsistently verify that:
- The serializer's
serializemethod is invoked withundefined(transforming it tonullfor JSON representation).- The serializer's
deserializemethod is invoked withundefined, ensuring the round-trip behavior holds.- In scenarios involving array items, the behavior remains as documented.
No further changes are necessary based on the current tests.
packages/client/src/adapters/standard/rpc-json-serializer.ts (6)
3-12: Good use of constants for type values.Using a constants object instead of numeric literals improves code readability and maintainability. The use of
as constensures type safety.
14-22: Well-designed interface for custom serializers.The
StandardRPCCustomJsonSerializerinterface is well-defined with clear methods and types, providing all the necessary functionality for custom serialization.
39-48: Well-implemented custom serializer handling.The
serializemethod now correctly prioritizes custom serializers by checking them first before proceeding with built-in type handling. This allows for flexible extension of the serialization logic.
153-159: Good implementation of custom deserializer handling.The deserialize method now properly processes custom serializers before moving on to built-in types. The use of a break statement ensures optimal performance by stopping iteration once a matching serializer is found.
161-197: Clean refactoring to use type constants.The switch statement now uses the named constants instead of magic numbers, which significantly improves code readability and maintainability.
124-127:Details
❓ Verification inconclusive
Consider adding a test for error handling in custom deserializers.
While the implementation looks solid, it would be beneficial to ensure there are tests for scenarios where custom deserializers throw errors or return unexpected values.
🏁 Script executed:
#!/bin/bash # Look for tests that verify error handling in custom deserializers rg -A 5 "test.*custom.*deserializer|custom.*deserializer.*test|error.*custom.*deserializer" --type tsLength of output: 103
Custom Deserializer Error Handling Test Needed
The current implementation in
packages/client/src/adapters/standard/rpc-json-serializer.ts(lines 124–127) is solid. However, our search for tests verifying error handling in custom deserializers didn’t return any evidence of such coverage. Please manually verify if there are tests (perhaps under alternate naming or file locations) that simulate scenarios where custom deserializers throw errors or yield unexpected values. If none exist, it would be beneficial to add tests to ensure robust error handling.
1 participant
The way to extend or override built-in types support by RPC Protocol
Summary by CodeRabbit
New Features
Documentation