feat(openapi): custom openapi json serializer #246
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces unit tests for the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Serializer as StandardOpenAPIJsonSerializer
participant Custom as StandardOpenAPICustomJsonSerializer
Client->>Serializer: serialize(data)
Note right of Serializer: Iterate over customSerializers
Serializer->>Custom: condition(data) check
alt condition met
Custom-->>Serializer: serialized data
else condition not met
Serializer-->>Serializer: default serialization
end
Serializer-->>Client: return serialized output
Possibly related PRs
Poem
🪧 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/react-query
@orpc/openapi
@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/openapi-client/src/adapters/standard/openapi-json-serializer.test.ts (1)
15-109: Thorough test coverage for built-in data typesExcellent test coverage for various data types including primitives, special values (NaN), dates, BigInt, RegExp, URL, objects, arrays, maps, sets, and binary data (Blob, File).
However, there's a commented section that could be removed or clarified.
- // { - // data: undefined, - // expected: expect.toSatisfy(v => v === null || v === undefined), CANNOT ASSERT UNDEFINED IN OBJECT? - // },The commented test case is addressed in the dedicated "serialize undefined" tests at the end of the file, so this commented code is no longer needed.
packages/openapi-client/src/adapters/standard/openapi-json-serializer.ts (4)
5-8: Good extensibility with custom serializer interface.The
StandardOpenAPICustomJsonSerializerinterface provides a clean approach for extensible serialization logic. The separation ofconditionandserializemethods creates a clear pattern for determining which serializer should handle specific data types.Consider using a generic type parameter instead of
anyin the serialize method signature to improve type safety:export interface StandardOpenAPICustomJsonSerializer { condition(data: unknown): boolean - serialize(data: any): unknown + serialize<T>(data: T): unknown }
22-28: Add protection against infinite recursion.The recursive call to
this.serializewith custom serializer output could potentially cause infinite recursion if a custom serializer'sconditionmethod matches its own output. Consider adding a simple depth counter or processed objects tracking mechanism.Here's a potential implementation:
serialize(data: unknown, hasBlobRef: { value: boolean } = { value: false }, depth: number = 0): StandardOpenAPIJsonSerialized { + // Prevent infinite recursion + if (depth > 100) { + throw new Error('Maximum serialization depth exceeded. Possible circular structure or infinite recursion in custom serializers.'); + } for (const custom of this.customSerializers) { if (custom.condition(data)) { - const result = this.serialize(custom.serialize(data), hasBlobRef) + const result = this.serialize(custom.serialize(data), hasBlobRef, depth + 1) return result } } // Rest of the method remains unchanged }
22-28: Consider adding error handling for custom serializers.If a custom serializer throws an exception, it would be helpful to include context about which serializer failed. This would make debugging easier for users who implement custom serializers.
for (const custom of this.customSerializers) { if (custom.condition(data)) { + try { const result = this.serialize(custom.serialize(data), hasBlobRef) return result + } catch (error) { + throw new Error(`Error in custom serializer: ${error instanceof Error ? error.message : String(error)}`); + } } }
22-28: Document serializer precedence behavior.The current implementation uses the first matching custom serializer based on array order. Consider adding a comment or documentation that explains this precedence behavior, so users understand how to order their custom serializers.
serialize(data: unknown, hasBlobRef: { value: boolean } = { value: false }): StandardOpenAPIJsonSerialized { + // Custom serializers are checked in order. The first one whose condition + // returns true will be used to process the data. for (const custom of this.customSerializers) { if (custom.condition(data)) { const result = this.serialize(custom.serialize(data), hasBlobRef) return result } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/openapi-client/src/adapters/standard/openapi-json-serializer.test.ts(1 hunks)packages/openapi-client/src/adapters/standard/openapi-json-serializer.ts(1 hunks)packages/openapi/src/adapters/fetch/openapi-handler.ts(1 hunks)packages/openapi/src/adapters/node/openapi-handler.ts(1 hunks)packages/openapi/src/adapters/standard/openapi-handler.ts(1 hunks)
🔇 Additional comments (10)
packages/openapi/src/adapters/fetch/openapi-handler.ts (1)
16-16: Pass options to the serializer constructorThe change correctly passes the options parameter to the StandardOpenAPIJsonSerializer constructor, allowing customization of the JSON serialization process.
packages/openapi/src/adapters/node/openapi-handler.ts (1)
16-16: Pass options to the serializer constructorThe change correctly passes the options parameter to the StandardOpenAPIJsonSerializer constructor, ensuring consistent behavior between the fetch and node adapters.
packages/openapi/src/adapters/standard/openapi-handler.ts (2)
1-1: Add import for serializer optionsThe import for StandardOpenAPIJsonSerializerOptions is correctly added to support the interface extension.
5-5: Extend handler options with serializer optionsThe StandardOpenAPIHandlerOptions interface now properly extends StandardOpenAPIJsonSerializerOptions, allowing serialization options to be passed through the handler.
packages/openapi-client/src/adapters/standard/openapi-json-serializer.test.ts (4)
1-14: Well-structured test setup for JSON serializerThe initial test setup with TestCase type and Test enum provides a clear structure for the comprehensive test suite.
111-148: Good test setup for custom data typesThe Person and Person2 classes with toJSON methods create a proper test fixture for verifying custom serialization support.
150-231: Comprehensive test scenarios with custom serializersThe test suite properly configures custom serializers and tests them against various data structures (flat, object, array, set, map, complex). The serializers are correctly registered using the new customJsonSerializers option.
233-249: Proper handling of undefined valuesGood edge case testing for undefined values, confirming they're preserved in objects but converted to null in arrays, matching standard JSON.stringify behavior.
packages/openapi-client/src/adapters/standard/openapi-json-serializer.ts (2)
10-12: Well-designed options interface with immutability.The
StandardOpenAPIJsonSerializerOptionsinterface uses good practices with thereadonlymodifier for the array of custom serializers, promoting immutability. The interface is also extensible for future options.
15-19: Good implementation of constructor and private properties.The private readonly property and the constructor using the nullish coalescing operator to provide defaults demonstrate clean code practices. The implementation properly initializes the custom serializers from the provided options.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/content/docs/rpc-handler.md (1)
60-60:⚠️ Potential issueTypo in Code Sample: Use
startsWithThe sample code in the Setup and Integration section shows:
if (url.pathname.startWith('/rpc')) {This appears to be a typo. It should be
startsWith. A small correction is needed to ensure the documentation example is accurate.Apply this diff:
- if (url.pathname.startWith('/rpc')) { + if (url.pathname.startsWith('/rpc')) {
🧹 Nitpick comments (1)
apps/content/docs/openapi/openapi-handler.md (1)
23-24: New Supported Data Types AdditionThe addition of Record (object) and Array to the list of supported types is clear and aligns with the new custom JSON serializer functionality. Ensure that these types are also correctly handled in the serializer implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/content/docs/openapi/openapi-handler.md(2 hunks)apps/content/docs/rpc-handler.md(1 hunks)
🔇 Additional comments (3)
apps/content/docs/openapi/openapi-handler.md (1)
35-37: Addition of Custom Serializer TipThe new tip block providing guidance to extend supported types via a custom serializer is well-formulated and consistent with similar documentation for other handlers. Double-check that the linked documentation (
/docs/openapi/advanced/openapi-json-serializer#extending-native-data-types) is current.apps/content/docs/rpc-handler.md (2)
27-28: New Supported Data Types AdditionThe inclusion of Record (object) and Array in the list of native types is a valuable update that mirrors the changes in the OpenAPI documentation. This consistency reinforces the framework's expanded serialization capabilities.
35-37: Addition of Custom Serializer TipThe tip block advising users on how to extend the list of supported types via a custom serializer has been added in a clear style. Confirm that the hyperlink (
/docs/advanced/rpc-json-serializer#extending-native-data-types) is accurate and up-to-date.
Summary by CodeRabbit
New Features
OpenAPIHandlerandRPCHandlerto include Record (object) and Array.Tests
undefinedvalues), to ensure robust serialization behavior.