fix(client): httpBatchLink with custom transformed object at top level
#6878
Conversation
|
🚅 Deployed to the trpc-pr-6878 environment in trpc-sse-and-websockets
|
WalkthroughThis change standardizes transformer function signatures, replaces a local Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant httpBatchStreamLink
participant Server
participant jsonlStreamProducer/Consumer
participant Transformer
Client->>httpBatchStreamLink: Send request (custom class)
httpBatchStreamLink->>Server: Forward request
Server->>Transformer: serialize(customClassInstance)
Transformer-->>Server: Serialized data
Server->>jsonlStreamProducer: Pass data with (data) => serialize(data)
jsonlStreamProducer-->>Server: Streamed JSONL response
Server-->>Client: Streamed response
Client->>jsonlStreamConsumer: Pass data with (data) => deserialize(data)
jsonlStreamConsumer->>Transformer: deserialize(data)
Transformer-->>jsonlStreamConsumer: Custom class instance
jsonlStreamConsumer-->>Client: Custom class instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ 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). (20)
🔇 Additional comments (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@trpc/client
@trpc/next
@trpc/react-query
@trpc/server
@trpc/tanstack-react-query
@trpc/upgrade
commit: |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/server/src/unstable-core-do-not-import/transformer.ts (1)
33-37: Same duplication comment as above applies toOutputDataTransformer
🧹 Nitpick comments (1)
packages/server/src/unstable-core-do-not-import/transformer.ts (1)
22-26: Redundant method redeclaration – let docs extend the base interface
InputDataTransformeralready extendsDataTransformer; repeating identical method signatures adds maintenance overhead and risks drift.Consider:
-interface InputDataTransformer extends DataTransformer { - /** Runs **on the client** before sending the data to the server. */ - serialize(object: any): any; - /** Runs **on the server** to transform the data before it is passed to the resolver */ - deserialize(object: any): any; -} +/** + * Transformer directions from client ➜ server. + * Inherits the call signatures from `DataTransformer`. + */ +interface InputDataTransformer extends DataTransformer {}You keep the descriptive comment while avoiding duplicate type definitions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/client/src/links/httpBatchStreamLink.ts(1 hunks)packages/server/src/unstable-core-do-not-import/http/resolveResponse.ts(1 hunks)packages/server/src/unstable-core-do-not-import/stream/jsonl.ts(1 hunks)packages/server/src/unstable-core-do-not-import/transformer.ts(1 hunks)packages/server/src/vendor/is-plain-object.ts(1 hunks)packages/tests/server/transformer.test.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: the cli options `trpcfile` and `trpcimportname` in `packages/upgrade/src/bin/cli.ts` are temporary a...
Learnt from: barelyhuman
PR: trpc/trpc#6530
File: packages/upgrade/src/bin/cli.ts:218-222
Timestamp: 2025-02-19T20:31:52.088Z
Learning: The CLI options `trpcFile` and `trpcImportName` in `packages/upgrade/src/bin/cli.ts` are temporary and planned to be removed in favor of using automatically discovered references from the TypeScript program.
Applied to files:
packages/server/src/unstable-core-do-not-import/stream/jsonl.ts
📚 Learning: in the trpc monorepo, packages/tests and packages/upgrade don't need main/module/types fields in the...
Learnt from: juliusmarminge
PR: trpc/trpc#6789
File: packages/tanstack-react-query/tsdown.config.ts:1-24
Timestamp: 2025-06-09T14:01:20.033Z
Learning: In the tRPC monorepo, packages/tests and packages/upgrade don't need main/module/types fields in their package.json because tests is not distributed and upgrade just exposes a binary.
Applied to files:
packages/server/src/unstable-core-do-not-import/stream/jsonl.tspackages/tests/server/transformer.test.ts
📚 Learning: in the minimal-react-typesafe-errors example, the database mock in db.ts should throw generic error ...
Learnt from: heitorlisboa
PR: trpc/trpc#6849
File: examples/minimal-react-typesafe-errors/server/db.ts:18-20
Timestamp: 2025-06-29T21:25:46.435Z
Learning: In the minimal-react-typesafe-errors example, the database mock in db.ts should throw generic Error objects rather than typed errors. The typed errors are handled at the tRPC procedure layer, which catches database errors and transforms them into API-specific typed errors. This maintains proper architectural separation between the database layer and API layer.
Applied to files:
packages/tests/server/transformer.test.ts
🧬 Code Graph Analysis (1)
packages/server/src/vendor/is-plain-object.ts (1)
packages/server/src/unstable-core-do-not-import/utils.ts (1)
isObject(31-33)
⏰ 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). (20)
- GitHub Check: E2E-tests (vercel-edge-runtime)
- GitHub Check: E2E-tests (soa)
- GitHub Check: E2E-tests (standalone-server)
- GitHub Check: E2E-tests (nuxt)
- GitHub Check: E2E-tests (express-minimal)
- GitHub Check: E2E-tests (next-formdata)
- GitHub Check: E2E-tests (tanstack-start)
- GitHub Check: E2E-tests (next-prisma-todomvc)
- GitHub Check: E2E-tests (.test/diagnostics-big-router)
- GitHub Check: E2E-tests (next-minimal-starter)
- GitHub Check: E2E-tests (.test/internal-types-export)
- GitHub Check: E2E-tests (cloudflare-workers)
- GitHub Check: E2E-tests (express-server)
- GitHub Check: e2e-legacy-node (next-prisma-websockets-starter, 18.x)
- GitHub Check: e2e-legacy-node (next-prisma-todomvc, 20.x)
- GitHub Check: e2e-legacy-node (next-prisma-starter, 18.x)
- GitHub Check: Test a monorepo using built declaration files
- GitHub Check: E2E-tests (Bun) (bun, ubuntu-latest)
- GitHub Check: E2E-tests (Deno) (deno-deploy)
- GitHub Check: test
🔇 Additional comments (9)
packages/server/src/vendor/is-plain-object.ts (2)
1-32: LGTM! Comprehensive plain object detection implementation.The new
isPlainObjectutility provides robust detection logic that properly handles custom classes and edge cases. This addresses the core issue described in the PR objectives where custom classes were not properly identified.
8-10: No consolidation needed forisObjectimplementations.The private
isObjectinpackages/server/src/vendor/is-plain-object.tsis only used internally byisPlainObjectto perform a strict “[object Object]” check (matching the upstream library’s behavior). The exportedisObjectinpackages/server/src/unstable-core-do-not-import/utils.tsprovides a broader generic object check (!!value && !Array.isArray(value) && typeof value === 'object') used throughout the codebase. They serve different purposes, have distinct semantics, and the vendored implementation isn’t even exported or imported elsewhere—merging them would change behavior.Likely an incorrect or invalid review comment.
packages/server/src/unstable-core-do-not-import/stream/jsonl.ts (2)
1-1: Good consolidation of plain object detection logic.Replacing the local
isPlainObjectimplementation with the shared utility from the vendor module improves consistency and leverages the more robust detection logic that properly handles custom classes.
237-239: Critical fix for custom class handling.This usage of the new
isPlainObjectfunction is where the fix for issue #6863 takes effect. The improved detection logic will correctly identify custom class instances as non-plain objects, preventing them from being incorrectly processed during encoding.packages/client/src/links/httpBatchStreamLink.ts (1)
94-95: Proper transformer method invocation.Wrapping the
deserializemethod in an arrow function ensures proper invocation regardless of the transformer implementation. This standardizes the function calling pattern and prevents potential issues with method context or reference handling.packages/server/src/unstable-core-do-not-import/http/resolveResponse.ts (1)
564-564: Consistent transformer method invocation pattern.This change mirrors the client-side pattern in
httpBatchStreamLink.ts, ensuring consistent handling of transformer methods across the codebase. The arrow function wrapper guarantees proper method invocation regardless of transformer implementation details.packages/tests/server/transformer.test.ts (3)
7-7: Good addition of missing import.Adding the
httpBatchStreamLinkimport enables comprehensive testing of the custom transformer functionality across all HTTP link types.
550-586: Comprehensive test setup for custom class serialization.The test suite properly sets up a custom class with superjson registration, creating a realistic scenario that matches the issue described in #6863. The custom transformer configuration correctly handles both serialization and deserialization of custom class instances.
588-630: Excellent regression test coverage.These tests verify that custom class instances are properly handled across all HTTP link types, with specific focus on the
httpBatchStreamLinkregression mentioned in issue #6863. The tests confirm that custom objects maintain their class identity after serialization/deserialization round trips.
|
This pull request has been locked because we are very unlikely to see comments on closed issues. If you think, this PR is still necessary, create a new one with the same branch. Thank you. |
Closes #6863
🎯 Changes
✅ Checklist
Summary by CodeRabbit
Bug Fixes
Refactor
New Features
Tests