fix: .next broken when calling multiple times#337
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis PR updates both test and production code related to middleware and interceptor flows. In the tests, it replaces Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Procedure Test
participant PC as Procedure Client
participant MW as Middleware (multiple)
participant Handler as Final Handler
Test->>PC: Invoke procedure client
PC->>MW: Call next(index, context, input)
MW-->>PC: Return output and update context
PC->>MW: Call next(index+1, mergedContext, newInput)
MW-->>PC: Return output
PC->>Handler: Execute final handler with context
Handler-->>PC: Return final result
PC-->>Test: Output aggregated responses
sequenceDiagram
participant Test as Interceptor Test
participant Int as intercept Function
participant I1 as Interceptor1
participant I2 as Interceptor2
participant Main as Main Function
Test->>Int: Call intercept with options and main
Int->>I1: Invoke interceptor at index 0
I1->>Int: Call next(index+1) with updated options
Int->>I2: Invoke interceptor at index 1
I2->>Int: Call next(index+1) with updated options
Int->>Main: Invoke main function
Main-->>Int: Return result
I2-->>Int: Return result after processing
I1-->>Int: Return result after processing
Int-->>Test: Return aggregated results
Possibly related PRs
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
WARN GET http://10.0.0.28:4873/@antfu/eslint-config/-/eslint-config-3.16.0.tgz error (ERR_PNPM_FETCH_503). Will retry in 10 seconds. 2 retries left. This error happened while installing a direct dependency of /tmp/eslint ✨ 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/contract
@orpc/openapi
@orpc/openapi-client
@orpc/react-query
@orpc/react
@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/server/src/procedure-client.test.ts (1)
483-512: Great test coverage for multiple.nextcallsThis test case effectively verifies that multiple calls to
next()within middleware work correctly, checking both the output array and the proper context propagation between calls. This directly addresses the issue mentioned in the PR title.However, consider using a type-safe approach instead of
(handler as any).mock.calls:- expect((handler as any).mock.calls[0][0].context.preMid2).toBe(0) + expect(handler.mock.calls[0][0].context.preMid2).toBe(0)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/server/src/procedure-client.test.ts(2 hunks)packages/server/src/procedure-client.ts(1 hunks)packages/shared/src/interceptor.test.ts(1 hunks)packages/shared/src/interceptor.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
packages/server/src/procedure-client.ts (2)
packages/server/src/context.ts (2)
Context(1-1)mergeCurrentContext(11-16)packages/server/src/middleware.ts (1)
middlewareOutputFn(64-66)
packages/shared/src/interceptor.test.ts (1)
packages/shared/src/interceptor.ts (1)
intercept(101-120)
🔇 Additional comments (4)
packages/server/src/procedure-client.test.ts (1)
48-48: Improved test isolation by usingresetAllMocksReplacing
clearAllMocks()withresetAllMocks()is a good change. WhileclearAllMocks()only clears the record of calls,resetAllMocks()additionally resets all mock implementations to their initial state, providing better isolation between tests.packages/shared/src/interceptor.test.ts (1)
119-152: Good test case for multiple interceptor.nextcallsThis test case thoroughly verifies that the
interceptfunction properly handles multiple calls tonext(). It creates an interceptor that callsnext()three times and verifies:
- The result is an array containing the expected values
- Each interceptor is called the expected number of times
- The arguments passed to each function are correct
This test works well with the refactored implementation in
interceptor.ts.packages/server/src/procedure-client.ts (1)
165-196: Improved middleware execution with explicit parametersThe refactored
nextfunction with explicit parameters (index,context,input) is a significant improvement over using closures to manage state. This change:
- Makes the flow of data through middleware more predictable
- Explicitly tracks the execution context through each middleware call
- Fixes the bug with multiple
.nextcalls by ensuring proper context and index managementThe use of
mergeCurrentContextensures that context is properly accumulated during multiple calls.packages/shared/src/interceptor.ts (1)
106-120: Enhanced interceptor execution with explicit index trackingThis change improves the
interceptfunction by making the next function take an explicitindexparameter instead of relying on mutable state. This:
- Makes the interceptor chain more predictable
- Allows for multiple
.nextcalls without index tracking issues- Aligns with the pattern used in the procedure client
The explicit passing of
index + 1to the next call ensures proper progression through the interceptor chain.
1 participant
Summary by CodeRabbit