fix(server): reflect middleware initial context in procedure when use#234
fix(server): reflect middleware initial context in procedure when use#234
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces comprehensive updates to middleware context handling across several server modules. Changes include the addition of a new Changes
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
ERR_PNPM_OPTIONAL_DEPS_REQUIRE_PROD_DEPS Optional dependencies cannot be installed without production dependencies Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ 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! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/server/src/context.test-d.ts (1)
14-16: Consider using a type alias instead of an empty interface.The
Emptyinterface doesn't have any properties, which is equivalent to{}. Consider using a type alias for better clarity.-interface Empty { - -} +type Empty = {}🧰 Tools
🪛 Biome (1.9.4)
[error] 13-16: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
packages/server/src/builder-variants.test-d.ts(15 hunks)packages/server/src/builder-variants.ts(8 hunks)packages/server/src/builder.test-d.ts(6 hunks)packages/server/src/builder.ts(3 hunks)packages/server/src/context.test-d.ts(1 hunks)packages/server/src/context.test.ts(1 hunks)packages/server/src/context.ts(1 hunks)packages/server/src/implementer-procedure.test-d.ts(6 hunks)packages/server/src/implementer-procedure.ts(3 hunks)packages/server/src/implementer-variants.test-d.ts(2 hunks)packages/server/src/implementer-variants.ts(2 hunks)packages/server/src/implementer.test-d.ts(2 hunks)packages/server/src/implementer.ts(2 hunks)packages/server/src/middleware-decorated.test-d.ts(5 hunks)packages/server/src/middleware-decorated.ts(2 hunks)packages/server/src/middleware.test-d.ts(1 hunks)packages/server/src/middleware.ts(2 hunks)packages/server/src/procedure-decorated.test-d.ts(3 hunks)packages/server/src/procedure-decorated.ts(2 hunks)packages/server/src/router-utils.test-d.ts(2 hunks)packages/server/src/router-utils.ts(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
packages/server/src/implementer.ts
[error] 52-52: Generic type 'EnhancedRouter' requires 4 type argument(s).
[error] 56-56: Generic type 'EnhancedRouter' requires 4 type argument(s).
[error] 52-52: Generic type 'EnhancedRouter' requires 4 type argument(s).
[error] 56-56: Generic type 'EnhancedRouter' requires 4 type argument(s).
packages/server/src/implementer-variants.ts
[error] 32-32: Generic type 'EnhancedRouter' requires 4 type argument(s).
[error] 36-36: Generic type 'EnhancedRouter' requires 4 type argument(s).
[error] 32-32: Generic type 'EnhancedRouter' requires 4 type argument(s).
[error] 36-36: Generic type 'EnhancedRouter' requires 4 type argument(s).
🪛 Biome (1.9.4)
packages/server/src/context.test-d.ts
[error] 13-16: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🔇 Additional comments (107)
packages/server/src/middleware-decorated.test-d.ts (5)
4-4: Appropriate replacement for removed MergedContext import.The import of
Contexttype is added correctly to replace the removedMergedContextimport, aligning with the changes to context handling approach in this PR.
55-55: Improved type safety with more precise context types.Changing from
CurrentContext & { extra: boolean }toOmit<CurrentContext, 'extra'> & { extra: boolean }improves type safety by explicitly handling property conflicts. This ensures the middleware'sextraproperty takes precedence over any potentialextraproperty inCurrentContext.Also applies to: 91-91
73-74: Enhanced context type definitions for better type safety.The updated type definitions for the returned middleware provide more precise type handling by:
- Using
CurrentContext & Record<never, never>for initial context- Using
Omit<{ extra: boolean }, never> & { extra2: boolean }for current contextThis approach correctly reflects how middleware contexts are merged, addressing the initial problem of middleware context types being ignored.
Also applies to: 113-114
82-83: Improved error messaging for context type mismatches.The error message update from context conflicts to "invalid TInContext" better describes the actual issue: incompatible initial context types between middlewares. This change helps developers understand type errors more easily.
Also applies to: 128-129
134-158: Thorough testing of initial context handling with middlewares.This new test case properly validates the middleware's behavior with specific initial context types. It confirms that the
DecoratedMiddlewarecorrectly merges the initial contexts from different middlewares while respecting type constraints.The specific test with
{ cacheable?: boolean }as initial context ensures that optional properties are handled correctly during the merging process.packages/server/src/context.test.ts (1)
1-1: Function renaming reflects improved context handling approach.Renaming from
mergeContexttomergeCurrentContextmakes the function's purpose more explicit, indicating that it specifically merges the current context rather than any context. This aligns with the PR's goal of distinguishing between initial and current context types in middleware.Also applies to: 3-5
packages/server/src/middleware.test-d.ts (1)
26-26: Simplified middleware next function type signature.Removing the context type parameter from
MiddlewareNextFnsimplifies the type signature while improving the separation between initial and current context handling. This change is part of the broader effort to fix how middleware context types are managed.packages/server/src/context.test-d.ts (4)
1-1: Updated imports reflect the new context handling approach.The import changes from
ConflictContextGuard, MergedContexttoContextExtendsGuard, MergedCurrentContext, MergedInitialContextproperly reflect the new type system that distinguishes between initial and current contexts.
3-7: Comprehensive testing of initial context merging.The tests for
MergedInitialContextthoroughly verify that:
- Properties from both input types are included in the merged result
- Conflicting properties are correctly identified (resulting in
nevertype)- Properties in the exclusion set are properly handled
This ensures the type system correctly enforces the desired behavior for initial context merging.
9-12: Thorough validation of current context merging.The tests for
MergedCurrentContextproperly verify that:
- Properties from both input types are included in the merged result
- When properties overlap, the second type's property takes precedence
This aligns with the expected behavior where middleware can override context properties.
18-28: Comprehensive testing of context extension guard.The tests for
ContextExtendsGuardthoroughly verify various extension scenarios:
- When the first type is assignable to the second type (returns
unknown)- When empty types or optional properties are involved
- When type extension fails (returns
never) due to incompatible typesThis ensures that middleware context type compatibility is properly enforced, addressing the core issue in the PR.
packages/server/src/implementer.ts (2)
3-3: Updated context type imports for enhanced middleware context handling.The imports now include the new
ContextExtendsGuard,MergedCurrentContext, andMergedInitialContexttypes which support the improved middleware context type validation introduced in this PR.
35-49: Improved middleware context type handling.This update to the
usemethod significantly enhances type safety by:
- Separating input and output context types with two generic parameters (
UOutContextandUInContext)- Using
ContextExtendsGuardto validate that the middleware's input context is compatible with the current context- Properly merging both initial and current contexts to preserve context type information throughout the middleware chain
This change directly addresses the PR's objective of reflecting middleware initial context in procedure types.
packages/server/src/implementer-variants.ts (2)
2-2: Updated context type imports for enhanced middleware context handling.The imports now include the new
ContextExtendsGuard,MergedCurrentContext, andMergedInitialContexttypes which are essential for the improved middleware context type system.
15-29: Improved middleware context type handling.This change to the
usemethod signature enhances type safety by:
- Using separate input and output context type parameters
- Adding
ContextExtendsGuardto validate that input context extends the current context- Properly merging initial and current contexts using the new utility types
These changes ensure that middleware context requirements are properly enforced throughout the middleware chain.
packages/server/src/implementer.test-d.ts (3)
4-4: Updated type imports to support new context handling system.The test now imports
MergedCurrentContextinstead of the older context merging type, and adds theMiddlewaretype to the imports. These changes align the tests with the new context handling system implemented in this PR.Also applies to: 10-10
91-125: Enhanced tests for middleware without input mapping.The updated test case for
.usewithout map input thoroughly validates the new context handling system. It includes:
- Detailed type checking for middleware parameters
- Verification that the resulting types use
MergedCurrentContextcorrectly- Additional tests for invalid input contexts and type mismatches
This ensures the middleware properly reflects the initial context types as intended by the PR.
127-137: Added test for middleware with custom input context.This new test case specifically validates the scenario where middleware specifies a custom input context (with
cacheable?property). It verifies that:
- The middleware's input context requirements are preserved in the resulting type
- The initial context is correctly merged with the input context requirements
This test directly addresses the core issue this PR aims to fix.
packages/server/src/middleware.ts (2)
16-18: Simplified middleware next function interface.The
MiddlewareNextFninterface has been simplified by:
- Removing the
TInContextgeneric parameter- Changing the constraint on the
Utype parameter fromContext & Partial<TInContext>to justContextThis change allows for more flexible context handling by decoupling the next function's context requirements from the input context, addressing the issue described in the PR.
37-37: Updated next function type in middleware options.The
nextproperty type has been updated to use the simplifiedMiddlewareNextFninterface, removing theTInContextparameter. This change is consistent with the interface update and completes the middleware context handling improvements.packages/server/src/router-utils.test-d.ts (6)
4-4: Added import for context merging types.The newly imported
ContextandMergedInitialContextare essential for properly handling middleware context types in router utilities.
21-24: Added type definitions for enhanced context testing.These new type definitions (
TErrorMap,InitialContext2,CurrentContext2) correctly model the extended context types needed to test the middleware context inheritance behavior.
26-37: Updated type assertions to use MergedInitialContext.This change fixes the core issue by ensuring that the procedure type correctly reflects the middleware's initial context. Now the
Enhanced['ping']type properly mergesInitialContext2with the originalInitialContext.
39-50: Applied MergedInitialContext to nested router procedures.Consistently applying the same context type changes to nested router procedures ensures type safety across the entire router hierarchy.
52-61: Applied context merging to pong procedure.Now the
pongprocedure also correctly usesMergedInitialContextto combineInitialContext2with the baseContext.
63-74: Applied context merging to nested pong procedure.Ensuring consistent application of the
MergedInitialContextapproach to all nested procedures.packages/server/src/implementer-variants.test-d.ts (5)
4-4: Updated import to use MergedCurrentContext.Replaced
MergedContextwithMergedCurrentContext, which better reflects the purpose of this type in handling the current context state during middleware execution.
10-10: Added Middleware type import.This allows for stronger typing when working with middleware in tests, enabling better type checking between middleware and procedures.
22-49: Enhanced use method test with explicit context handling.The test has been improved to:
- Clarify that it's testing the
.usemethod without mapping input- Verify type safety for all middleware parameters
- Test that extra context properties are properly merged with
MergedCurrentContext- Add type safety checks for the resulting implementer
This ensures the implementer correctly reflects middleware context.
51-56: Added tests for invalid context types.These tests validate that the type system correctly rejects invalid context types when using middleware, preventing potential runtime errors from mismatched contexts.
58-68: Added test for middleware with explicit input context.This test case validates the key fix in this PR - ensuring that when a middleware specifies an input context type (like
{ cacheable?: boolean }), the implementer correctly merges this with the existing context.packages/server/src/implementer-procedure.test-d.ts (7)
10-10: Added Middleware type import.Including the
Middlewaretype enables stronger typing for middleware components in the implementer procedure tests.
83-91: Refined context type handling in ImplementedProcedure.Updated to use
Omit<CurrentContext, 'extra'>instead of directly extending the context. This approach prevents property conflicts and provides more precise control over which properties are included in the merged context.
93-100: Added tests for invalid context inputs.These tests ensure that the type system properly rejects invalid input context types in middleware, preventing potential runtime errors from mismatched contexts.
142-166: Added support for middleware with explicit input context.This test case validates the core fix in this PR - when a middleware specifies an input context type (like
{ cacheable?: boolean }), the procedure correctly merges this with the existing context types.
259-266: Updated ProcedureImplementer context handling.Improved the type definition for
appliedto properly reflect how context types are merged in procedure implementers.
268-274: Added type validation for middleware context types.These tests ensure that invalid context types are properly rejected by the type system when using middleware with procedure implementers.
324-348: Added support for middleware with explicit input context in ProcedureImplementer.This test case confirms that the procedure implementer correctly merges the middleware's input context (
{ cacheable?: boolean }) with the existing context, both with and without input mapping.packages/server/src/procedure-decorated.test-d.ts (6)
7-7: Added Middleware type import.Including the
Middlewaretype enables stronger typing for middleware components in decorated procedure tests.
112-118: Refined context type handling in DecoratedProcedure.Updated to use
Omit<CurrentContext, 'extra'>for more precise control over merged context properties, ensuring that property conflicts are correctly handled.
121-129: Added tests for invalid context inputs.These tests verify that the type system correctly rejects invalid middleware context types, preventing potential runtime errors.
151-158: Updated context type handling for mapped inputs.Maintained consistent context type handling when using middleware with input mapping, ensuring proper type safety.
160-168: Added type validation for middleware with mapped inputs.These tests ensure that invalid context types are properly rejected even when using middleware with input mapping.
170-194: Added tests for middleware with explicit input context.These tests validate the key improvement in this PR - ensuring that decorated procedures correctly reflect and merge the initial context type specified by middleware (such as
{ cacheable?: boolean }).packages/server/src/context.ts (4)
3-7: Ensure clarity on key collisions in MergedInitialContext.
MergedInitialContextcleverly mergesTInitialand partialTAdditionalwhile excluding the overlapping keys fromTCurrent. This approach is valid, but be mindful of scenarios whereTInitialandTAdditionalshare keys not appearing inTCurrent; it may produce unexpected name collisions or overshadowing if the code evolves. Overall, the logic is sound.
9-9: MergedCurrentContext structure looks solid.
Omit<T, keyof U> & Uis a standard pattern for layering context types. This ensures thatUoverrides any corresponding keys inT.
11-14: Straightforward function for merging contexts.
mergeCurrentContextcorrectly applies the newMergedCurrentContexttype and merges the two objects. The implementation is straightforward and aligns with your updated type definitions.
18-19:Details
✅ Verification successful
Verify usage of ContextExtendsGuard in complex intersections.
ContextExtendsGuardconditionally yieldsunknownornever, ensuring type compatibility. In more advanced intersection or extension scenarios, false negatives or positives can occur if upstream contexts are especially intricate.To confirm correct usage, consider scanning for all references to
ContextExtendsGuardto ensure sound context constraints:
🏁 Script executed:
#!/bin/bash # Searching through the entire codebase for "ContextExtendsGuard" references: rg -A 5 "ContextExtendsGuard"Length of output: 17261
ContextExtendsGuard Usage Verified
The scan confirms thatContextExtendsGuardis consistently used across the codebase (in builder, implementer, middleware, etc.) and the tests incontext.test-d.tsvalidate its behavior in both straightforward and complex intersection scenarios. No issues were found; the conditional type correctly yieldsunknownorneveras expected.packages/server/src/procedure-decorated.ts (7)
4-4: Imports updated for new context merging types.
No issues here—importingContext,ContextExtendsGuard,MergedCurrentContext, andMergedInitialContextaligns with the implemented changes incontext.ts.
56-59: use(...) overload with defaulted UInContext is well-defined.
AllowingUInContextto default toTCurrentContextprovides flexibility. This ensures that middleware contexts can remain consistent if no specialized input context is needed.
65-66: Intersection return type for context validation.
CombiningContextExtendsGuard<UInContext, TCurrentContext>andContextExtendsGuard<TCurrentContext, MergedCurrentContext<TCurrentContext, UOutContext>>properly enforces correct context evolution across middleware.
68-69: MergedInitialContext in returned DecoratedProcedure.
Providing an updatedTInitialContextwith merged in-context types ensures that the final decorated procedure consistently reflects the newly added middleware’s context.
76-78: Overload with an additional UInput parameter.
This second overload adds an extra level of flexibility by letting a custom input type differ from the original schema output. The signature is coherent with the typed approach.
86-87: Intersection return type for the second overload.
The repeated pattern of context intersection remains consistent and helps maintain type safety across different input variants.
89-90: Refined TInitialContext and TCurrentContext type merges.
IntegratingMergedInitialContextandMergedCurrentContexthere matches the design incontext.ts, ensuring the final, decorated procedure’s context is properly updated.packages/server/src/implementer-procedure.ts (5)
5-5: Newly imported merged context utilities.
IncludingMergedCurrentContext,MergedInitialContext, andContextExtendsGuardreflects the unified approach introduced incontext.ts.
22-25: Revamped use(...) method for ImplementedProcedure (first overload).
AdoptingUOutContextandUInContextclarifies the distinction between incoming and outgoing middleware contexts. Returning bothContextExtendsGuardchecks and the updatedImplementedProcedureensures safe type evolution.Also applies to: 31-35
42-45: Second overload with custom input type.
Similar to the first overload, but includes an additionalUInputparameter and amapInputfunction. These changes maintain consistency with the overhauled context system while enabling tailored input transformations.Also applies to: 52-56
111-114: use(...) method in ProcedureImplementer (first overload).
Mirroring theImplementedProcedurepatterns, this ensures the builder interface also enforces correct context layering. The intersection types remain consistent with the broader design.Also applies to: 120-124
130-133: Extended overload in ProcedureImplementer with custom input.
ProvidingUInputand mapping logic is identical to the approach above. The type constraints effectively create a seamless middleware chain, preserving correctness from builder to final procedure.Also applies to: 140-144
packages/server/src/router-utils.ts (7)
2-2: Imports look good.
No concerns regarding importing these types; they are used consistently throughout the file.
74-79: Robust type definition for EnhancedRouter.
IntroducingTInitialContext,TCurrentContext, andTErrorMapas constraints strengthens context validation throughout the router. This approach improves maintainability and type safety.
81-83: Appropriate recursive type usage.
These lines properly extendEnhancedRouterfor nested lazy routers and procedures, preserving type safety in recursive structures.
91-91: Merging initial context.
UsingMergedInitialContext<TInitialContext, UInitialContext, TCurrentContext>ensures that all context layers are properly combined with minimal duplication.
99-99: Mapped type extension for child routers.
This mapped type approach effectively applies the enhanced router type logic to each entry in the router object, promoting scalability.
107-112: Enhanced generic parameters on enhanceRouter.
RequiringTInitialContext,TCurrentContext, andTErrorMapadds clarity and consistency, aligning with other updated router definitions.
115-115: Return type alignment for enhanceRouter.
ReturningEnhancedRouter<T, TInitialContext, TCurrentContext, TErrorMap>enforces consistency and ensures consumers get the fully typed router object.packages/server/src/middleware-decorated.ts (6)
2-2: Expanded imports for advanced context handling.
AddingContextExtendsGuard,MergedCurrentContext, andMergedInitialContextsupports more thorough type checks in middleware composition.
18-20: New generic parameters in concat signature.
AllowingUOutContext extends ContextandUInContext extends Context = MergedCurrentContext<...>ensures higher flexibility when chaining middlewares.
27-35: Strengthened type checks with ContextExtendsGuard.
This pattern ensures any concatenated middleware extends the existing in-context contract, preventing incompatible context merges.
40-41: Refined overload with default generic arguments.
DefaultingUInContexttoMergedCurrentContext<TInContext, TOutContext>helps avoid unwieldy type placeholders in common middleware compositions.
43-44: Consistent context enforcement in second overload.
RequiringUInContextaligns the typed input flow across multiple middleware usage scenarios.
51-59: Extended DecoratedMiddleware return signature.
By merging initial/current contexts and confirming them viaContextExtendsGuard, these lines guarantee that the final decorated middleware remains type safe.packages/server/src/builder.test-d.ts (7)
9-9: Added Middleware import.
Pulling inMiddlewareclarifies the typed approach to builder usage and tests.
235-236: Refined BuilderWithMiddlewares type.
WrappingInitialContextand adjustingCurrentContextwithextraproperty is consistent with how contexts are extended in the codebase.
245-246: Ensuring incompatible TInContext yields never.
These test lines confirm that invalid middleware context types are disallowed, preventing silent type mismatches.
277-278: Additional context merging test.
Again verifyingInitialContext & Record<never, never>merges with a specializedCurrentContextthat includesextra.
292-293: Incompatible TInContext scenario repeated.
This negative test ensures the builder rejects middleware with incompatible context types, preserving correctness.
434-435: Correct usage of EnhancedRouter with router.
These lines confirm that.router(...)returns a strongly typedEnhancedRouterreflecting the updated context and error map.
459-460: Lazy router integration with EnhancedRouter.
The type check validates that a lazily imported router is properly wrapped withEnhancedRouter, maintaining the same context constraints.packages/server/src/builder-variants.test-d.ts (14)
8-8: New imports look appropriate.
Pulling inMiddlewareandMiddlewareOutputFnbetter supports the updated.usemethod type checks and output function references.
71-108: Thorough type testing for.usewithout mapped input.
These changes correctly ensure that the middleware function sees the right input (“unknown” by default) and merges the context intoCurrentContextas expected. The negative tests (@ts-expect-error) further confirm that invalid input contexts or output mappings are detected properly.
110-123: Consistent handling ofTInContext.
AllowingTInContextto default toTCurrentContextwhen unspecified—and mapping explicitly when needed—provides crucial flexibility in typed middleware usage. This matches the broader approach and looks good.
331-367: Expanded.usecoverage forProcedureBuilder.
These lines mirror the same context-merging approach, ensuring consistent type-checking for procedures. The tests are well-structured, with negative checks confirming type safety.
370-383:TInContextusage withProcedureBuilderis well-validated.
Explicitly testing optionalcacheable?: booleanmerges seamlessly with the existingCurrentContext. The negative test coverage looks robust.
547-548: Context merging logic in.useforProcedureBuilderWithInput.
The lines reflect the same strategy of verifying the updated context types (e.g.,extra: boolean). This helps ensure every new context key merges properly.Also applies to: 556-557
613-636: Proper mapping forTInContextinProcedureBuilderWithInput.
Splitting the middleware function from the mapping function is especially useful for advanced input transformations. The type-level checks here appear accurate.
764-792:ProcedureBuilderWithOutput.usewithout map input.
Testing the default input asunknownwhile applying an output shape is consistent with the design. The sync betweenInferSchemaInput<TOutputSchema>andMiddlewareOutputFnis verified.
800-814: HandlingTInContextwith output-specific builder.
Ensuring a separate context can be introduced (e.g.,{ cacheable?: boolean }) maintains consistent merges at compile time. Good coverage of potential error scenarios.
940-980: Type safety for.useinProcedureBuilderWithInputOutput(no map input).
These lines confirm the context merges with both input and output schemas present. The negative tests keep everything safe.
1030-1054:TInContextinProcedureBuilderWithInputOutput.
Excellent update—flexible input contexts with thorough negative tests. Mapping is handled carefully to avoid incorrect input usage.
1140-1173:RouterBuilder.usetests without map input.
Merging the new partial context fields intoCurrentContextand confirming negative scenarios with'invalid'keys ensures routers respect the same style as procedures.
1176-1186:RouterBuilder.usewith customTInContext.
The refined approach for dynamic context merges is consistent with the other builder variants. Great to see the thorough coverage.
1206-1230: Enhanced router usage withinRouterBuilder.
These changes show the final step in merging contexts when loading external routers—both immediate (.router()) and deferred (.lazy()). Type checks align well with updated generics.packages/server/src/builder.ts (5)
3-3: Refined import for context merging.
Switching toContextExtendsGuard,MergedCurrentContext, andMergedInitialContextensures more granular control when checking context compatibility.
125-163: Overhauled.useAPI for flexible input and output contexts.
The new<UInContext extends Context = TCurrentContext>paradigm is a clear improvement, letting developers specify narrower or broader incoming contexts without losing strong typing. Negative path checks are handled byContextExtendsGuard.
145-163: Mapped.usemethod variant.
Providing the secondarymapInputparameter fosters advanced transformations while preserving the core context-based checks. This significantly enhances middleware extensibility.
243-243: Refined router integration return type.
Ensuring thatenhanceRouterreturnsEnhancedRouter<U, TInitialContext, TCurrentContext, TErrorMap>cements correct routing context merges as soon as the router is attached.
249-250: Improved lazy-loading for routers.
The new type signature enforces concurrency in context definitions even for asynchronously loaded routers. Excellent guard against mismatched contexts at runtime.packages/server/src/builder-variants.ts (8)
3-3: Importing refined context utilities.
This change alignsContextExtendsGuardandMergedContextusage throughout builder variants, ensuring all derived interfaces can share the new context merging logic.
33-51:BuilderWithMiddlewares::usenow supportsUInContext.
The additional generic clarifies where a middleware’s input context can differ from the builder’s current context, which improves type safety for advanced designs.
107-124:ProcedureBuilder::usewith the updated generics.
This mirrors the approach inBuilderWithMiddlewares, ensuring procedure-level usage can also handle distinct input contexts. The negative tests likely catch incorrect merges.
160-178:ProcedureBuilderWithInput::usegains improved context merging flows.
Explicitly factoring outInferSchemaOutput<TInputSchema>for the middleware’s input type is a neat addition. The twin call style (with or withoutmapInput) is flexible yet safe.Also applies to: 180-198
238-255:ProcedureBuilderWithOutput::use—output shapes validated.
By referencingInferSchemaInput<TOutputSchema>, these lines preserve the full chain of transformations. This ensures that a builder calls.use()with the correct output type.
288-305:ProcedureBuilderWithInputOutput::usemaintains input-output correctness.
The generics forInferSchemaOutput<TInputSchema>andInferSchemaInput<TOutputSchema>confirm that both the request and response shapes remain strongly typed. Great consistency with the rest of the changes.Also applies to: 307-325
351-367:RouterBuilder::use—refined for merged contexts.
Shifting from a singleTCurrentContextto a parameterizedUInContextfosters the same advanced behavior as in procedure builders, unifying the entire codebase’s approach to typed contexts.
378-379: Lazy router signature broadening.
These lines confirm that lazy-loading a router still respects the newly introducedTCurrentContextmerges, matching the immediate.router()method.
More templates
@orpc/client
@orpc/contract
@orpc/openapi
@orpc/openapi-client
@orpc/react-query
@orpc/shared
@orpc/server
@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: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/server/src/context.test-d.ts (1)
14-16: Consider using a type alias instead of an empty interfaceThe
Emptyinterface is equivalent to{}. Using a type alias would be more appropriate here.-interface Empty { - -} +type Empty = {};🧰 Tools
🪛 Biome (1.9.4)
[error] 13-16: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/server/src/builder-variants.ts(8 hunks)packages/server/src/builder.ts(3 hunks)packages/server/src/context.test-d.ts(1 hunks)packages/server/src/context.ts(1 hunks)packages/server/src/implementer-procedure.ts(3 hunks)packages/server/src/implementer-variants.ts(2 hunks)packages/server/src/implementer.ts(2 hunks)packages/server/src/middleware-decorated.ts(2 hunks)packages/server/src/procedure-decorated.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/server/src/implementer-variants.ts
- packages/server/src/builder-variants.ts
- packages/server/src/middleware-decorated.ts
- packages/server/src/builder.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/server/src/context.test-d.ts
[error] 13-16: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🔇 Additional comments (20)
packages/server/src/context.test-d.ts (4)
1-1: Properly updated imports to match new context type systemThe import statement has been correctly updated to import the new context-related types that replace the old ones:
ContextExtendsGuard,MergedCurrentContext, andMergedInitialContext.
3-7: LGTM: Comprehensive test coverage for MergedInitialContextThis test correctly verifies the type merging behavior for the new
MergedInitialContexttype, including how it handles property merging and conflicts between the different context types.
9-12: LGTM: Proper validation of MergedCurrentContextThe test correctly validates both the successful merging of properties and the conflict resolution behavior where properties in the second context override those in the first context.
18-31: LGTM: Thorough test coverage for ContextExtendsGuardThe test covers a comprehensive set of scenarios for the new
ContextExtendsGuardtype, including:
- Successfully extending when the first type has all properties of the second type
- Successfully extending when types are identical
- Successfully extending with empty types and optional properties
- Correctly failing when extending in the wrong direction or with incompatible types
This robust test coverage ensures the type guard behaves as expected in all cases.
packages/server/src/implementer.ts (4)
3-3: Properly updated imports to match new context type systemThe import statement has been correctly updated to reflect the new context types:
ContextExtendsGuard,MergedCurrentContext, andMergedInitialContext.
35-49: Added input/output context separation in middleware's use methodThis change is crucial for the PR objective. The middleware
usemethod has been updated to properly reflect the middleware's context types:
Two generic type parameters are now used:
UOutContextfor the output contextUInContextfor the input context (defaulting to current context)The return type now properly enforces that:
- The current context extends the middleware's input context
- The middleware's output context is properly merged with initial context
This fixes the bug where a procedure wasn't validating that the context passed to the middleware matches the expected type.
52-52: Added missing type parameter to EnhancedRouterThe return type for the
routermethod has been properly updated to include the required fourth type parameter (TCurrentContext) forEnhancedRouter, fixing the type error mentioned in the past review comments.
56-56: Added missing type parameter to EnhancedRouterThe return type for the
lazymethod has been properly updated to include the required fourth type parameter (TCurrentContext) forEnhancedRouter, fixing the type error mentioned in the past review comments.packages/server/src/context.ts (4)
3-8: Improved context merging with MergedInitialContextThis new type properly addresses the PR objective by introducing a more sophisticated context merging strategy:
TInitial: The base initial contextTAdditional: The middleware's input context typeTCurrent: The current context typeThe implementation correctly merges the initial context with the additional context, but excludes properties from the additional context that are already present in the current context. This ensures that middleware's initial context is properly reflected in the procedure context.
9-9: LGTM: Added MergedCurrentContext for output context handlingThe new
MergedCurrentContexttype correctly implements the merging strategy for the current context and middleware's output context, where properties from the output context override matching properties in the current context.
11-16: Renamed and updated mergeContext to reflect new type systemThe function has been properly renamed from
mergeContexttomergeCurrentContextto match its specific purpose, and its return type has been updated to use the newMergedCurrentContexttype.
18-18: Improved type safety with ContextExtendsGuardThe new
ContextExtendsGuardtype replaces the previousConflictContextGuardwith a more precise type check. It validates that the first context type extends the second context type, returningunknownif valid andneverif invalid. This ensures type safety when combining contexts in middleware.packages/server/src/procedure-decorated.ts (3)
4-4: Properly updated imports to match new context type systemThe import statement has been correctly updated to import the new context types:
ContextExtendsGuard,MergedCurrentContext, andMergedInitialContext.
56-74: Added input/output context separation in decorated procedureThis change is crucial for the PR objective. The decorated procedure's
usemethod now:
- Takes separate generic type parameters for input and output contexts
- Uses
ContextExtendsGuardto ensure type safety for both:
- Validating that the current context extends the middleware's input context
- Validating that the middleware's output context extends the current context
- Properly merges initial and current contexts using the new type system
This implementation ensures that middleware context types are correctly reflected in the procedure, addressing the core issue in the PR.
76-95: LGTM: Consistently updated the overloaded use methodThe second overload of the
usemethod (with input mapping) has been consistently updated to match the changes in the first overload, ensuring type safety and proper context merging for this variant as well.packages/server/src/implementer-procedure.ts (5)
5-5: Import additions align with improved context type handlingThe addition of
ContextExtendsGuard,MergedCurrentContext, andMergedInitialContexttypes provides the necessary tools for the stricter context type validation being implemented in this PR.
22-40: Improved type safety for middleware context handlingThis change properly reflects the middleware's initial context type in the procedure by:
- Separating input (
UInContext) and output (UOutContext) context types- Using
ContextExtendsGuardto ensure context compatibility- Using
MergedInitialContextto propagate middleware context requirements to the procedureThis addresses the core issue where procedures previously ignored the middleware's initial context type requirements.
42-61: Consistent type safety improvements for input-mapping middlewareSimilar to the previous overload, these changes correctly implement the middleware context type validation for the input-mapping version of the
usemethod. The implementation maintains consistency with the other overload and properly handles context type validation.
111-128: Consistent implementation in ProcedureImplementer interfaceThe changes to the
ProcedureImplementerinterface mirror those in theImplementedProcedureinterface, ensuring consistent type safety across the builder pattern. The context type validation is properly implemented to reflect middleware's context requirements.
130-148: Input-mapping middleware consistently updated in ProcedureImplementerThe second overload of the
usemethod inProcedureImplementerhas been correctly updated to match the pattern established in the other changes. This completes the implementation of stricter context type validation throughout the procedure builder system.
1 participant
Bug Description:
The procedure ignores the middleware's initial context type. It only checks that the current context meets the expected type when calling
.use. As a result, you can accidentally pass extra or invalid context values that the middleware doesn't expect, potentially causing it to break.Example:
In this example,
proceduredoes not reflect the initial context type{ retryable?: boolean }. It only verifies that the current context satisfies the{ retryable?: boolean }.Now, the middleware
midcan break when it encounters unexpected context values.This PR also removes the unnecessary current context check after
.use, as we had already removed thededupe-middlewarelogic earlier. Therefore, it is safe to remove this check.Summary by CodeRabbit
Refactor
Tests