feat: add new way of defining api contracts#902
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: lokalise/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request introduces a new type-safe REST API contract system within the Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/api-contracts/src/new/contractResponse.ts`:
- Around line 82-102: The current matchTypedResponse uses substring includes
which is brittle; modify matchTypedResponse to parse the incoming contentType
into a MIME-aware media type (strip parameters after ';', trim and lowercase)
and then perform exact comparisons: for text/blob/sse compare the normalized
media type to the normalized entry.contentType (for SSE check equality to
'text/event-stream'), and for JSON treat media types that are exactly
'application/json' or that endWith('+json') (case-insensitive) as JSON; update
the logic in matchTypedResponse (and any comparisons involving entry.contentType
or contentType) to use this normalized media type instead of includes so
matching is robust to case, parameters, and vendor suffixes.
In `@packages/app/api-contracts/src/new/README.md`:
- Around line 129-131: Update the README to reflect the actual required type for
requestPathParamsSchema: change the documented type from z.ZodType to
z.ZodObject and add a short note that path params must be an object schema,
matching the RequestPathParamsSchema definition in defineApiContract.ts so users
don't try to pass non-object Zod schemas for path parameters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: lokalise/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b133c223-8efc-4b98-aa46-ea6fdd4509fe
📒 Files selected for processing (11)
packages/app/api-contracts/src/HttpStatusCodes.tspackages/app/api-contracts/src/index.tspackages/app/api-contracts/src/new/README.mdpackages/app/api-contracts/src/new/constants.tspackages/app/api-contracts/src/new/contractResponse.spec.tspackages/app/api-contracts/src/new/contractResponse.tspackages/app/api-contracts/src/new/defineApiContract.spec.tspackages/app/api-contracts/src/new/defineApiContract.tspackages/app/api-contracts/src/new/inferTypes.spec.tspackages/app/api-contracts/src/new/inferTypes.tspackages/app/api-contracts/src/typeUtils.ts
9c71dce to
2a0003b
Compare
Changes
Summary
defineApiContract— a new way to define contracts that unifies REST and SSE use-casesinferTypeshelpers (InferSuccessResponse,InferSuccessSchema,HasAnyNonJsonSuccessResponse) for consuming contract response types in application code; introducesContractNoBody/ContractNonJsonResponsesymbols for non-JSON and empty-body scenariosOld vs new contract definition
The old
buildContract/buildRestContractAPI threaded every aspect of the route through explicit generic type parameters. A typical payload route required up to 9 type parameters just to get full inference:The response type lived on a single successResponseBodySchema field with no per-status-code granularity, and flags like isEmptyResponseExpected / isNonJSONResponseExpected had to be specified as both a runtime value and a boolean generic to keep inference correct.
The new defineApiContract drops all explicit generics. Types are inferred entirely from the shape of the object you pass in — the same DX, with significantly less noise:
// New
Per-status-code responses are first-class, and ContractNoBody / ContractNonJsonResponse symbols replace the boolean flags entirely — no generics, no duplication.
▎ Note: The new API achieves the same level of type safety without being heavily dependent on explicit generics. This results in cleaner contract definitions that are easier to read, write, and maintain, while TypeScript still infers everything correctly.
Checklist
major,minor,patchorskip-releaseAI Assistance Tracking
We're running a metric to understand where AI assists our engineering work. Please select exactly one of the options below:
Mark "Yes" if AI helped in any part of this work, for example: generating code, refactoring, debugging support,
explaining something, reviewing an idea, or suggesting an approach.