refactor(contract, server): improve lazy, naming, hidden, ...#214
refactor(contract, server): improve lazy, naming, hidden, ...#214
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request refactors several TypeScript modules by renaming and replacing types and functions. Major changes include renaming “Adapted…” entities to “Enhanced…” and “adapt…” functions to “enhance…” across contract, server, and router modules. The revisions enforce immutability (using readonly arrays) and update function signatures—often by adding default empty objects for options. Several test files and helper modules were removed, and the public API of some modules was restructured with new exports. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Builder
participant RouterUtils
Client->>Builder: Invoke router() / lazy() methods
Builder->>RouterUtils: Call enhanceRouter(router, options)
RouterUtils-->>Builder: Return EnhancedRouter instance
Builder-->>Client: Provide EnhancedRouter for further operations
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 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ 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 (
|
More templates
@orpc/client
@orpc/contract
@orpc/openapi
@orpc/openapi-client
@orpc/react-query
@orpc/server
@orpc/shared
@orpc/standard-server
@orpc/standard-server-node
@orpc/standard-server-fetch
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
dinwwwh
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/server/src/router-hidden.ts (1)
19-21: Consider adding type safety for the contract accessThe function uses a type assertion
(router as any)which bypasses TypeScript's type checking. While this may be necessary for the symbol access pattern, consider if there's a more type-safe approach.export function getHiddenRouterContract(router: Lazyable<AnyRouter | AnyContractRouter>): AnyContractRouter | undefined { - return (router as any)[HIDDEN_ROUTER_CONTRACT_SYMBOL] + // Use a more type-safe approach that still allows symbol access + return Reflect.get(router, HIDDEN_ROUTER_CONTRACT_SYMBOL) as AnyContractRouter | undefined }packages/contract/src/router-utils.ts (1)
17-38: Refine enumerability and type safety.
Consider usingObject.keys(...)to avoid enumerating inherited properties if that behavior is unwanted. Additionally, removing theas anycast might preserve stronger type safety if feasible.packages/server/src/procedure-utils.ts (1)
11-27: Lazy procedure assertion is well-structured.
The error message is descriptive, though you might consider tailoring it further if end users are likely to see it. Otherwise, this provides a safe fallback if the unwrapped value is not a procedure.packages/server/src/builder.test.ts (1)
223-226: Consider adding negative test scenarios for rejected promises.
The.resolvesassertion is correct for success cases; adding a test for promise rejection would enhance coverage.packages/openapi/src/adapters/standard/openapi-matcher.ts (1)
16-16: Consider extracting the inline type into a named interface.
Currently, the inlined typeLazyTraverseContractProceduresOptions & { httpPathPrefix: HTTPPath; laziedPrefix: string | undefined }might obscure readability. Defining a named interface or type alias for this structure would improve clarity.packages/server/src/router-utils.ts (2)
74-97:EnhancedRoutertype is comprehensive.
The generics are quite extensive. Adding explanatory comments or documentation near this definition could help future contributors understand the layered types.
151-207:traverseContractProceduresiteration.
Usingfor (const key in currentRouter)may iterate inherited properties. PreferObject.keys(currentRouter)if inheritance is not intended. This can reduce potential edge cases.packages/server/src/implementer.ts (1)
107-108: Consider refactoring the repeated logic in this block.
The code within therouterbranch is nearly identical to thelazybranch (lines 119-120). Extracting a helper function would reduce duplication and improve maintainability.+function enhanceAndSetHidden( + router: AnyRouter, + contract: AnyContractRouter, + middlewares: AnyMiddleware[], +) { + const adapted = enhanceRouter(router, { + middlewares, + errorMap: {}, + prefix: undefined, + tags: undefined, + }) + return setHiddenRouterContract(adapted, contract) +} ... else if (key === 'router') { - const adapted = enhanceRouter(router, { - middlewares, - errorMap: {}, - prefix: undefined, - tags: undefined, - }) - return setHiddenRouterContract(adapted, contract) + return enhanceAndSetHidden(router, contract, middlewares) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (80)
apps/content/docs/client/event-iterator.md(1 hunks)packages/contract/src/builder-variants.test-d.ts(2 hunks)packages/contract/src/builder-variants.ts(3 hunks)packages/contract/src/builder.test-d.ts(2 hunks)packages/contract/src/builder.test.ts(2 hunks)packages/contract/src/builder.ts(2 hunks)packages/contract/src/index.ts(1 hunks)packages/contract/src/route.test.ts(3 hunks)packages/contract/src/route.ts(1 hunks)packages/contract/src/router-utils.test-d.ts(1 hunks)packages/contract/src/router-utils.test.ts(1 hunks)packages/contract/src/router-utils.ts(1 hunks)packages/contract/src/router.test-d.ts(2 hunks)packages/contract/src/router.test.ts(0 hunks)packages/contract/src/router.ts(2 hunks)packages/openapi/src/adapters/fetch/openapi-handler.ts(1 hunks)packages/openapi/src/adapters/node/openapi-handler.ts(1 hunks)packages/openapi/src/adapters/standard/openapi-matcher.ts(4 hunks)packages/openapi/src/openapi-generator.ts(2 hunks)packages/server/src/adapters/fetch/rpc-handler.ts(1 hunks)packages/server/src/adapters/node/rpc-handler.ts(1 hunks)packages/server/src/adapters/standard/handler.ts(3 hunks)packages/server/src/adapters/standard/rpc-matcher.ts(4 hunks)packages/server/src/adapters/standard/types.ts(1 hunks)packages/server/src/builder-variants.test-d.ts(17 hunks)packages/server/src/builder-variants.ts(4 hunks)packages/server/src/builder.test-d.ts(7 hunks)packages/server/src/builder.test.ts(2 hunks)packages/server/src/builder.ts(3 hunks)packages/server/src/hidden.test.ts(0 hunks)packages/server/src/hidden.ts(0 hunks)packages/server/src/implementer-procedure.test-d.ts(5 hunks)packages/server/src/implementer-variants.test-d.ts(4 hunks)packages/server/src/implementer-variants.ts(2 hunks)packages/server/src/implementer.test-d.ts(5 hunks)packages/server/src/implementer.test.ts(3 hunks)packages/server/src/implementer.ts(3 hunks)packages/server/src/index.ts(1 hunks)packages/server/src/lazy-utils.test-d.ts(0 hunks)packages/server/src/lazy-utils.test.ts(0 hunks)packages/server/src/lazy-utils.ts(0 hunks)packages/server/src/lazy.test.ts(1 hunks)packages/server/src/lazy.ts(1 hunks)packages/server/src/middleware-utils.ts(2 hunks)packages/server/src/middleware.test-d.ts(1 hunks)packages/server/src/middleware.ts(1 hunks)packages/server/src/plugins/base.ts(1 hunks)packages/server/src/plugins/cors.ts(2 hunks)packages/server/src/plugins/response-headers.ts(1 hunks)packages/server/src/procedure-client.test-d.ts(1 hunks)packages/server/src/procedure-client.ts(2 hunks)packages/server/src/procedure-decorated.test-d.ts(2 hunks)packages/server/src/procedure-utils.test.ts(1 hunks)packages/server/src/procedure-utils.ts(1 hunks)packages/server/src/procedure.test.ts(1 hunks)packages/server/src/procedure.ts(1 hunks)packages/server/src/router-accessible-lazy.test-d.ts(0 hunks)packages/server/src/router-accessible-lazy.test.ts(0 hunks)packages/server/src/router-accessible-lazy.ts(0 hunks)packages/server/src/router-client.ts(3 hunks)packages/server/src/router-hidden.test.ts(1 hunks)packages/server/src/router-hidden.ts(1 hunks)packages/server/src/router-utils.test-d.ts(1 hunks)packages/server/src/router-utils.test.ts(1 hunks)packages/server/src/router-utils.ts(1 hunks)packages/server/src/router.test-d.ts(2 hunks)packages/server/src/router.test.ts(0 hunks)packages/server/src/router.ts(1 hunks)packages/server/src/utils.test.ts(1 hunks)packages/server/src/utils.ts(1 hunks)packages/server/tests/shared.ts(1 hunks)packages/standard-server-fetch/playground/event-source.ts(1 hunks)packages/standard-server-fetch/src/body.ts(1 hunks)packages/standard-server-fetch/src/event-iterator.ts(1 hunks)packages/standard-server-fetch/src/request.ts(1 hunks)packages/standard-server-fetch/src/response.ts(1 hunks)packages/standard-server-node/src/body.ts(1 hunks)packages/standard-server-node/src/event-iterator.ts(1 hunks)packages/standard-server-node/src/response.ts(1 hunks)packages/zod/src/coercer.ts(2 hunks)
💤 Files with no reviewable changes (10)
- packages/server/src/router-accessible-lazy.test-d.ts
- packages/server/src/router-accessible-lazy.test.ts
- packages/server/src/lazy-utils.test-d.ts
- packages/server/src/hidden.ts
- packages/server/src/lazy-utils.ts
- packages/server/src/router-accessible-lazy.ts
- packages/server/src/hidden.test.ts
- packages/contract/src/router.test.ts
- packages/server/src/router.test.ts
- packages/server/src/lazy-utils.test.ts
🔇 Additional comments (197)
packages/openapi/src/adapters/fetch/openapi-handler.ts (1)
14-14: Confirm the reversed type parameters for the router.Switching from
Router<T, any>toRouter<any, T>in the constructor may invert how context is inferred and used. Verify that this change aligns properly with other parts of the codebase (e.g., downstream references, tests, and usage patterns).packages/standard-server-node/src/response.ts (1)
11-11: Adding a default value for options is beneficial.Providing a default
{}for theoptionsparameter makes the function more convenient to call and reduces boilerplate. This looks good!packages/contract/src/route.ts (3)
104-107: Renaming the route options interface aligns with the new enhancement approach.Replacing
AdaptRouteOptionswithEnhanceRouteOptionsreflects the larger shift from "adapt" to "enhance" in the codebase. Make sure all references are updated accordingly.
109-109: Function renamed to enhanceRoute.The rename from
adaptRoutetoenhanceRouteis consistent with the new naming convention. No immediate concerns here.
116-116: Use ofoptions.tags?.lengthimproves clarity.Checking
options.tags?.lengthensures tags are only appended if defined and non-empty, preventing accidental handling of undefined or empty arrays.packages/server/src/router.ts (9)
1-1: Refined imports from '@orpc/contract'.The trimmed imports make the file more focused. Confirm that no required types/functions are inadvertently removed.
3-4: Lazyable import introduced.You’re importing
Lazyablefrom./lazy. Verify all references to the old lazy imports are replaced, and confirm consistency with the simplified router structure.
6-7: Router type logic updated.Now the router logic is split between a procedure case and an object case. Validate that any shared code or calls to
Router<T>are updated accordingly throughout the project.
10-10: Index signature for sub-routers.The new index signature infers sub-router types if
T[K]extendsAnyContractRouter. Check for potential edge cases ifTcontains non-router fields.
15-15: Inferred initial context.This specialized type logic helps unify how contexts are derived. Ensure no existing references rely on a previous approach to context inference.
20-24: Refactored InferRouterInitialContexts.Switched to a clearer pattern that depends on
Procedure<infer UInitialContext>. Please confirm usage in deeply nested or lazy-based routers remains correct.
27-31: Refactored InferRouterCurrentContexts.Similarly, the current context is now pulled from
Procedure<any, infer UCurrentContext>. Verify multi-layer or chained contexts still resolve as intended.
34-38: Refactored InferRouterInputs.Taking
SchemaInput<UInputSchema>from a procedure is more straightforward. If older code expected a different shape (especially with lazy usage), ensure it’s been updated.
40-46: Refactored InferRouterOutputs.Leveraging
SchemaOutput<UOutputSchema, UFuncOutput>clarifies the final structure for outputs. Confirm that test coverage is updated to reflect these new inferences.packages/contract/src/router.test-d.ts (3)
3-4: LGTM: Updated type importsThe import changes appropriately reflect the shift from "Adapted..." types to "Enhanced..." types as mentioned in the PR summary, focusing on the necessary type imports for the updated tests.
7-11: LGTM: Simplified test structureThe test suite has been streamlined by consolidating the "meta" tests into a single more focused test case, which improves readability while still effectively testing the type compatibility.
43-45: LGTM: New type inference testThis new test appropriately validates the type inference for the router meta, ensuring it matches the expected
BaseMeta | Metatype.packages/contract/src/index.ts (1)
14-14: LGTM: Added router-utils exportThe addition of the router-utils export aligns with the PR objectives of refactoring and improving naming conventions. This makes the enhanced router functionality available through the main package entry point.
apps/content/docs/client/event-iterator.md (1)
74-74: LGTM: Simplified section headerThe simplified section header "Connection Status" improves readability while still clearly indicating the purpose of this section, which focuses on tracking the connection status of event iterators.
packages/server/src/middleware.ts (1)
33-33: LGTM: Enhanced type safety with readonly pathChanging the
pathproperty type fromstring[]toreadonly string[]improves type safety by enforcing immutability of the path array. This prevents accidental mutations and aligns with similar changes across the codebase.packages/standard-server-fetch/playground/event-source.ts (1)
38-42: Simplified function call by removing unnecessary parameterThe empty object parameter has been removed from the
toFetchResponsecall, which aligns with updates made elsewhere in the codebase. ThetoFetchResponsefunction now has a default empty object parameter in its definition, making this change both cleaner and more consistent.packages/server/src/lazy.test.ts (4)
2-2: LGTM: Updated imports to include new getLazyMeta functionThe import statement has been correctly updated to include the new
getLazyMetafunction.
4-5: Good enhancement to test coverageThe test description has been updated to include the new
getLazyMetafunction, and thelazyfunction is now called with metadata ({ prefix: '/adapt' }), which allows for testing the new metadata functionality.
8-8: LGTM: Added test for new metadata functionalityThis assertion validates that the new
getLazyMetafunction correctly retrieves the metadata object passed to thelazyfunction.
12-12: Expanded test coverage for unlazy functionThis new assertion tests the behavior of
unlazywhen called with a primitive value, ensuring it gets properly wrapped in a default object.packages/server/src/procedure-client.test-d.ts (1)
106-106: Enhanced type safety with readonly arrayThe type assertion has been improved to expect
readonly string[]for thepathparameter, enforcing immutability. This is consistent with similar changes made across other files in the codebase and helps prevent accidental mutations of the path array.packages/server/src/router-hidden.test.ts (1)
1-9: LGTM: Good test coverage for new hidden router contract functionalityThis new test file provides adequate coverage for the hidden router contract functionality. It verifies both setting and retrieving the hidden contract, ensuring that the original router is returned by
setHiddenRouterContractand that the original contract can be retrieved usinggetHiddenRouterContract.packages/server/src/middleware.test-d.ts (1)
20-20: Improved type safety with readonly arrayThe change from
string[]toreadonly string[]enhances type safety by ensuring the path array cannot be modified within middleware functions. This aligns with the immutability pattern being applied consistently across the codebase.packages/server/tests/shared.ts (1)
58-58: Clean re-exports with descriptive aliasesThis new export statement re-exports entities from the contract test shared module with more descriptive aliases that clarify their origin. The renaming follows the pattern mentioned in the PR summary where components are being renamed for better clarity.
packages/server/src/procedure.ts (1)
16-16: Enhanced immutability with readonly path arrayChanging
path: string[]topath: readonly string[]prevents procedure handlers from accidentally modifying the path array. This is a good practice for ensuring immutability of input parameters and maintaining predictable behavior.packages/server/src/adapters/standard/types.ts (1)
10-10: Consistent immutability pattern for path arraysThe change to
readonly string[]aligns with the same pattern applied to path arrays throughout the codebase. This consistency improves type safety and prevents unintended modifications to path data.packages/standard-server-fetch/src/response.ts (1)
11-11: Good improvement to the API ergonomicsMaking the
optionsparameter optional with a default empty object is a good practice that improves the API's usability by allowing the function to be called without explicitly passing options when the defaults are sufficient.packages/standard-server-node/src/body.ts (1)
62-62: Good improvement to the API ergonomicsAdding a default empty object for the
optionsparameter makes the function more flexible and easier to use. This change is consistent with similar improvements in other parts of the codebase and follows good API design practices.packages/contract/src/route.test.ts (2)
1-1: Consistent renaming from "adapt" to "enhance"The import and test name have been updated to use
enhanceRouteinstead ofadaptRoute, which aligns with the broader refactoring effort to improve naming conventions throughout the codebase.Also applies to: 32-32
39-46: Improved test clarity with explicit undefined parametersThe test cases now explicitly specify
undefinedfor optional parameters rather than omitting them. This improves test readability by making it clear which parameters are intentionally not provided versus those that might have been accidentally omitted.Also applies to: 48-55, 57-64, 66-69
packages/server/src/adapters/fetch/rpc-handler.ts (1)
14-14:Details
✅ Verification successful
Type parameter order change and optional options parameter
Two changes have been made to this constructor:
- The type parameters for
Routerhave been swapped fromRouter<T, any>toRouter<any, T>, reflecting a change in the underlying Router type definition- The
optionsparameter is now optional with a default empty objectThis change aligns with the broader refactoring effort and improves the API's usability.
🏁 Script executed:
#!/bin/bash # Verify that the Router type parameters have been consistently updated echo "Checking Router type definition..." grep -n "export type Router" packages/server/src/router.tsLength of output: 224
Action: Approve Constructor and Router Changes
The constructor update in
packages/server/src/adapters/fetch/rpc-handler.tsnow accepts aRouterwith type parameters in the new order (Router<any, T>) and uses an optionaloptionsparameter (defaulting to an empty object). The router definition inpackages/server/src/router.ts—export type Router<T extends AnyContractRouter, TInitialContext extends Context> =—confirms that these changes are consistent with the updated type parameter order. No further adjustments are necessary.
packages/server/src/procedure.test.ts (1)
1-9: Good addition of contract procedure validationThis test suite addition nicely complements the existing test by verifying that
pingnot only satisfies theisProcedurepredicate but also theisContractProcedurepredicate from the contract package. This ensures proper type compatibility in the new enhancement-based approach.packages/server/src/adapters/node/rpc-handler.ts (1)
14-14:Details
✅ Verification successful
Modified router type parameter order
The router parameter type has changed from
Router<T, any>toRouter<any, T>, swapping the order of generic parameters. This aligns with the broader refactoring effort that changes "Adapted" entities to "Enhanced" entities throughout the codebase.
🏁 Script executed:
#!/bin/bash # Check for consistent Router type parameter usage across the codebase echo "Checking for consistent Router type parameter usage..." rg "Router<any, \w+>" --type tsLength of output: 975
Router Generic Parameters Order Verified
The change from
Router<T, any>toRouter<any, T>inpackages/server/src/adapters/node/rpc-handler.tsaligns with the overall refactoring effort. Our repository search confirms that this new generic parameter order is consistently applied across multiple modules (such as in both the OpenAPI and RPC handlers), ensuring uniformity throughout the codebase.packages/contract/src/router-utils.test-d.ts (1)
1-46: Well-structured type tests for EnhancedContractRouterThis new test file provides thorough type checking for the
EnhancedContractRouter, validating both root and nested procedure types. The tests ensure that the enhanced router correctly preserves the expected types for input/output schemas, error maps, and metadata across different procedures.The comprehensive type assertions verify the type safety of the refactored router enhancement functionality, which is crucial for this PR's transition from "Adapted..." to "Enhanced..." patterns.
packages/openapi/src/adapters/node/openapi-handler.ts (1)
14-14: Modified router type parameter orderThe router parameter type has changed from
Router<T, any>toRouter<any, T>, consistent with the changes in other handler classes. This change is part of the broader refactoring to standardize the router type parameter ordering across the codebase.packages/standard-server-fetch/src/request.ts (1)
27-27: Good improvement to function signature flexibilityMaking the
optionsparameter optional with a default empty object value improves the API by allowing callers to omit the parameter when default options are sufficient.packages/server/src/procedure-decorated.test-d.ts (1)
103-103: Improved type safety with readonly arraysChanging the type expectation from
string[]toreadonly string[]enforces immutability on the path parameter, which is a good practice to prevent accidental modifications of the array.Also applies to: 140-140
packages/server/src/index.ts (1)
17-18: Module restructuring for better organizationAdding new exports for router-related utilities is part of the broader refactoring effort to improve code organization and naming conventions. These new exports likely replace the previous ones mentioned in the summary (such as
hidden,lazy-utils, androuter-accessible-lazy).packages/server/src/implementer-procedure.test-d.ts (1)
73-73: Consistent enforcement of immutabilityThe type changes from
string[]toreadonly string[]across multiple test cases ensure that path parameters are consistently treated as immutable throughout the codebase, reinforcing type safety.Also applies to: 110-110, 223-223, 263-263, 314-314
packages/standard-server-fetch/src/body.ts (1)
64-64: LGTM: Making options parameter optional improves API flexibilityAdding a default empty object for the
optionsparameter is a good practice that simplifies the API usage. This change allows callers to omit the options parameter when default behavior is sufficient.packages/contract/src/router-utils.test.ts (1)
1-25: Well-structured test forenhanceContractRouterThis test effectively validates that the enhancement process:
- Properly merges error maps
- Enhances routes with the provided options
- Maintains the nested structure of contract routers
The test coverage is thorough, checking both top-level and nested routes.
packages/contract/src/builder.test.ts (2)
5-5: LGTM: Updated import and spy to use new module naming conventionThe change from
RoutertoRouterUtilsModuleand the corresponding spy update properly reflect the refactoring from "adapt" to "enhance" naming convention.Also applies to: 7-7
129-131: LGTM: Updated test assertions to match new function nameThese assertion changes correctly verify that the new
enhanceContractRouterfunction is called with the expected parameters.packages/standard-server-node/src/event-iterator.ts (1)
96-96: LGTM: Adding default value for options parameter improves APIMaking the
optionsparameter optional with a default empty object is a good change that simplifies API usage without changing functionality. The function already handles missing options properties with null coalescing operators (lines 98-100).packages/server/src/router-hidden.ts (3)
1-4: Clean imports with good type definitionsThe imports are well-organized and correctly specify the types needed for the router contract functionality.
5-5: Good use of Symbol for private implementation detailUsing a Symbol for the hidden contract property ensures it won't conflict with other router properties and won't be enumerable in standard property iterations.
7-17: Well-implemented proxy pattern for contract hidingThe
setHiddenRouterContractfunction uses the Proxy pattern effectively to intercept property access. The implementation is clean and maintains the original object's behavior for all properties except the special symbol key.packages/standard-server-fetch/src/event-iterator.ts (1)
95-95: Good improvement to function usabilityAdding a default empty object for the options parameter is a good improvement. This makes the function more user-friendly by allowing it to be called without explicitly passing options, while maintaining the same functionality since the code already handles defaults for individual options.
packages/server/src/utils.test.ts (2)
1-1: Consistent naming with implementationUpdating the import from
convertPathToHttpPathtotoHttpPathmaintains consistency with the implementation changes and improves the naming convention.
4-6: Test updated correctly for renamed functionThe test cases have been properly updated to use the
toHttpPathfunction instead ofconvertPathToHttpPathwhile maintaining the same test coverage and assertions.packages/zod/src/coercer.ts (2)
2-2: Updated import for renamed interfaceThe import has been correctly updated to use
HandlerPlugininstead ofPlugin, consistent with the broader renaming in the codebase.
32-32: Interface implementation updated correctlyThe class implementation has been properly updated to implement
HandlerPlugin<TContext>instead ofPlugin<TContext>, maintaining the same functionality while using the new interface name.packages/server/src/router-utils.test-d.ts (2)
1-17: Well-structured test for AccessibleLazyRouter type checkingThe test for
AccessibleLazyRouterproperly verifies that both direct and nested methods are correctly typed as lazy versions of their respective procedures. This ensures type safety when accessing methods through theAccessibleLazyRoutertype.
19-76: Comprehensive type checking for EnhancedRouterThe test for
EnhancedRouterthoroughly validates the complex type structure, including:
- Proper context type inheritance (InitialContext → CurrentContext)
- Correct schema propagation for input and output
- Error map merging (custom TErrorMap with baseErrorMap)
- Metadata inheritance
- Proper handling of nested lazy procedures
This ensures type safety across all router operations, which is essential for the refactoring from "Adapted" to "Enhanced" router patterns.
packages/server/src/plugins/cors.ts (1)
4-4: Interface rename from Plugin to HandlerPluginThis change reflects a broader refactoring in the codebase to rename the
Plugininterface toHandlerPlugin, making the naming more explicit about its purpose. The functionality remains the same, only the interface name has changed.Also applies to: 17-17
packages/contract/src/builder-variants.ts (2)
5-6: Updated imports to use EnhancedContractRouterThe imports have been updated to use the new
EnhancedContractRouterandEnhanceContractRouterOptionstypes from the router-utils module, which aligns with the PR objective of renaming from "Adapted" to "Enhanced" patterns.
105-105: Changed router options and return types to Enhanced versionsThe builder's
~orpcproperty and the return type of theroutermethod have been updated to use the Enhanced versions instead of Adapted versions, completing the transition to the new naming scheme.Also applies to: 115-115
packages/contract/src/builder.test-d.ts (1)
6-6: Updated type assertions for EnhancedContractRouterThe type expectations have been properly updated to verify against
EnhancedContractRouterinstead ofAdaptedContractRouter, maintaining type safety during the refactoring process.Also applies to: 137-139
packages/server/src/plugins/base.ts (2)
4-4: Interface renamed fromPlugintoHandlerPlugin.The interface has been renamed from
PlugintoHandlerPlugin, which better reflects its specific purpose in the handler context. This naming change improves code clarity and makes the API more self-explanatory.
8-9: CompositePlugin implementation updated to use HandlerPlugin.The CompositePlugin class has been properly updated to implement the new HandlerPlugin interface and use it for the plugins array type. This change maintains consistency with the renaming pattern introduced in this PR.
packages/server/src/adapters/standard/handler.ts (3)
5-5: Import updated from Plugin to HandlerPlugin.The import has been updated to use the renamed HandlerPlugin interface, maintaining consistency with the changes in the plugins base file.
25-25: StandardHandlerOptions updated to use HandlerPlugin.The plugins property type has been updated from Plugin to HandlerPlugin, ensuring consistency with the interface renaming changes.
52-52:Details
✅ Verification successful
Router type parameters swapped in constructor.
The order of type parameters for Router has been changed from
Router<T, any>toRouter<any, T>. This is a significant change that suggests the Router generic parameters have been reordered in its definition.Please confirm that this change in type parameter order has been consistently applied throughout the codebase, particularly in other handlers that use Router. Inconsistent application could lead to type errors.
🏁 Script executed:
#!/bin/bash # Check for other uses of Router type across the codebase rg 'Router<' --type ts | grep -v 'Router<any, T>' | grep -v "packages/server/src/adapters/standard/handler.ts"Length of output: 7921
Router parameter order verified and consistent throughout the codebase.
I checked other occurrences in files such aspackages/server/src/implementer.ts,packages/server/src/builder.ts, and the corresponding tests. Although the type variable names differ (for example, some useTvs.anyorTCurrentContextvs.InitialContext), all usages follow the same ordering—first parameter representing the contract router type and second representing the context type. No inconsistent usages were found.packages/server/src/plugins/response-headers.ts (2)
3-3: Import updated from Plugin to HandlerPlugin.The import has been updated to use the renamed HandlerPlugin interface, maintaining consistency with the changes in the plugins base file.
9-9: ResponseHeadersPlugin implementation updated to use HandlerPlugin.The ResponseHeadersPlugin class has been properly updated to implement the new HandlerPlugin interface, maintaining consistency with the renaming pattern introduced in this PR.
packages/server/src/procedure-utils.test.ts (3)
1-7: New imports added for testing new functionality.The imports have been expanded to include necessary dependencies for testing the newly added functions
createAssertedLazyProcedureandcreateContractedProcedure. This ensures that all required components are available for the new tests.
14-19: Good test coverage for createAssertedLazyProcedure.The test for
createAssertedLazyProcedureproperly verifies both the success case (with a valid procedure) and the error case (with an invalid procedure). The test ensures that the function correctly validates that a lazy-loaded value is a procedure.
21-45: Thorough test for createContractedProcedure.The test for
createContractedProcedureverifies that the function correctly creates a Procedure instance and preserves the required properties. The test uses a mismatch path object to ensure that property overriding works correctly while preserving the original procedure's properties.packages/contract/src/router-utils.ts (2)
6-12: Robust use of recursive TypeScript transformations.
Your approach to merging error maps using advanced TS generics is well-structured, ensuring flexibility for future extensions.
13-15: Interface extension is sensible.
TheEnhanceContractRouterOptionsinterface neatly extendsEnhanceRouteOptionsto unify router and route transformations.packages/server/src/procedure-client.ts (2)
36-36: Immutability enhancement.
Usingreadonly string[]forpathhelps prevent accidental mutations in interceptor options.
58-58: Consistent immutability for path property.
Changing the optionalpathtoreadonly string[]aligns with best practices for safer type usage.packages/server/src/procedure-utils.ts (2)
2-2: Import adjustments look consistent.
These type import changes clarify usage without bloating the bundle.Also applies to: 5-5, 7-7, 8-8
29-40: Straightforward contract merge.
createContractedProcedurecleanly merges the~orpcproperties between procedures and contracts, promoting consistency.packages/server/src/builder.test.ts (3)
11-11: Spy creation appears consistent.
Switching toenhanceRouteraligns well with the updated approach.
215-217: Tests validate the new router enhancement correctly.
Verifying the returned object and call arguments looks good.
8-8:Details
❓ Verification inconclusive
Confirm all references to the old router are removed.
The new import forrouter-utilsis aligned with the refactor. Kindly verify there are no dangling references to./router.Run the following script to confirm:
🏁 Script executed:
#!/bin/bash # Searching for any leftover references to "import * as Router from './router'" rg "import\s+\*\s+as\s+Router\s+from\s+'\.\/router'"Length of output: 212
Attention: Manual Verification Needed for Router References
The original regex in the verification script failed due to escaping issues. Please re-run the following updated script to ensure that all references to the old router (
'./router') have been removed:#!/bin/bash # Searching for any leftover references to "import * as Router from './router'" using corrected escapes rg "import\\s+\\*\\s+as\\s+Router\\s+from\\s+'\\.\\/router'"Once confirmed that this command produces no output, you'll have verified that the new
router-utilsimport is in place with no lingering references to the old router module.packages/server/src/utils.ts (1)
1-3: Immutable path signature is a welcome improvement.
Renaming and usingreadonly string[]preserves immutability and consistency with the refactoring effort.packages/openapi/src/openapi-generator.ts (3)
7-8: Import statements align with the new utilities.
Switching totoHttpPathandresolveContractProceduresis consistent with the refactor.
36-36: Refactor usesresolveContractProcedureseffectively.
ReplaceseachAllContractProcedurewith an updated approach, ensuring clarity in procedure resolution.
43-43: Path conversion function updated consistently.
The switch totoHttpPathremoves potential confusion from the oldconvertPathToHttpPathname.packages/contract/src/builder-variants.test-d.ts (2)
6-6: Type import renamed from AdaptedRouter to EnhancedRouterThis change reflects the broader refactoring effort to rename "Adapted..." entities to "Enhanced..." across the codebase, improving the naming conventions as mentioned in the PR objectives.
313-315: Return type updated to use EnhancedContractRouterThe return type for the
.routermethod has been updated fromAdaptedContractRoutertoEnhancedContractRouter, maintaining consistency with the renamed type imports.packages/contract/src/builder.ts (4)
3-4: Added import for ContractRouter typeThe explicit import of the ContractRouter type improves code clarity and maintainability.
9-9: Updated imports to use enhance-prefixed functions and typesReplaced imports for adapt-prefixed functions and types with their enhance-prefixed equivalents, maintaining consistency with the naming convention changes throughout the codebase.
16-16: Updated interface extension from AdaptRouterOptions to EnhanceContractRouterOptionsThis change ensures the BuilderDef interface uses the renamed router enhancement options, aligning with the PR's naming convention improvements.
117-119: Updated router method signature and implementationChanged method signature to return
EnhancedContractRouterinstead ofAdaptedRouterand implementation to useenhanceContractRouterinstead ofadaptRouter. This maintains functional equivalence while improving naming clarity.packages/server/src/router-utils.test.ts (5)
1-6: Well-structured imports for the new test fileThe imports are organized logically, bringing in all necessary functions and test fixtures for comprehensive testing of the router utilities.
7-20: Comprehensive tests for getRouter functionGood test coverage for the
getRouterfunction, testing various scenarios including:
- Getting the root router
- Getting an existing route
- Getting a lazy-loaded route
- Handling non-existent routes
- Edge cases with dot notation paths
This ensures the function behaves correctly in all expected use cases.
22-76: Thorough test suite for enhanceRouter functionThe tests comprehensively verify the
enhanceRouterfunctionality:
- Verifying middleware application
- Ensuring route enhancement with prefixes and tags
- Checking preservation of validation indices
- Testing lazy-loading behavior
- Verifying prefix merging in nested enhancement scenarios
This level of testing is crucial for ensuring the reliability of the router enhancement mechanism.
78-159: Complete testing of traverseContractProceduresTests cover all important use cases:
- Traversal with contract objects
- Traversal with router objects
- Traversal with hidden contracts
The verification of callback invocation with correct path arguments ensures the traversal logic works as expected.
161-190: Effective tests for resolveContractProceduresThese tests verify the asynchronous resolution of contract procedures, including:
- Calling the callback with the correct contract and path
- Proper handling of lazy-loaded contracts
- Correct resolution of nested contracts
This ensures that the contract resolution mechanism works correctly with various nesting levels and lazy loading.
packages/server/src/builder-variants.test-d.ts (4)
11-11: Updated import to use EnhancedRouterThis change maintains consistency with the renaming pattern applied across the codebase, replacing "Adapted" with "Enhanced" in type names.
75-75: Enhanced type safety with readonly string arraysChanged the type of
pathparameters fromstring[]toreadonly string[], which prevents accidental mutation of path arrays. This is a good practice for improving type safety, especially for parameters that shouldn't be modified.Also applies to: 179-179, 426-426, 542-542, 718-718, 806-806, 886-886, 926-926, 1009-1009
203-268: Added test coverage for new builder methodsNew tests for the
.prefix,.tag,.router, and.lazymethods on theBuilderWithMiddlewaresclass ensure that:
- Methods return the correct types
- Type checking properly validates input parameters
- Error cases are properly caught by TypeScript
- Integration with the enhanced router system works correctly
This significantly improves the test coverage for the builder's functionality.
1072-1073: Updated return type assertions for router and lazy methodsChanged the expected return types from
AdaptedRoutertoEnhancedRouter, maintaining consistency with the renamed router enhancement system throughout the codebase.Also applies to: 1096-1097
packages/server/src/implementer.test.ts (5)
7-8: Module imports renamed to match new architectureThese imports were updated to reflect the refactored module structure - moving from
hiddentorouter-hiddenand introducing the newrouter-utilsmodule, which aligns with the PR's objectives of improving naming conventions.
10-12: Spy functions updated to reflect renamed methodsThe spy functions have been updated to match the renamed module exports:
setHiddenRouterContractSpyreplaces previoussetRouterContractSpyenhanceRouterSpyreplaces previousadaptRouterSpyThis is consistent with the PR goal of renaming "adapt..." functions to "enhance..." across the codebase.
64-69: Test expectations updated to use renamed router functionsThe test assertions have been updated to use
getHiddenRouterContractinstead ofgetRouterContractand verify thatsetHiddenRouterContractSpywas called with the enhanced router value. This properly validates the behavior of the refactored implementation.
70-74: Router enhancement verification updatedThe test now verifies the
enhanceRouterfunction was called with the correct parameters, maintaining test coverage of the refactored implementation while using the new naming convention.
80-90: Similar updates applied to lazy router testThe same pattern of renamed function calls is consistently applied to the lazy router test, ensuring all code paths continue to be properly tested after the refactoring.
packages/server/src/router.test-d.ts (5)
1-2: Updated imports to match new type structureThe imports have been updated to:
- Remove deprecated types (
MergedErrorMap,baseErrorMap, etc.)- Add the new
ContractRoutertype- Include additional router-related type utilities
This aligns with the PR's objective of improving the type system and naming conventions.
Also applies to: 5-5
9-21: Rewritten contract router type testThe previous test has been replaced with a more focused test that verifies router compatibility with the
ContractRoutertype. This better aligns with the refactored architecture where "Enhanced..." replaces "Adapted..." entities.
23-28: Enhanced initial context type testsThe tests for initial context have been improved to:
- Check compatibility with the base
InitialContext- Check compatibility with extended contexts (adding optional properties)
- Verify incompatibility with contexts missing required properties or having incorrect types
This provides more thorough type-checking coverage.
53-59: Simplified InferRouterInputs testsThe tests for
InferRouterInputshave been simplified while maintaining full type coverage. The change improves readability by first capturing the inferred type and then testing against it, rather than repeating the inference in each expectation.
63-69: Simplified InferRouterOutputs testsSimilar to the inputs tests, the outputs tests follow the same pattern of simplification while maintaining full type coverage. This consistency in testing approach improves maintainability.
packages/server/src/lazy.ts (6)
3-3: Symbol renamed for clarityThe constant
LAZY_LOADER_SYMBOLhas been renamed toLAZY_SYMBOL, which is more concise and better reflects its purpose as a key in theLazyinterface. This change is part of the PR's goal to improve naming conventions.
9-14: Enhanced Lazy interface with metadata supportThe
Lazyinterface has been significantly improved by restructuring it to include both:
- A
loaderfunction that returns a promise- A
metaproperty to store metadataThis change supports the PR objective of improving the hidden functionality by providing a more structured way to associate metadata with lazy-loaded components.
18-25: Updated lazy function with metadata supportThe
lazyfunction now accepts an optionalmetaparameter (defaulting to an empty object) and returns an object with the enhanced structure. This change makes the lazy-loading mechanism more flexible while maintaining backward compatibility through the default parameter.
27-33: Updated isLazy checkThe
isLazyfunction has been updated to check for the presence of the renamedLAZY_SYMBOLproperty. The check remains efficient and maintains the same behavior for identifying lazy-loaded components.
35-37: Added metadata retrieval functionA new utility function
getLazyMetahas been added to safely extract metadata from lazy-loaded components. This addition supports the enhanced metadata functionality and provides a clean API for accessing the metadata.
39-41: Updated unlazy functionThe
unlazyfunction has been updated to work with the new structure, extracting the loader function from the enhanced object structure. This maintains backward compatibility while supporting the new metadata features.packages/server/src/adapters/standard/rpc-matcher.ts (5)
3-5: Updated imports to reflect refactored modulesThe imports have been updated to:
- Import
AnyRoutertype directly- Replace
EachContractProcedureLaziedOptionswithLazyTraverseContractProceduresOptions- Import utility functions from their new locations after refactoring
These changes align with the module restructuring mentioned in the PR objectives.
Also applies to: 8-10
16-16: Enforced immutability with readonly arrayThe
pathproperty has been changed from a mutable array (string[]) to an immutable array (readonly string[]). This is a good practice that prevents accidental modification of the path and aligns with TypeScript best practices for immutability.
23-23: Updated type for pendingRoutersThe type of
pendingRoutershas been updated to useLazyTraverseContractProceduresOptions, reflecting the refactored options structure used for lazy traversal. This change maintains type safety while adapting to the new API.
25-50: Refactored initialization methodThe
initmethod has been updated to:
- Accept a readonly path parameter
- Use
traverseContractProceduresinstead ofeachContractProcedure- Use the renamed
toHttpPathutility functionThese changes maintain the same functionality while using the refactored API and enforcing immutability.
76-87: Updated router access and contract procedure creationThis section has been refactored to:
- Use
getRouterinstead ofgetRouterChildfor router access- Update the error message path conversion using
toHttpPath- Reverse the parameter order in
createContractedProcedurefor better clarityThese changes maintain the same functionality while aligning with the refactored API.
packages/contract/src/router.ts (2)
27-32: Type rename for better semantic clarityThe renaming from
ContractRouterToErrorMaptoInferContractRouterErrorMapimproves clarity by using a verb that better describes what the type is doing - inferring error maps from contract routers. This aligns with TypeScript naming conventions for utility types.
34-34: Consistent naming pattern appliedRenaming
ContractRouterToMetatoInferContractRouterMetamaintains consistency with the error map type change above, creating a clear pattern for type inference utilities.packages/server/src/implementer.test-d.ts (5)
13-13: Type import updated to reflect refactoringThe import change from
AdaptedRoutertoEnhancedRouterreflects the broader renaming pattern in this PR, maintaining consistency across the codebase.
45-45: Improved type safety with readonly arraysUsing
readonly string[]instead ofstring[]for path parameters prevents accidental mutation of the path array, which is a good practice for parameters that shouldn't be modified.
95-95: Consistent immutability applied to path parametersSimilar to the previous change, this enforces immutability on path parameters in the
.usemethod, ensuring consistent type safety throughout the codebase.
126-126: Router type updated to match new naming conventionThe expected return type for the
.routermethod has been updated fromAdaptedRoutertoEnhancedRouter, maintaining consistency with the new naming pattern.
155-155: Lazy router type updated similarlyThe expected return type for the
.lazymethod has also been updated to useEnhancedRouter, ensuring consistent naming throughout the type system.packages/server/src/router-client.ts (6)
4-4: Import updated for improved type flexibilityChanging from
LazytoLazyabletype import suggests a more flexible approach to handling lazy-loaded components, likely part of the FlatLazy component removal mentioned in the PR objectives.
14-19: Simplified RouterClient type definitionThe type definition has been streamlined to directly check if
TRouterextendsProcedureand ifTRouter[K]extendsLazyable, making the code more readable and maintainable.
21-34: Function signature improved for clarityThe
createRouterClientfunction signature has been updated to useTinstead ofTRouterand accept aLazyable<T | undefined>parameter, which better reflects its usage and aligns with the type system changes.
36-36: Simplified parameter handlingThe function now directly passes
optionstocreateProcedureClientinstead of using more complex parameter handling, improving code clarity.
42-42: Updated lazy procedure creationUsing
createAssertedLazyProcedureinstead of the previous implementation aligns with the refactoring of the lazy loading mechanism mentioned in the PR objectives.
51-51: Router access method updatedChanged from
getRouterChildtogetRouter, which likely simplifies the API and makes the code more intuitive.packages/server/src/implementer-variants.ts (6)
1-1: Updated imports to match renamed typesImport statement updated to use the new
InferContractRouterErrorMapandInferContractRouterMetatypes, ensuring consistency with the changes in the contract module.
5-8: Streamlined imports for router functionalityThe imports have been updated to directly reference
Lazy,Router, andEnhancedRouter, reflecting the architectural changes in the codebase.
11-11: Simplified type parameter nameRenaming the type parameter from
TContracttoTmakes the code more concise while maintaining its functionality, which is a good practice for interfaces with multiple generic parameters.
21-23: Updated error and meta type referencesThe middleware type now uses the renamed
InferContractRouterErrorMapandInferContractRouterMetatypes, maintaining consistency with the contract module changes.
27-28: Router method signature updatedThe
routermethod now accepts aRouter<T, TCurrentContext>parameter and returns anEnhancedRoutertype, aligning with the new naming convention and parameter ordering.
30-32: Lazy loading method signature updatedThe
lazymethod has been updated to useRouter<T, TInitialContext>and returns anEnhancedRouter<Lazy<U>>type, which reflects the changes to the lazy loading mechanism mentioned in the PR objectives.packages/server/src/implementer-variants.test-d.ts (4)
12-12: Updated import aligned with renamed entityThe import has been updated from
AdaptedRoutertoEnhancedRouter, which is consistent with the PR's objective of improving naming conventions.
26-26: Enhanced type safety with readonly arrayChanged
string[]toreadonly string[]for thepathparameter, enforcing immutability which prevents accidental modifications to the path array during middleware execution.
61-62: Updated type expectation for consistencyThe type expectation has been updated from
AdaptedRoutertoEnhancedRouter, maintaining consistency with the renamed entity.
90-91: Updated type expectation for lazy routerThe type expectation for the lazy router implementation has been updated from
AdaptedRoutertoEnhancedRouter, maintaining consistency with the renamed entity.packages/server/src/builder.test-d.ts (7)
13-13: Updated import aligned with renamed entityThe import has been updated from
AdaptedRoutertoEnhancedRouter, which is consistent with the PR's objective of improving naming conventions.
124-124: Enhanced type safety with readonly arrayChanged
string[]toreadonly string[]for thepathparameter in the middleware, enforcing immutability which prevents accidental modifications to the path array.
181-181: Enhanced type safety with readonly arrayChanged
string[]toreadonly string[]for thepathparameter in the middleware with map input, enforcing immutability.
221-221: Enhanced type safety with readonly arrayChanged
string[]toreadonly string[]for thepathparameter in the middleware with map input, enforcing immutability.
336-336: Enhanced type safety with readonly arrayChanged
string[]toreadonly string[]for thepathparameter in the handler, enforcing immutability.
377-378: Updated type expectation for router methodThe type expectation for the router method has been updated from
AdaptedRoutertoEnhancedRouter, maintaining consistency with the renamed entity.
401-402: Updated type expectation for lazy methodThe type expectation for the lazy method has been updated from
AdaptedRoutertoEnhancedRouter, maintaining consistency with the renamed entity.packages/server/src/middleware-utils.ts (3)
3-3: Improved type safety with readonly array parametersThe
dedupeMiddlewaresfunction now accepts readonly arrays as parameters, preventing accidental mutation of the arrays within the function. This is a solid improvement for immutability and type safety.
19-19: Improved type safety with readonly array parametersThe
mergeMiddlewaresfunction now accepts readonly arrays as parameters, preventing accidental mutation of the arrays within the function while still returning a new array.
23-23: Improved type safety with readonly array parametersThe
addMiddlewarefunction now accepts a readonly array as its first parameter, preventing accidental mutation of the array within the function while still returning a new array with the addition.packages/server/src/builder.ts (6)
5-5: Updated import for Lazy typeUpdated import statement to use the
Lazytype directly from './lazy' rather than importing from a utility file.
9-10: Updated imports for enhanced router typesUpdated imports to use
EnhancedRouterandEnhanceRouterOptionsinstead of the previous "Adapted" naming, which aligns with the PR's naming improvements objective.
17-17: Updated import for router enhancement functionUpdated import to use
enhanceRouterinstead ofadaptRouter, which is consistent with the renaming pattern in the PR.
29-29: Updated interface extensionThe
BuilderDefinterface now extendsEnhanceRouterOptionsinstead ofAdaptRouterOptions, maintaining consistency with the renamed entities.
232-236: Updated router method implementationThe
routermethod has been updated to:
- Use the
EnhancedRouterreturn type instead ofAdaptedRouter- Call
enhanceRouterinstead ofadaptRouterThese changes align with the PR's objective to improve naming conventions and maintain consistency across the codebase.
238-242: Updated lazy method implementationThe
lazymethod has been updated to:
- Use the
EnhancedRouter<Lazy<U>, TInitialContext, TErrorMap>return type- Simplify the implementation by using
enhanceRouter(lazy(loader), this['~orpc'])directlyThis removes the dependency on
FlatLazyas mentioned in the PR description and maintains consistency with the renaming convention.packages/openapi/src/adapters/standard/openapi-matcher.ts (9)
1-1: No concerns with the new import statement.
4-4: Imports updated successfully.
18-19: No issues here.
43-43: No issues.
50-50: Verify concurrency assumptions inmatch.
If multiple requests callmatch()in parallel, the loop modifyingthis.pendingRoutersmay introduce race conditions. Confirm whether the environment guarantees single-threaded execution or consider applying synchronization.
60-60: No concerns here.
78-78: No issues.
82-82: No issues.
87-87: No issues.packages/server/src/router-utils.ts (6)
1-10: Imports appear correct and comprehensive.
11-47:getRouterimplementation looks solid.
The approach to handle both lazy and non-lazy router segments is clear and maintains good readability.
49-57:AccessibleLazyRoutertype definition is well-structured.
No issues found here.
58-72:createAccessibleLazyRouterproxy approach looks good.
This effectively handles dynamic property access on lazy objects.
99-149:enhanceRouterfunction.
The layering of middlewares, error maps, and prefix handling appears coherent. No immediate issues found.
209-227:resolveContractProceduresBFS logic is clear.
No concerns regarding the async unwrapping steps.packages/server/src/implementer.ts (14)
1-1: No issues with the updated import.
6-6: No issues with the addedLazytype import.
8-8: No concerns aboutAnyRouter, Routerimport.
15-16:setHiddenRouterContractandEnhancedRouterimports look correct.
19-19: Declaration forRouterImplementeris updated properly.
29-30: Use ofInferContractRouterErrorMap<T>is a nice improvement.
33-33: No issues with returning a decorated middleware.
40-41: Error map usage is consistent.
44-44:ImplementerInternalWithMiddlewaresreference is coherent.
46-47: ReturningEnhancedRouterfromrouteraligns with overall refactor.
49-51:lazymethod returning anEnhancedRouter<Lazy<U>>is consistent with the lazy loading pattern.
115-115: No issues noted here.
119-120: Duplicate logic identified earlier.
The same pattern appears here as in therouterbranch. Consolidating into a single helper function would maintain DRY principles.
127-127: No issues here.packages/server/src/builder-variants.ts (7)
5-5: Switching from FlattenLazy to Lazy typeThis change is part of the removal of the FlatLazy component as mentioned in the PR objectives. The simplification from
FlattenLazyto justLazylikely indicates a streamlined approach to lazy loading.
9-10: Renaming from Adapted to Enhanced naming conventionGood renaming from
AdaptedRoutertoEnhancedRouterandAdaptRouterOptionstoEnhanceRouterOptions. This improves semantics and better describes the purpose of these types, aligning with the naming improvements mentioned in the PR.
76-78: Type parameter reordering and return type updateThe type parameter ordering in the Router generic has been changed, swapping the position of
ContractRouter<TMeta>andTCurrentContext. The return type has also been updated fromAdaptedRoutertoEnhancedRouter, consistent with the new naming convention.
80-82: Updated lazy method signatureThe
lazymethod now returnsEnhancedRouter<Lazy<U>, TInitialContext, TErrorMap>instead of usingFlattenLazy<U>, which aligns with the removal of the FlatLazy component mentioned in the PR objectives.
345-345: Updated RouterBuilder options property typeThe
~orpcproperty type has been updated to useEnhanceRouterOptionsinstead ofAdaptRouterOptions, maintaining consistency with the renamed types throughout the codebase.
367-369: Consistent type parameter reordering in RouterBuilder.routerThe same type parameter reordering applied to the
BuilderWithMiddlewaresinterface has been consistently applied here to theRouterBuilderinterface, ensuring type consistency throughout the codebase.
371-373: Consistent update to lazy method in RouterBuilderThe
lazymethod inRouterBuilderhas been updated to match the changes in theBuilderWithMiddlewaresinterface, maintaining consistent typing across the codebase and supporting the removal ofFlattenLazy.
| it('InferContractRouterErrorMap', () => { | ||
| expectTypeOf < InferContractRouterErrorMap<typeof router>>().toEqualTypeOf<typeof baseErrorMap | Record<never, never>>() | ||
| }) | ||
|
|
||
| it('InferContractRouterErrorMap', () => { | ||
| expectTypeOf<InferContractRouterErrorMap<typeof router>>().toEqualTypeOf<typeof baseErrorMap | Record<never, never>>() | ||
| }) |
There was a problem hiding this comment.
Fix duplicated test case
There's a duplicate test case for InferContractRouterErrorMap - the exact same test is defined twice at lines 35-37 and again at lines 39-41. Please remove one of these duplicates to maintain clean test code.
it('InferContractRouterErrorMap', () => {
expectTypeOf < InferContractRouterErrorMap<typeof router>>().toEqualTypeOf<typeof baseErrorMap | Record<never, never>>()
})
-it('InferContractRouterErrorMap', () => {
- expectTypeOf<InferContractRouterErrorMap<typeof router>>().toEqualTypeOf<typeof baseErrorMap | Record<never, never>>()
-})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('InferContractRouterErrorMap', () => { | |
| expectTypeOf < InferContractRouterErrorMap<typeof router>>().toEqualTypeOf<typeof baseErrorMap | Record<never, never>>() | |
| }) | |
| it('InferContractRouterErrorMap', () => { | |
| expectTypeOf<InferContractRouterErrorMap<typeof router>>().toEqualTypeOf<typeof baseErrorMap | Record<never, never>>() | |
| }) | |
| it('InferContractRouterErrorMap', () => { | |
| expectTypeOf < InferContractRouterErrorMap<typeof router>>().toEqualTypeOf<typeof baseErrorMap | Record<never, never>>() | |
| }) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/server/src/implementer.test.ts (1)
149-155: New test case for symbol access protection.This new test case ensures that the implementer doesn't allow recursive access when using symbols, which is a good security practice. However, there's a duplicate assertion on line 154 that's identical to line 153.
expect((implementer.nested as any)[Symbol.for('test')]).toBeUndefined() expect((implementer.nested.ping as any)[Symbol.for('test')]).toBeUndefined() expect((implementer.use as any)[Symbol.for('test')]).toBeUndefined() -expect((implementer.use as any)[Symbol.for('test')]).toBeUndefined()packages/contract/src/router-utils.ts (4)
10-10: Consider removing the non-null assertion.The non-null assertion (
!) onsegmentmay not be necessary if you're iterating through a valid array. TypeScript should understand that array access within bounds will yield a defined value.- const segment = path[i]! + const segment = path[i]
37-58: Type assertion usage could be improved.The
enhanceContractRouterfunction implements a recursive enhancement pattern correctly. However, the use ofas anytype assertions in two places (lines 48 and 57) bypasses TypeScript's type checking, which could lead to potential runtime errors.Consider using more specific type assertions when possible:
- return enhanced as any + return enhanced as EnhancedContractRouter<T, TErrorMap>While this doesn't eliminate type assertions entirely, it provides more specific type information that TypeScript can use for validation.
54-54: Add null check for router properties.The current implementation assumes that
router[key]is always defined when callingenhanceContractRouter. Add a null check to avoid potential runtime errors.- enhanced[key] = enhanceContractRouter(router[key]!, options) + if (router[key] !== undefined) { + enhanced[key] = enhanceContractRouter(router[key], options) + }
51-56: Consider usingObject.entriesfor cleaner iteration.The current approach with a for-in loop works but could be improved for clarity and to avoid iterating over inherited properties.
- const enhanced: Record<string, any> = {} - - for (const key in router) { - enhanced[key] = enhanceContractRouter(router[key]!, options) - } + const enhanced: Record<string, any> = {} + + Object.entries(router).forEach(([key, value]) => { + if (value !== undefined) { + enhanced[key] = enhanceContractRouter(value, options) + } + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/contract/src/route.test.ts(3 hunks)packages/contract/src/router-utils.test.ts(1 hunks)packages/contract/src/router-utils.ts(1 hunks)packages/openapi/src/adapters/standard/openapi-matcher.ts(4 hunks)packages/openapi/src/openapi-custom.ts(2 hunks)packages/server/src/implementer.test.ts(4 hunks)packages/server/src/implementer.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/contract/src/router-utils.test.ts
- packages/contract/src/route.test.ts
- packages/server/src/implementer.ts
🔇 Additional comments (19)
packages/openapi/src/openapi-custom.ts (2)
1-1: Import type update improves type consistency.The import change from
ErrorMapItemtoErrorMapaligns with the broader refactoring effort to improve naming and type consistency across the codebase.
30-30: Type assertion enhanced for better type safety.The change from
(ErrorMapItem<any> | undefined)[]toErrorMap[keyof ErrorMap][]represents a more precise and type-safe approach. This new type accurately captures that we're dealing with an array of any possible values in the ErrorMap, eliminating the need for the explicitundefinedunion as it's likely handled within the ErrorMap type definition itself.packages/server/src/implementer.test.ts (4)
7-8: Imports updated to reflect the new module organization.The imports have been correctly updated to reference the new module names after the refactoring from "adapt" to "enhance" pattern.
10-12: Spy references updated to match the new function names.The spy references have been properly updated from
setRouterContractSpytosetHiddenRouterContractSpyand fromadaptRouterSpytoenhanceRouterSpy, which aligns with the refactoring goal mentioned in the PR objectives.
64-75: Test expectations correctly updated to use new function names.The test assertions have been properly updated to use the new function names (
getHiddenRouterContractinstead ofgetRouterContract) and the new spy references. The parameters passed to the spy functions in the assertions have also been updated to align with the new function signatures.
80-96: Lazy router test assertions properly updated.Similar to the router test above, the lazy router test assertions have been correctly updated to use the new function names and spy references. The validation of the lazy router implementation remains intact.
packages/contract/src/router-utils.ts (4)
1-5: Imports look good.All necessary imports are correctly included to support the module's functionality.
6-24: LGTM: Traversal function follows immutable pattern.The
getContractRouterfunction correctly handles the traversal of contract router path segments with appropriate early returns. Good use of readonly for the path parameter to enforce immutability.
26-32: Well-designed recursive type.The
EnhancedContractRoutergeneric type effectively uses conditional types to distinguish between contract procedures and nested routers, applying the appropriate transformation to each.
33-35: Interface extends base options appropriately.The
EnhanceContractRouterOptionsinterface properly extendsEnhanceRouteOptionswith the additionalerrorMapproperty.packages/openapi/src/adapters/standard/openapi-matcher.ts (9)
1-1: Import naming changes align with broader refactoring effortThe imports have been updated to reflect the renamed functions and types (
LazyTraverseContractProceduresOptionsinstead ofEachContractProcedureLaziedOptions, and imports liketraverseContractProceduresinstead ofeachContractProcedure). This is consistent with the broader refactoring described in the PR objectives.Also applies to: 4-4
10-10: Good immutability enforcementMaking the path array readonly enforces immutability, which is a good practice for preventing unintended mutations of the path array.
16-16: Type update for pendingRoutersThe type has been updated from
EachContractProcedureLaziedOptionstoLazyTraverseContractProceduresOptions, aligning with the renaming convention changes in the PR.
18-19: Method parameter and implementation changesThe
initmethod now:
- Takes a readonly path array with a default empty array
- Uses
traverseContractProceduresinstead ofeachContractProcedureThese changes enhance type safety and match the broader refactoring pattern.
21-21: Consistent naming with toHttpPathThe function
convertPathToHttpPathhas been replaced withtoHttpPathacross multiple locations, which improves naming consistency and readability.Also applies to: 43-43, 80-80
44-44: Using getLazyMeta helperUsing
getLazyMeta(option.router).prefixinstead of direct access improves the code's maintainability by abstracting the internal structure of the router object.
58-58: Improved module import destructuringThe code now consistently uses destructuring to extract the default export from lazy-loaded modules, which improves readability and makes the pattern more explicit.
Also applies to: 76-76
76-76: Router access method changeChanged from using
getRouterChildtogetRouter, which aligns with the broader API simplification in the PR.
85-85:Details
✅ Verification successful
Parameter order change in createContractedProcedure
The parameter order in
createContractedProcedurehas been reversed from(contract, procedure)to(procedure, contract). Ensure this change is consistent across all usages in the codebase.
🏁 Script executed:
#!/bin/bash # Check for other usages of createContractedProcedure to verify consistent parameter order rg "createContractedProcedure\(" --type tsLength of output: 604
Verified Consistency in Parameter Order for createContractedProcedure
- The updated parameter order
(procedure, contract)is consistently used across all instances.- Verified usages in
packages/server/src/procedure-utils.ts,packages/server/src/procedure-utils.test.ts,packages/openapi/src/adapters/standard/openapi-matcher.ts, andpackages/server/src/adapters/standard/rpc-matcher.tsconfirm the change.No further modifications are required.
1 participant
Summary by CodeRabbit
createAssertedLazyProcedureandcreateContractedProcedure.getRouter,enhanceRouter, and other router utility functions.These improvements maintain existing functionality while enhancing stability and developer efficiency.