refactor: dedupe with PromiseWithError#310
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request updates several type definitions and test expectations to use the new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Interceptor
participant NextFunction
participant ErrorHandler
participant FinishHandler
Client->>Interceptor: Invoke operation with options
Interceptor->>NextFunction: Call next() -> PromiseWithError
alt Success Response
NextFunction-->>Interceptor: Return success result
Interceptor->>FinishHandler: Execute onFinish(success state)
else Error Occurs
NextFunction-->>Interceptor: Return promise with __error set
Interceptor->>ErrorHandler: Execute onError(extracted error)
Interceptor->>FinishHandler: Execute onFinish(error state)
end
Possibly related PRs
Poem
✨ 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:
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
More templates
@orpc/arktype
@orpc/client
@orpc/openapi
@orpc/openapi-client
@orpc/contract
@orpc/react
@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/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
![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 (2)
packages/shared/src/interceptor.ts (2)
60-60: Consider avoidinganytypeWhile the code functions correctly, the use of
as anybypasses TypeScript's type checking. Consider using a more specific type or a type guard if possible.- await callback(error as any, options, ...rest) + await callback(error as ReturnType<TOptions['next']> extends PromiseWithError<any, infer E> ? E : unknown, options, ...rest)
81-81: Consider using a more specific type thananyUsing
anyfor the state variable reduces type safety. Consider using the OnFinishState type with appropriate generic parameters:- let state: any + let state: OnFinishState< + Awaited<ReturnType<TOptions['next']>>, + ReturnType<TOptions['next']> extends PromiseWithError<any, infer E> ? E : unknown + > | undefined
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/client/src/types.ts(2 hunks)packages/shared/src/interceptor.test-d.ts(4 hunks)packages/shared/src/interceptor.ts(4 hunks)packages/shared/src/types.test-d.ts(2 hunks)packages/shared/src/types.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
packages/shared/src/interceptor.test-d.ts (2)
packages/shared/src/interceptor.ts (2)
Interceptor(14-18)onStart(23-30)packages/shared/src/types.ts (1)
PromiseWithError(5-5)
packages/client/src/types.ts (1)
packages/shared/src/types.ts (1)
PromiseWithError(5-5)
packages/shared/src/types.test-d.ts (1)
packages/shared/src/types.ts (1)
PromiseWithError(5-5)
🔇 Additional comments (14)
packages/shared/src/types.ts (1)
5-5: Well-designed type for error handlingThe new
PromiseWithErrortype creates a clean, reusable pattern for attaching error information to Promise objects. This type will help standardize error handling across the codebase and reduce duplication.packages/shared/src/interceptor.test-d.ts (2)
2-2: Good addition of import for new typeAdding the import for
PromiseWithErrormakes sense given the type test updates that follow.
9-9: Nice simplification of type testsReplacing the complex intersection type with the new
PromiseWithErrortype makes these tests more readable and maintainable while still testing the same functionality.Also applies to: 19-19, 29-29, 39-39
packages/shared/src/types.test-d.ts (2)
1-1: Appropriate import updateAdding the
PromiseWithErrorimport here is necessary for the new test case.
16-21: Good test coverage for the new typeThis test case thoroughly validates that the
PromiseWithErrortype behaves as expected with respect to type inference. Testing with union types includingundefinedandnullis particularly valuable as these are common edge cases.packages/client/src/types.ts (2)
1-2: Appropriate new importAdding the import for
PromiseWithErrorfrom@orpc/sharedis necessary for its usage in the updated type definition.
18-18: Good refactoring to use the shared typeReplacing the custom promise-with-error implementation with the shared
PromiseWithErrortype reduces duplication and creates a more consistent API across the codebase. The constraintTError extends Erroris maintained from the original definition, ensuring type safety.packages/shared/src/interceptor.ts (7)
2-2: Import successfully adds the new PromiseWithError typeThis import aligns with the PR objective of deduplicating code by introducing the shared PromiseWithError type.
11-11: Improved type clarity with PromiseWithErrorThe return type update for the
nextmethod improves type safety by using the dedicatedPromiseWithErrortype instead of a more complex union type construction.
18-18: Consistent return type updateThe Interceptor return type is now consistently using PromiseWithError, maintaining alignment with the updated InterceptorOptions type.
48-54: Enhanced type extraction for error handlingThe updated onError function now properly extracts the error type from PromiseWithError using conditional types, providing better type inference when using the interceptor pattern.
71-80: Improved state type handling for onFinishThe onFinish function now correctly derives the error type from PromiseWithError, maintaining consistency with the onError implementation and improving type inference.
90-91: Simplified error handlingThe error handling is now simpler without explicit type casting, leveraging the PromiseWithError type structure effectively.
94-94: Cleaner callback implementationThe callback now receives the correctly typed state without needing additional type assertions, improving code readability.
1 participant
Summary by CodeRabbit
New Features
Refactor
Tests