Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes refactor how options are passed in various client modules. Multiple functions that previously accepted separate parameters (e.g., options, path, input) now receive a single consolidated object. This update affects test cases and implementations in the fetch client, standard link, RPC codec, retry plugin, and type definitions. Interface signatures have been updated to use new interceptor option types, and deprecated parameter structures have been removed, standardizing calls across these modules. Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test
participant LFC as LinkFetchClient
participant F as fetch API
T->>LFC: call(request, options, path, input)
LFC->>F: fetch(request, init, { ...options, path, input })
sequenceDiagram
participant T as Test
participant SL as StandardLink
participant I as Interceptor
T->>SL: call(request, options, path, input)
SL->>I: intercept({ ...options, path, input, request })
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
✨ 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/openapi
@orpc/openapi-client
@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 (1)
packages/client/src/plugins/retry.ts (1)
39-39: Refine return type to avoid confusing void
According to the static analysis hints, usingvoidwithin a union can be confusing. Consider replacing it withundefinedor a more specific type.Apply this diff to clarify the union type:
- onRetry?: (options: ClientRetryPluginAttemptOptions<ClientRetryPluginContext>) => void | (() => void) + onRetry?: (options: ClientRetryPluginAttemptOptions<ClientRetryPluginContext>) => undefined | (() => void)🧰 Tools
🪛 Biome (1.9.4)
[error] 39-39: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/client/src/adapters/fetch/link-fetch-client.test.ts(1 hunks)packages/client/src/adapters/fetch/link-fetch-client.ts(2 hunks)packages/client/src/adapters/standard/link.test.ts(1 hunks)packages/client/src/adapters/standard/link.ts(3 hunks)packages/client/src/adapters/standard/rpc-link-codec.ts(5 hunks)packages/client/src/plugins/retry.test.ts(7 hunks)packages/client/src/plugins/retry.ts(4 hunks)packages/client/src/types.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (5)
packages/client/src/adapters/fetch/link-fetch-client.ts (2)
packages/client/src/types.ts (1)
ClientContext(6-6)packages/client/src/adapters/standard/link.ts (1)
StandardLinkInterceptorOptions(13-16)
packages/client/src/plugins/retry.ts (2)
packages/client/src/types.ts (1)
ClientContext(6-6)packages/client/src/adapters/standard/link.ts (1)
StandardLinkInterceptorOptions(13-16)
packages/client/src/types.ts (2)
packages/server/src/index.ts (1)
ClientContext(24-24)packages/client/tests/helpers.ts (1)
ClientContext(130-130)
packages/client/src/adapters/standard/rpc-link-codec.ts (3)
packages/client/src/adapters/standard/link.ts (1)
StandardLinkInterceptorOptions(13-16)packages/client/src/types.ts (1)
HTTPMethod(4-4)packages/standard-server/src/types.ts (1)
StandardHeaders(1-3)
packages/client/src/adapters/standard/link.ts (3)
packages/client/src/types.ts (2)
ClientContext(6-6)ClientOptions(8-12)packages/standard-server/src/types.ts (1)
StandardRequest(13-24)packages/shared/src/interceptor.ts (2)
Interceptor(14-18)intercept(101-122)
🪛 Biome (1.9.4)
packages/client/src/plugins/retry.ts
[error] 39-39: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (49)
packages/client/src/adapters/fetch/link-fetch-client.test.ts (1)
44-44: LGTM: Parameter consolidation follows new patternThe test now properly uses the consolidated parameter pattern, combining
options,path, andinputinto a single object. This matches the implementation changes in thelink-fetch-client.tsfile.packages/client/src/adapters/standard/link.test.ts (2)
46-49: LGTM: Parameter structure updated correctlyThe test now uses individual properties (
context,signal,lastEventId) instead of passing them in a single options object. This matches the updated interceptor API pattern.
55-59: LGTM: Required parameters added to clientInterceptor testThe test now correctly includes
pathandinputproperties in the clientInterceptor call expectations, along with the expanded individual context properties. This properly reflects the standardized parameter structure.packages/client/src/plugins/retry.test.ts (12)
61-61: LGTM: Retry function call expectations updatedTest now correctly verifies that the retry function receives a consolidated object containing context, path, and input properties.
103-109: LGTM: shouldRetry function expectations updatedTest now properly verifies that shouldRetry receives an object containing all required properties (attemptIndex, error, context, input, path).
114-121: LGTM: Second shouldRetry call expectations updatedThe second call expectations are correctly updated to match the new parameter structure.
137-144: LGTM: onRetry function expectations updatedTest now correctly verifies that onRetry receives the standardized parameter object structure.
147-154: LGTM: Second onRetry call expectations updatedThe second onRetry call expectations are properly updated to match the new parameter structure.
157-164: LGTM: Third onRetry call expectations updatedThe third onRetry call expectations are properly updated to match the new parameter structure.
281-288: LGTM: shouldRetry expectations in event iterator test updatedTest now properly verifies that shouldRetry in the event iterator context receives the consolidated parameter object with required properties.
386-393: LGTM: shouldRetry expectations in shouldRetry=false test updatedFirst shouldRetry call verification is properly updated to the new object parameter structure.
396-403: LGTM: Second shouldRetry call in shouldRetry=false test updatedSecond shouldRetry call verification is properly updated to the new object parameter structure.
424-432: LGTM: onRetry expectations in event iterator test updatedFirst onRetry call verification in event iterator context is properly updated to the new parameter structure.
435-443: LGTM: Second onRetry call in event iterator test updatedSecond onRetry call verification is properly updated to the new parameter structure.
446-454: LGTM: Third onRetry call in event iterator test updatedThird onRetry call verification is properly updated to the new parameter structure.
packages/client/src/adapters/fetch/link-fetch-client.ts (3)
4-4: LGTM: Added StandardLinkInterceptorOptions importCorrectly imported StandardLinkInterceptorOptions type to support the updated fetch method signature.
11-11: LGTM: Updated fetch method signatureThe fetch method signature now correctly uses StandardLinkInterceptorOptions instead of ClientOptions, which includes the additional path and input properties needed for the new parameter structure.
27-27: LGTM: Updated fetch method callThe fetch method call now correctly spreads the options and adds path and input to a single object, matching the updated signature requirements.
packages/client/src/types.ts (2)
14-16: Advanced conditional type usage
This conditional setup forFriendlyClientOptionsis clever and appears to fulfill the requirement thatcontextbe optional only whenTis an empty type. Nicely done!
8-12:Details
✅ Verification successful
Verify mandatory context usage
Now thatClientOptionsrequires acontextproperty, please confirm that all consuming code correctly provides this property. Missing or undefinedcontextreferences may cause runtime errors.Below is a script to locate all references to
ClientOptionsand confirm thatcontextis always passed:
🏁 Script executed:
#!/bin/bash # Scan the entire repository for references to ClientOptions and show surrounding lines rg -A 5 "ClientOptions<"Length of output: 17619
Verification Complete: All Consumers Provide a Valid
contextAfter reviewing the repository, I've confirmed that every use of
ClientOptionscorrectly supplies a definedcontext. In particular:
Default Handling in Utility:
The functionresolveFriendlyClientOptions(inpackages/client/src/utils.ts) provides a default (an empty object cast to typeT) whencontextisn’t supplied, ensuring no runtime errors occur.Explicit Context in Tests and Implementations:
For example, inpackages/client/src/dynamic-link.test.ts,contextis explicitly provided. Similarly, other consumers inclient.ts, various adapter files, and server implementations pass thecontexteither directly or via the utility.Consistent Propagation Across Adapters:
All adapter and client method calls (including those in the standard, fetch, and openapi modules) forward a validClientOptionsobject that always contains a propercontext.No further action is required regarding the mandatory
contextproperty.packages/client/src/plugins/retry.ts (15)
2-3: Imports updated to reflect new interceptors
The revised import lines correctly reflect the new consolidated options approach.
7-7: Interface extension for consistency
ExtendingClientRetryPluginAttemptOptionsfromStandardLinkInterceptorOptions<T>unifies retry logic with the standard link mechanism. This looks good.
20-20: Refined parameterization
Definingretry?to acceptStandardLinkInterceptorOptions<ClientRetryPluginContext>ensures consistent argument passing and simplifies usage.
27-27: Enhanced retryDelay signature
Switching to[ClientRetryPluginAttemptOptions<ClientRetryPluginContext>]gives access to more contextual data (likeattemptIndex) for computing delay.
34-34: Aligned shouldRetry signature
AcceptingClientRetryPluginAttemptOptions<ClientRetryPluginContext>parallelizes the approach used inretryDelayfor richer context.
66-67: Correct usage of default or context-supplied retry value
The code correctly falls back onthis.defaultRetryifcontext.retryis undefined. This promotes flexible configuration.
70-72: Successfully retrieving other context-based retry settings
ExtractingretryDelay,shouldRetry, andonRetryfrominterceptorOptions.contextensures each property can be overridden individually.
78-78: Local storage of lastEventId
StoringlastEventIdin a local variable is a clean approach, avoiding repeated property lookups. Looks good.
87-87: Configuring updated interceptor options
Merging the updatedlastEventIdback intointerceptorOptionsis straightforward and ensures consistent usage downstream.
94-95: Constructing attemptOptions
Spreading all updated interceptor options plus additional fields (attemptIndex,error, etc.) creates a cohesive context for retry logic.
100-100: Closing object construction
This line simply concludes the object definition. No issues here.
110-110: Conditional function invocation
Using the optional chaining operator withonRetryis a robust, concise way to handle potential undefined references.
112-112: Awaiting dynamic retryDelay
Awaiting the resolved delay fromretryDelayis consistent with the rest of the dynamic value logic. Good approach.
120-120: Returning from interceptor
Forwarding the call tointerceptorOptions.next(updatedInterceptorOptions)completes the chain of interceptors neatly.
123-124: Check for signal abort
Throwing the error when the signal is aborted ensures the retry loop stops gracefully.packages/client/src/adapters/standard/rpc-link-codec.ts (8)
2-2: Updated link import
ImportingStandardLinkInterceptorOptionsis appropriate for the newly consolidated options pattern.
14-14: Use of StandardLinkInterceptorOptions
Revising theurlproperty to rely on a single argument ofStandardLinkInterceptorOptions<T>streamlines method calls.
21-21: Unified approach for maxUrlLength
AdaptingmaxUrlLengthto acceptStandardLinkInterceptorOptions<T>aligns well with the rest of the refactoring.
28-28: Consistent HTTPMethod signature
Switchingmethodto use[StandardLinkInterceptorOptions<T>]follows the same consolidated options structure.
41-41: Consolidated headers handling
Applyingheadersas aValue<StandardHeaders, [StandardLinkInterceptorOptions<T>]>ensures consistent scoping of request customization.
63-63: Forming a single generalOptions object
Mergingoptions,path, andinputintogeneralOptionssimplifies subsequent calls tovalue(). Good approach.
65-67: Leverage dynamic property resolution
Fetching runtime values such asexpectedMethod,headers, andbaseUrlviavalue()with the newly combinedgeneralOptionsis cleanly implemented.
82-82: Adjusted maxUrlLength usage
Usingvalue(this.maxUrlLength, generalOptions)ensures the correct property is dynamically retrieved during runtime.packages/client/src/adapters/standard/link.ts (6)
13-16: Good addition of a well-structured interfaceThis new interface consolidates previously separate parameters into a single type, making the code more maintainable and type-safe. The extension of
ClientOptions<T>is appropriate as it builds on the existing type system.
18-20: Proper hierarchical type extensionThis interface correctly extends
StandardLinkInterceptorOptions<T>to include therequestproperty, providing a clean type hierarchy that reflects the functional relationships between these interceptor options.
23-24: Good refactoring to use the new interfacesReplacing inline type definitions with the newly created interfaces improves code readability and maintainability. This change makes the code more self-documenting and easier to understand.
46-46: Properly updated parameter structureThe updated parameter structure correctly consolidates the previously separate parameters into a single object, making the function call cleaner and more consistent with modern JavaScript/TypeScript patterns.
58-59: Correctly refactored parameter passingThe consolidation of parameters in this interceptor call matches the pattern established in the rest of the PR. The destructuring in the callback ensures that the parameters are used correctly in the function call.
13-59:Details
❓ Verification inconclusive
Verify that all consumers are updated for this breaking change
These interface changes and parameter consolidation constitute a breaking change (as noted by the
!in the PR title). While the changes themselves are well-structured, it's important to ensure all consumers of these APIs are updated.
🏁 Script executed:
#!/bin/bash # Search for direct imports and usages of StandardLink echo "Searching for direct imports and usages of StandardLink" rg -t ts -A 5 "import.*StandardLink" --exclude="link.ts" # Search for usages of call method echo "Searching for calls to 'call' method that might need updating" rg -t ts "\.call\(\s*[^{].*path.*input.*options" --exclude="link.ts"Length of output: 1209
Below is a revised review comment that reflects your concerns and the difficulties encountered with our automated search. Note that while we attempted to verify consumer updates using automated searches, the original flags caused errors. A revised script using the correct flags is suggested below. However, please manually verify that all consumers have updated their usage of the new
StandardLinkAPIs.Action: Confirm Consumer Updates for Breaking Change
The modifications to the
StandardLinkinterfaces and consolidation of parameters introduce a breaking change (as indicated by the!in the PR title). While automated searches usingrgwere attempted, initial runs encountered flag errors. A revised verification script is provided below that uses the correct flag (--glob '!**/link.ts') instead of--exclude. Please confirm manually that:
- No outdated imports or direct usages of
StandardLinkexist in TypeScript files (other than inlink.ts).- All calls to the
callmethod throughout the codebase have been updated to meet the new parameter structure.Suggested revised verification script:
#!/bin/bash echo "Searching for direct imports and usages of StandardLink" rg -t ts --glob '!**/link.ts' "import.*StandardLink" || true echo "Searching for calls to 'call' method that might need updating" rg -t ts --glob '!**/link.ts' "\.call\(" || truePlease rerun these commands and perform a manual review to ensure that all consumers have been updated accordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/content/docs/plugins/client-retry.md (2)
27-36: Refined Parameter Destructuring in the DefaultretryOption
The updated function signature now uses object destructuring ({ path }), which streamlines parameter handling. Please ensure that all callers guarantee thatpathis provided in the expected format (typically an array) so that the use of.join('.')does not lead to runtime errors. Consider adding a brief inline comment or update the documentation to clarify the expected type forpath.
52-60: Clarify Consistency Between Default Options and Client Context in Usage Example
In the default options (lines 27–36),retryis specified as a function, while in the usage snippet (lines 52–60) theretryvalue is provided as a number. Make sure the documentation clearly explains the dual nature of this property—i.e. when it is used as a callable function versus a numeric override. This clarification will help prevent any confusion for clients integrating with the plugin.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/content/docs/client/rpc-link.md(1 hunks)apps/content/docs/plugins/client-retry.md(1 hunks)packages/client/src/adapters/fetch/rpc-link.test.ts(2 hunks)
🔇 Additional comments (2)
apps/content/docs/client/rpc-link.md (1)
74-74: Refactored Parameter Destructuring in Custom Request Method
The updated function now destructures bothcontextandpathfrom a single parameter object, which aligns with the PR objectives of consolidating options and standardizing parameter handling. This change simplifies the function signature while maintaining clarity. Ensure that all downstream calls and references to this function have been updated accordingly.packages/client/src/adapters/fetch/rpc-link.test.ts (1)
21-21: Updated fetch signature aligned with the PR objectives.The change from
fetch: async (url, init) => {...}tofetch: async (request) => {...}consolidates parameters into a single object, which aligns with the PR objective to standardize how options are passed in client modules. This is consistent with modern fetch API patterns and makes the code more maintainable.Also applies to: 53-53
revert some options from "feat(client)!: update standard link interceptors and plugins options (#333)"
revert some options from "feat(client)!: update standard link interceptors and plugins options (#333)" <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Improved the ordering and handling of request parameters in the client API. - Enhanced overall consistency in how communication options are processed, ensuring smoother API interactions without affecting the end-user experience. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 participant
This update change options related to
RPCLinkandOpenAPILinkfrom receive 3 args like thisoptions, input, pathto this{ context, input, path }Summary by CodeRabbit
Refactor
Tests
requestparameter, simplifying the interface.