perf(types): reduce deep and exhaustive type inference complexity#45
perf(types): reduce deep and exhaustive type inference complexity#45halvaradop merged 8 commits intomasterfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR centralizes endpoint schema inference via a new UnwrapSchema and adds a consolidated src/@types surface (client/http/types). RequestContext and MiddlewareFunction are retyped to use EndpointSchemas; getSearchParams, executeMiddlewares, router imports, tests, and package/deno exports are updated to the new types. ChangesSchema Inference Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/types.ts (1)
129-144:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
RequestContexttyping relies onNonNullable<Config["schemas"]>, which collapses toneverwhenschemasis absent.
Configdefaults to{ schemas?: EndpointSchemas }, soConfig["schemas"]can beundefined, andNonNullable<undefined>isnever. PassingneverintoContextParams/ContextBody/ContextSearchParamscauses the[S] extends [ZodObject<any>]branch to match (sinceneveris assignable to every type), producing surprising inferred shapes for callers without schemas. This is very likely the root cause of the//@ts-ignore`` workarounds insrc/router.tsand `test/context.test.ts`.Consider switching to a
Config["schemas"] extends EndpointSchemas ? Config["schemas"] : {}pattern (or makingRequestContexttakeSchemasdirectly and let call sites compute it), so the absent-schemas case yieldsEndpointSchemas/{}andUnwrapSchemacleanly hits theFallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/types.ts` around lines 129 - 144, RequestContext currently uses NonNullable<Config["schemas"]> which collapses to never when schemas is absent and causes ContextParams/ContextBody/ContextSearchParams to pick the wrong branch; change the generic handling so absent schemas resolve to an empty shape (e.g. replace NonNullable<Config["schemas"]> with a conditional type like Config["schemas"] extends EndpointSchemas ? Config["schemas"] : {}), or refactor RequestContext to accept a Schemas generic parameter computed at call sites, and update the references to ContextParams, ContextBody and ContextSearchParams to use that resolved Schemas type (symbols: RequestContext, Config["schemas"], ContextParams, ContextBody, ContextSearchParams, EndpointSchemas).src/middlewares.ts (1)
48-65:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPer-route middleware loop silently drops
Responsereturns.
MiddlewareFunctionis typed to returnResponse | RequestContext<Route, { schemas: Config }> | Promise<Response | ...>, but line 55 unconditionally casts the result toRequestContext. If a route-level middleware returns aResponse(which the public type advertises and whichexecuteGlobalMiddlewaresalready handles correctly), it gets assigned toctxand the next middleware/handler receives aResponseinstance instead of aRequestContext— at runtime this will fail when code callsctx.searchParams,ctx.headers, etc.Mirror the global-middleware short-circuit (lines 24–27 in
executeGlobalMiddlewares): checkinstanceof Responseand return early. Update the caller insrc/router.tsto also branch onResponse.🛠 Proposed fix
try { let ctx = context for (const middleware of use) { if (typeof middleware !== "function") { throw new RouterError("BAD_REQUEST", "Middleware must be a function") } try { - ctx = (await middleware(ctx)) as RequestContext<Route, { schemas: Config }> + const result = await middleware(ctx) + if (result instanceof Response) { + return result + } + ctx = result as RequestContext<Route, { schemas: Config }> } catch (error) { if (error instanceof RouterError) throw error throw new RouterError("BAD_REQUEST", "Handler threw an error", "MiddlewareError") } } return ctx } catch { throw new RouterError("BAD_REQUEST", "Handler threw an error") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/middlewares.ts` around lines 48 - 65, The per-route middleware loop assigns middleware results to ctx without handling Response returns, causing a Response to be passed as a RequestContext; update the loop in the function that iterates over use/middleware (the block that sets let ctx = context and does ctx = (await middleware(ctx)) as RequestContext<Route, { schemas: Config }>) to detect if the result is an instance of Response and return it immediately (mirroring executeGlobalMiddlewares behavior) instead of casting to RequestContext; preserve existing RouterError checks and throw MiddlewareError for non-RouterError exceptions, and also update the caller in src/router.ts that awaits this middleware runner to branch on a returned Response (handle short-circuiting) before treating the result as a RequestContext.
🧹 Nitpick comments (3)
src/router.ts (1)
58-59: ⚡ Quick winSuppress the symptom, not the type system — and prefer
@ts-expect-error.The "overlapping types" being silenced here are the
NonNullable<Config["schemas"]>issue flagged onsrc/types.ts. Two improvements:
- Once
RequestContext/UnwrapSchemacorrectly handle the absent-schemas case, this directive should become unnecessary — fixing the helpers is preferable to suppressing each call site.- In the meantime, use
@ts-expect-error(with a short reason) so TS will fail the build the moment the underlying type is fixed and the directive becomes obsolete. A bare@ts-ignorewill silently linger.♻️ Stop-gap until the helper types are fixed
- // `@ts-ignore` Skip type checking here because there's overlapping types + // `@ts-expect-error` endpoint.config has a wider Schemas than getSearchParams's generic; tracked in src/types.ts UnwrapSchema/RequestContext const searchParams = getSearchParams(globalRequestContext.request.url, endpoint.config)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/router.ts` around lines 58 - 59, Replace the bare "@ts-ignore" before the getSearchParams call with an "@ts-expect-error" and a short reason (e.g. "// `@ts-expect-error`: temporary until RequestContext/UnwrapSchema handle absent Config['schemas']") so TypeScript will fail once the underlying types are corrected; also add a TODO referencing RequestContext and UnwrapSchema to remind maintainers to remove this once the NonNullable<Config['schemas']> issue is fixed, and avoid adding further ignores at other call sites like usages of globalRequestContext.request.url or getSearchParams.test/context.test.ts (1)
298-299: ⚡ Quick winPrefer
@ts-expect-error(with a reason) over@ts-ignore.Same applies to line 348. These are workarounds for the upstream
RequestContext/UnwrapSchematyping issue. Switching to@ts-expect-errorensures the directive auto-fails once the helper types are fixed, so these tests stop silently masking real type regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/context.test.ts` around lines 298 - 299, Replace the two occurrences of the loose suppression comment with explicit expected-error comments: change the existing `@ts-ignore` above the getSearchParams call (and the similar suppression at the other test around line with the UnwrapSchema/RequestContext workaround) to `@ts-expect-error` and include a brief reason string explaining it’s a temporary workaround for the upstream RequestContext/UnwrapSchema typing issue so the tests will fail once the upstream types are fixed; locate the suppressions by the getSearchParams call and the test referencing UnwrapSchema/RequestContext and update both comments accordingly.src/middlewares.ts (1)
44-47: 💤 Low valueGeneric name
Configis misleading now that it holdsEndpointSchemas.The parameter is no longer an
EndpointConfig— it's the schemas slice — soConfig extends EndpointSchemasreads confusingly at call sites ({ schemas: Config }). Renaming toSchemaskeeps it consistent withMiddlewareFunction<Route, Schemas>insrc/types.ts.♻️ Proposed rename
-export const executeMiddlewares = async <Route extends RoutePattern, Config extends EndpointSchemas = EndpointSchemas>( - context: RequestContext<Route, { schemas: Config }>, - use: MiddlewareFunction<Route, Config>[] = [] -) => { +export const executeMiddlewares = async <Route extends RoutePattern, Schemas extends EndpointSchemas = EndpointSchemas>( + context: RequestContext<Route, { schemas: Schemas }>, + use: MiddlewareFunction<Route, Schemas>[] = [] +) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/middlewares.ts` around lines 44 - 47, Rename the generic type parameter Config to Schemas in the executeMiddlewares declaration so the signature reads executeMiddlewares<Route extends RoutePattern, Schemas extends EndpointSchemas = EndpointSchemas>(context: RequestContext<Route, { schemas: Schemas }>, use: MiddlewareFunction<Route, Schemas>[] = []) to reflect that the generic represents the schemas slice; update all references within the function and its type annotations (including the RequestContext generic payload and the MiddlewareFunction generic) from Config to Schemas to keep names consistent with src/types.ts and avoid confusing call-site reads.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/types.ts`:
- Around line 90-99: The UnwrapSchema conditional for the valibot branch returns
the literal type false when an ObjectSchema's "~types" doesn't match the
expected { output: infer Output }, which then pollutes
ContextBody/ContextSearchParams/ContextParams; change the valibot branch in
UnwrapSchema (the clause matching [S] extends [ObjectSchema<any, undefined>]) to
fall back to Fallback (or unknown) instead of false by making the final
conditional yield Fallback when the inferred Output can't be extracted from
S["~types"] so callers receive a usable type; update references to UnwrapSchema,
ObjectSchema and the "~types" extraction logic accordingly.
- Around line 92-96: The code uses valibot's internal NonNullable<S["~types"]>
to get schema output which is fragile; change this branch to use the public
InferOutput<S> helper (same approach as InferSchema) instead of accessing
"~types", and ensure InferOutput is imported/available where the conditional is
declared; if you intentionally need the internal path instead, pin valibot to an
exact version and add a tsd/expectTypeOf regression test that will fail if
"~types" changes.
---
Outside diff comments:
In `@src/middlewares.ts`:
- Around line 48-65: The per-route middleware loop assigns middleware results to
ctx without handling Response returns, causing a Response to be passed as a
RequestContext; update the loop in the function that iterates over
use/middleware (the block that sets let ctx = context and does ctx = (await
middleware(ctx)) as RequestContext<Route, { schemas: Config }>) to detect if the
result is an instance of Response and return it immediately (mirroring
executeGlobalMiddlewares behavior) instead of casting to RequestContext;
preserve existing RouterError checks and throw MiddlewareError for
non-RouterError exceptions, and also update the caller in src/router.ts that
awaits this middleware runner to branch on a returned Response (handle
short-circuiting) before treating the result as a RequestContext.
In `@src/types.ts`:
- Around line 129-144: RequestContext currently uses
NonNullable<Config["schemas"]> which collapses to never when schemas is absent
and causes ContextParams/ContextBody/ContextSearchParams to pick the wrong
branch; change the generic handling so absent schemas resolve to an empty shape
(e.g. replace NonNullable<Config["schemas"]> with a conditional type like
Config["schemas"] extends EndpointSchemas ? Config["schemas"] : {}), or refactor
RequestContext to accept a Schemas generic parameter computed at call sites, and
update the references to ContextParams, ContextBody and ContextSearchParams to
use that resolved Schemas type (symbols: RequestContext, Config["schemas"],
ContextParams, ContextBody, ContextSearchParams, EndpointSchemas).
---
Nitpick comments:
In `@src/middlewares.ts`:
- Around line 44-47: Rename the generic type parameter Config to Schemas in the
executeMiddlewares declaration so the signature reads executeMiddlewares<Route
extends RoutePattern, Schemas extends EndpointSchemas =
EndpointSchemas>(context: RequestContext<Route, { schemas: Schemas }>, use:
MiddlewareFunction<Route, Schemas>[] = []) to reflect that the generic
represents the schemas slice; update all references within the function and its
type annotations (including the RequestContext generic payload and the
MiddlewareFunction generic) from Config to Schemas to keep names consistent with
src/types.ts and avoid confusing call-site reads.
In `@src/router.ts`:
- Around line 58-59: Replace the bare "@ts-ignore" before the getSearchParams
call with an "@ts-expect-error" and a short reason (e.g. "// `@ts-expect-error`:
temporary until RequestContext/UnwrapSchema handle absent Config['schemas']") so
TypeScript will fail once the underlying types are corrected; also add a TODO
referencing RequestContext and UnwrapSchema to remind maintainers to remove this
once the NonNullable<Config['schemas']> issue is fixed, and avoid adding further
ignores at other call sites like usages of globalRequestContext.request.url or
getSearchParams.
In `@test/context.test.ts`:
- Around line 298-299: Replace the two occurrences of the loose suppression
comment with explicit expected-error comments: change the existing `@ts-ignore`
above the getSearchParams call (and the similar suppression at the other test
around line with the UnwrapSchema/RequestContext workaround) to `@ts-expect-error`
and include a brief reason string explaining it’s a temporary workaround for the
upstream RequestContext/UnwrapSchema typing issue so the tests will fail once
the upstream types are fixed; locate the suppressions by the getSearchParams
call and the test referencing UnwrapSchema/RequestContext and update both
comments accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 686914a1-766f-4f94-8d47-8183570d9adf
📒 Files selected for processing (10)
src/context.tssrc/middlewares.tssrc/router.tssrc/types.tssrc/validator/registry.tstest/context.test.tstest/endpoint.test.tstest/middlewares.test.tstest/type.test-d.tstsconfig.json
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tsconfig.json (1)
13-32: 💤 Low valueSeveral added strict flags are already implied by
strict: true.
alwaysStrict,noImplicitAny,strictNullChecks, andstrictFunctionTypesare all enabled when"strict": trueis set; listing them again is redundant. The non-redundant additions worth keeping explicit arenoUnusedLocals,noUnusedParameters,noImplicitReturns,noFallthroughCasesInSwitch, andverbatimModuleSyntax(since these are not part of thestrictfamily).♻️ Proposed cleanup
"strict": true, "allowJs": true, "checkJs": true, - "alwaysStrict": true, "esModuleInterop": true, ... /* Type checking */ "noUnusedLocals": true, "noUnusedParameters": true, - "noImplicitAny": true, "noImplicitReturns": true, "noFallthroughCasesInSwitch": true, - "strictNullChecks": true, - "strictFunctionTypes": true, "verbatimModuleSyntax": true,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tsconfig.json` around lines 13 - 32, Remove redundant strict-family compiler options that are implied by "strict": true — specifically delete "alwaysStrict", "noImplicitAny", "strictNullChecks", and "strictFunctionTypes" from tsconfig.json; retain the explicit non-strict flags you want to keep such as "noUnusedLocals", "noUnusedParameters", "noImplicitReturns", "noFallthroughCasesInSwitch", and "verbatimModuleSyntax" so they remain explicit and effective.src/@types/client.ts (2)
47-62: 💤 Low value
HasSchemas<C>collapses toneverfor empty schemas, relying on vacuousnever extends false.When
Schemas = {},Schemas[keyof Schemas]isnever, soHasSchemas<C>distributes tonever. The downstreamHasSchemas<Config> extends false ? noBodyArm : bodyArmstill resolves tonoBodyArmbecausenever extends falseis true, so the result is correct — but this is non-obvious. A short comment would help future readers, or you could short-circuit explicitly withkeyof Schemas extends never ? false : ....🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/`@types/client.ts around lines 47 - 62, The conditional type HasSchemas currently collapses to never for empty schema objects (because Schemas[keyof Schemas] becomes never), causing a non-obvious `never extends false` behavior used by Client; update HasSchemas to explicitly short-circuit empty schemas (e.g., make it return false when `keyof Schemas extends never`) so the downstream conditional in the Client type is clear and stable, or add a concise comment next to HasSchemas explaining the vacuous `never` distribution; refer to the HasSchemas type and the Client<Endpoints...> generic so the fix is applied where the schema presence check is defined and consumed.
5-6: 💤 Low valueMerge the two imports from the same module.
Lines 5 and 6 both pull from
@/@types/types.ts; combine into a singleimport typestatement.♻️ Proposed change
-import type { InferValibotSchema, RoutePattern } from "@/@types/types.ts" -import type { EndpointConfig, Prettify, RouteEndpoint } from "@/@types/types.ts" +import type { EndpointConfig, InferValibotSchema, Prettify, RouteEndpoint, RoutePattern } from "@/@types/types.ts"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/`@types/client.ts around lines 5 - 6, Merge the two separate import lines that both import from the same module into a single import type statement; replace the two imports that reference InferValibotSchema, RoutePattern and EndpointConfig, Prettify, RouteEndpoint with one consolidated `import type` line that lists all five symbols (InferValibotSchema, RoutePattern, EndpointConfig, Prettify, RouteEndpoint) so there is a single import from the module.src/@types/types.ts (1)
70-81: 💤 Low valueAdd a documentation note about the internal
~typesdependency.
InferValibotSchemaintentionally reads from Valibot's internal~typesphantom field to avoid performance issues withInferOutput. Valibot's documentation marks~typesas internal API, meaning it could change in minor versions. Document this dependency so future Valibot upgrades trigger a deliberate review.♻️ Suggested doc update
/** * Utility type to infer the output type of a Valibot ObjectSchema. This utility is used instead * of `InferOutput` directly to avoid performance issues caused by deeply nested conditional types. + * + * Note: relies on Valibot's internal `~types` field. Verify compatibility on every Valibot major/minor bump. */ export type InferValibotSchema<S extends ObjectSchema<any, undefined>> = NonNullable<S["~types"]>["output"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/`@types/types.ts around lines 70 - 81, Add a short doc comment above InferValibotSchema explaining that it intentionally reads Valibot's internal phantom field "~types" (used by InferValibotSchema) to avoid performance issues with InferOutput, and note that "~types" is an internal API that may change across minor versions so upgrades should include a deliberate review and testing; reference the InferValibotSchema type and the UnwrapSchema helper (which branches on ObjectSchema and uses InferValibotSchema) so maintainers know where the dependency exists.package.json (1)
83-83: 💤 Low valueThe string form
"./types": "./dist/@types/index.d.ts"is valid but consider the explicit conditional for robustness.TypeScript's modern
moduleResolutionmodes (node16,nodenext) will resolve the string-valued export and discover the.d.tsfile via sibling lookup. However, downstream consumers using older TypeScript configurations benefit from the explicit"types"condition for clearer intent and more predictable type resolution across different module-resolution strategies.♻️ Suggested form
- "./types": "./dist/@types/index.d.ts" + "./types": { + "types": "./dist/@types/index.d.ts" + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 83, Replace the simple string export for the "./types" key with an explicit conditional export so TypeScript's older moduleResolution modes reliably pick up the .d.ts entry; locate the "./types" entry in package.json and change it to an exports-style conditional that includes a "types" condition targeting "./dist/@types/index.d.ts (and fallbacks for default/node as needed)" so both modern and legacy TS consumers resolve your declaration file consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/`@types/types.ts:
- Around line 144-149: The exported MiddlewareFunction signature changed its
second generic from the old config-wrapper shape to take EndpointSchemas
directly, which is a breaking API change; revert or restore a
backwards-compatible overload so MiddlewareFunction<Route, { schemas: ... }>
continues to work: either restore the original generic shape that expects
RequestContext<Route, { schemas: Schemas }> (using the existing types
RequestContext, RoutePattern and EndpointSchemas) or add an overload/alias that
maps the wrapper form to the new shape, and add a CHANGELOG/migration note
documenting the change and recommended typings for external users.
---
Nitpick comments:
In `@package.json`:
- Line 83: Replace the simple string export for the "./types" key with an
explicit conditional export so TypeScript's older moduleResolution modes
reliably pick up the .d.ts entry; locate the "./types" entry in package.json and
change it to an exports-style conditional that includes a "types" condition
targeting "./dist/@types/index.d.ts (and fallbacks for default/node as needed)"
so both modern and legacy TS consumers resolve your declaration file
consistently.
In `@src/`@types/client.ts:
- Around line 47-62: The conditional type HasSchemas currently collapses to
never for empty schema objects (because Schemas[keyof Schemas] becomes never),
causing a non-obvious `never extends false` behavior used by Client; update
HasSchemas to explicitly short-circuit empty schemas (e.g., make it return false
when `keyof Schemas extends never`) so the downstream conditional in the Client
type is clear and stable, or add a concise comment next to HasSchemas explaining
the vacuous `never` distribution; refer to the HasSchemas type and the
Client<Endpoints...> generic so the fix is applied where the schema presence
check is defined and consumed.
- Around line 5-6: Merge the two separate import lines that both import from the
same module into a single import type statement; replace the two imports that
reference InferValibotSchema, RoutePattern and EndpointConfig, Prettify,
RouteEndpoint with one consolidated `import type` line that lists all five
symbols (InferValibotSchema, RoutePattern, EndpointConfig, Prettify,
RouteEndpoint) so there is a single import from the module.
In `@src/`@types/types.ts:
- Around line 70-81: Add a short doc comment above InferValibotSchema explaining
that it intentionally reads Valibot's internal phantom field "~types" (used by
InferValibotSchema) to avoid performance issues with InferOutput, and note that
"~types" is an internal API that may change across minor versions so upgrades
should include a deliberate review and testing; reference the InferValibotSchema
type and the UnwrapSchema helper (which branches on ObjectSchema and uses
InferValibotSchema) so maintainers know where the dependency exists.
In `@tsconfig.json`:
- Around line 13-32: Remove redundant strict-family compiler options that are
implied by "strict": true — specifically delete "alwaysStrict", "noImplicitAny",
"strictNullChecks", and "strictFunctionTypes" from tsconfig.json; retain the
explicit non-strict flags you want to keep such as "noUnusedLocals",
"noUnusedParameters", "noImplicitReturns", "noFallthroughCasesInSwitch", and
"verbatimModuleSyntax" so they remain explicit and effective.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7405de8-1529-4ca3-a0fd-839a39c8d41e
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockdeno.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
bench/comparator.bench.mtsbench/router.bench.tsdeno.jsonpackage.jsonsrc/@types/client.tssrc/@types/http.tssrc/@types/index.tssrc/@types/types.tssrc/assert.tssrc/client.tssrc/context.tssrc/endpoint.tssrc/index.tssrc/middlewares.tssrc/on-error.tssrc/response.tssrc/router.tssrc/trie.tstest/client.test.tstest/context.test.tstest/endpoint.test.tstest/middlewares.test.tstest/type.test-d.tstsconfig.json
✅ Files skipped from review due to trivial changes (11)
- src/trie.ts
- deno.json
- src/@types/index.ts
- src/endpoint.ts
- src/on-error.ts
- src/response.ts
- src/assert.ts
- src/@types/http.ts
- src/client.ts
- bench/comparator.bench.mts
- test/endpoint.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/router.ts
- test/context.test.ts
- test/middlewares.test.ts
- src/context.ts
- src/middlewares.ts
| export type MiddlewareFunction<Route extends RoutePattern = RoutePattern, Schemas extends EndpointSchemas = {}> = ( | ||
| ctx: Prettify<RequestContext<Route, { schemas: Schemas }>> | ||
| ) => | ||
| | Response | ||
| | RequestContext<Route, { schemas: Schemas }> | ||
| | Promise<Response | Prettify<RequestContext<Route, { schemas: Schemas }>>> |
There was a problem hiding this comment.
MiddlewareFunction's second generic shape changed — this is a breaking change for direct users.
Previously the second generic accepted a config-like wrapper (e.g. { schemas: ... }); it now takes EndpointSchemas directly. Tests have been updated, but external code that types middleware as MiddlewareFunction<Route, { schemas: { body: ... } }> will silently start to mean something different (the wrapper would be treated as EndpointSchemas and not satisfy the constraint). Worth a CHANGELOG / migration note.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/`@types/types.ts around lines 144 - 149, The exported MiddlewareFunction
signature changed its second generic from the old config-wrapper shape to take
EndpointSchemas directly, which is a breaking API change; revert or restore a
backwards-compatible overload so MiddlewareFunction<Route, { schemas: ... }>
continues to work: either restore the original generic shape that expects
RequestContext<Route, { schemas: Schemas }> (using the existing types
RequestContext, RoutePattern and EndpointSchemas) or add an overload/alias that
maps the wrapper form to the new shape, and add a CHANGELOG/migration note
documenting the change and recommended typings for external users.
1 participant
Description
This pull request improves internal type-system consistency for type inference and type references across the router internals, especially for deeply nested types such as
EndpointSchemas,EndpointConfig,MiddlewareFunction, andRequestContext.These improvements significantly reduce occurrences of the following TypeScript compiler error:
During the investigation, one of the primary causes of this issue was identified as Valibot's InferOutput type. To address this, a built-in output inference utility was introduced to reduce type complexity and improve compiler stability.
Additionally, several distributive conditional types were simplified to minimize recursive type expansion. Some of these optimizations were initially introduced in the previous PR and are further refined in this update.
Benchs
Related PRs
#44
Summary by CodeRabbit
Release Notes
New Features
Refactor
Chores