-
-
Notifications
You must be signed in to change notification settings - Fork 126
feat(contract): inferRPCMethodFromContractRouter & minifyContractRouter #578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces new utilities and documentation for converting routers into client-safe contracts. It adds the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Server as Server (Router)
participant Utils as Contract Utils
participant Client as Client
Dev->>Server: Define router (possibly with lazy routers)
Dev->>Utils: Call minifyContractRouter(router)
Utils->>Dev: Return minified contract JSON
Dev->>Client: Export/import minified contract
Client->>Utils: Use inferRPCMethodFromContractRouter(minifiedContract)
Client->>Server: Make RPC/OpenAPI requests with inferred HTTP methods
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/hey-api
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/react
@orpc/react-query
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/contract/src/link-utils.test.ts (1)
22-23: Good error handling test, but could be more specific.The test verifies that calling with a non-existent path throws an error, which is important. However, consider making the assertion more specific about the error message or type.
Consider enhancing the error assertion:
- expect(() => method({}, ['nested', 'not-exist'])).toThrow() + expect(() => method({}, ['nested', 'not-exist'])).toThrow(/No valid procedure found at path/)packages/contract/src/link-utils.ts (1)
12-27: Solid implementation with proper error handling.The function correctly:
- Returns a closure matching the expected signature for RPCLink
- Uses
getutility for safe path resolution- Validates the procedure with
isContractProcedure- Provides a descriptive error message for invalid paths
- Uses
fallbackContractConfigfor proper method resolution- Converts HEAD to GET for HTTP compatibility
The error message formatting could be slightly improved for better readability.
Consider improving the error message formatting:
- throw new Error(` - [inferRPCMethodFromContractRouter] No valid procedure found at path "${path.join('.')}". - This may happen when the contract router is not properly configured. - `) + throw new Error( + `[inferRPCMethodFromContractRouter] No valid procedure found at path "${path.join('.')}". ` + + `This may happen when the contract router is not properly configured.` + )apps/content/docs/contract-first/router-to-contract.md (3)
12-12: Fix subject–verb agreement in the guide text.The sentence currently reads:
If your router include a [lazy router](/docs/router#lazy-router), you need to fully resolve it to make it compatible with contract.It should be:
- If your router include a [lazy router](/docs/router#lazy-router), you need to fully resolve it to make it compatible with contract. + If your router includes a [lazy router](/docs/router#lazy-router), you need to fully resolve it to make it compatible with contract.🧰 Tools
🪛 LanguageTool
[uncategorized] ~12-~12: This verb does not appear to agree with the subject. Consider using a different form.
Context: .... ## Unlazy the Router If your router include a [lazy router](/docs/router#lazy-route...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~12-~12: You might be missing the article “the” here.
Context: ...y resolve it to make it compatible with contract. ```ts import { unlazyRouter } from '@...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
17-17: Correct the variable name typo in the code snippet.The example declares
unlaziedRouter, which appears to be a misspelling. For clarity and consistency, rename it to something likeresolvedRouter:- const unlaziedRouter = await unlazyRouter(router) + const resolvedRouter = await unlazyRouter(router)
46-46: Add a note about JSON module import requirements.Importing a JSON file in TypeScript usually requires enabling
resolveJsonModule(and sometimesesModuleInterop) intsconfig.json. Consider adding a brief note or a link to the TS docs to help users avoid import errors:::: tip Make sure your `tsconfig.json` has `"resolveJsonModule": true` (and `"esModuleInterop": true` if needed) so that `import contract from './contract.json'` works without compilation issues. :::
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/content/.vitepress/config.ts(1 hunks)apps/content/docs/client/rpc-link.md(1 hunks)apps/content/docs/contract-first/router-to-contract.md(1 hunks)apps/content/docs/openapi/client/openapi-link.md(1 hunks)packages/contract/src/index.ts(1 hunks)packages/contract/src/link-utils.test-d.ts(1 hunks)packages/contract/src/link-utils.test.ts(1 hunks)packages/contract/src/link-utils.ts(1 hunks)packages/contract/src/router-utils.test.ts(2 hunks)packages/contract/src/router-utils.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/contract/src/link-utils.test.ts (3)
packages/contract/src/link-utils.ts (1)
inferRPCMethodFromContractRouter(12-27)packages/contract/src/router-utils.ts (1)
minifyContractRouter(71-91)packages/contract/src/builder.ts (1)
oc(189-198)
packages/contract/src/link-utils.ts (4)
packages/contract/src/router.ts (1)
AnyContractRouter(17-17)packages/contract/src/index.ts (1)
HTTPMethod(21-21)packages/contract/src/procedure.ts (1)
isContractProcedure(51-66)packages/contract/src/config.ts (1)
fallbackContractConfig(20-26)
🪛 LanguageTool
apps/content/docs/contract-first/router-to-contract.md
[uncategorized] ~12-~12: This verb does not appear to agree with the subject. Consider using a different form.
Context: .... ## Unlazy the Router If your router include a [lazy router](/docs/router#lazy-route...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~12-~12: You might be missing the article “the” here.
Context: ...y resolve it to make it compatible with contract. ```ts import { unlazyRouter } from '@...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: publish-commit
- GitHub Check: lint
🔇 Additional comments (15)
apps/content/.vitepress/config.ts (1)
82-82: Verify sidebar ordering consistency.
The new "Router to Contract" entry is correctly added under "Contract First". Ensure the ordering matches related sections and the link path is accurate.packages/contract/src/index.ts (1)
8-8: Expose link-utils in public API.
Addingexport * from './link-utils'makesinferRPCMethodFromContractRouteravailable through the main package index. It follows the existing export ordering.apps/content/docs/openapi/client/openapi-link.md (1)
41-41: Refresh advanced router note.
Updated the info box to direct users to the new "Router to Contract" guide. The wording and link are clear and consistent with other docs.packages/contract/src/link-utils.test-d.ts (1)
1-4: Validate test imports.
Imports forRPCLink, the shared testcontractrouter, andinferRPCMethodFromContractRouterappear correct. Confirm that the shared router test data provides the expected metadata for method inference.apps/content/docs/client/rpc-link.md (1)
108-111: Check "Automatically use method" details header.
Thedetailsblock introduction is useful. Verify that VitePress properly renders this expandable section.packages/contract/src/link-utils.test.ts (3)
1-4: LGTM! Clean and appropriate imports.The imports are well-organized and include all necessary dependencies for the test.
5-14: Good test setup with comprehensive route coverage.The test creates a well-structured contract router that covers:
- Different HTTP methods (HEAD, GET, POST, DELETE)
- Default method behavior (line 9 -
postroute without explicit method)- Nested routes for testing path resolution
The use of
minifyContractRouterin the setup is appropriate and follows the expected workflow.
16-21: Comprehensive method inference validation.The test assertions properly verify:
- HEAD to GET conversion (line 16)
- Explicit method preservation (lines 17, 19, 20)
- Default method behavior (line 18 - POST is the default)
- Nested route handling (lines 19-20)
This covers the core functionality thoroughly.
packages/contract/src/router-utils.ts (3)
63-70: Excellent documentation with helpful context.The JSDoc comment clearly explains the purpose, usage, and benefits of the function. The link to documentation provides additional context for users.
71-82: Solid procedure minification logic.The function correctly:
- Creates a new minimal procedure object
- Clears the
errorMapfor security and size reduction- Preserves
metaandroutewhich are essential for client functionality- Maintains the correct structure expected by
isContractProcedure
84-91: Proper recursive handling for nested routers.The recursive logic correctly processes nested router structures by:
- Creating a new object to avoid mutations
- Recursively calling
minifyContractRouteron each property- Preserving the router structure
packages/contract/src/router-utils.test.ts (2)
2-4: Good import additions for new functionality.The imports correctly add the necessary functions (
isContractProcedureandminifyContractRouter) to support the new test case.
40-74: Comprehensive test coverage for router minification.The test effectively validates:
- Correct minification of different procedure types (
pingwith metadata vspongwithout)- Proper structure preservation (lines 43-61 define expected minimal shapes)
- Contract procedure compliance using
isContractProcedurepredicate- Both top-level and nested procedure handling
The test data shows that:
errorMapis properly cleared to empty objectmetais preserved when present (mode: 'dev' for ping)routeis preserved when present (path: '/base' for ping)This thoroughly validates the minification logic.
packages/contract/src/link-utils.ts (2)
1-6: Clean imports with appropriate dependencies.The imports are well-organized and include all necessary utilities:
- Type imports for HTTPMethod and AnyContractRouter
- Utility functions for path resolution, config fallback, and procedure validation
7-11: Clear and helpful documentation.The JSDoc comment effectively explains the function's purpose and includes a link to relevant documentation for additional context.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests