perf(types): optimize deeply nested type computations#51
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 46 minutes and 2 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRefactors types to unify Zod/Valibot/TypeBox/Arktype schema handling, adds EndpointMeta-driven RequestContext, updates client/endpoint/middleware generics to include HTTP method, migrates Zod imports to v4, and aligns benches/tests/configs to the new typing. ChangesType System Modernization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 3
🧹 Nitpick comments (1)
tsconfig.diag.json (1)
3-6: Drop or wire uptsconfig.diag.json
tsconfig.diag.jsonextends./tsconfig.json, which already sets"noEmit": trueand"skipLibCheck": true, so these options are redundant.- No repo files/scripts/CI references
tsconfig.diag.json;package.json’stype-checkrunstsc --noEmitwithout-p tsconfig.diag.json.- Remove it to avoid confusion, or update the intended script/CI/IDE config to explicitly use
tsc -p tsconfig.diag.json.🤖 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.diag.json` around lines 3 - 6, tsconfig.diag.json is redundant because it duplicates settings from tsconfig.json and isn’t referenced by CI or scripts; either delete tsconfig.diag.json to avoid confusion, or wire it up by updating the package.json "type-check" script (currently running "tsc --noEmit") and any CI/IDE configurations to run "tsc -p tsconfig.diag.json" so the file is actually used; ensure references to "tsconfig.json" remain correct if you remove or change the diag file.
🤖 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/client.ts:
- Around line 34-44: The tuple-check `[SchemaValues<Schemas>] extends
[SupportedSchema]` treats an empty `Schemas = {}` (where `SchemaValues<{}> =
never`) as matching, so update the type logic in HasSchemas and InferContent to
explicitly handle the `never` case: first check whether `[SchemaValues<Schemas>]
extends [never]` and return false/unknown for empty schemas, otherwise proceed
to test against SupportedSchema and yield true or the inferred
RemoveUndefined<ToInferSchema<Schemas>> as appropriate; modify the discriminated
branches in HasSchemas and InferContent that currently reference EndpointConfig
and unstable__EndpointConfig so they short-circuit on the never case before
checking SupportedSchema.
In `@src/`@types/types.ts:
- Around line 263-270: The conditional type InferMethod currently matches a
mutable array pattern and thus fails for readonly tuples; update its conditional
to infer a readonly element type by changing the check from "Endpoints extends
(infer Endpoint)[]" to "Endpoints extends readonly (infer Endpoint)[]", keeping
the subsequent extraction of Method from unstable__RouteEndpoint and the same
fallback logic so readonly/as const endpoint tuples correctly produce the HTTP
method union used by GetHttpHandlers.
In `@src/endpoint.ts`:
- Around line 43-44: The return type of createEndpoint narrows the config to {
schemas?: Schemas } and drops the optional use middleware; update the generic
return type so it preserves the full unstable__EndpointConfig (including use).
Concretely, change the third type parameter on unstable__RouteEndpoint from {
schemas?: Schemas } to unstable__EndpointConfig<Route, Method, Schemas> (or a
suitably named generic that forwards the exact config type), ensuring
createEndpoint(Route, Method, Schemas, Handler) returns
unstable__RouteEndpoint<Route, Method, unstable__EndpointConfig<Route, Method,
Schemas>, Handler> so endpoint.config.use remains typed.
---
Nitpick comments:
In `@tsconfig.diag.json`:
- Around line 3-6: tsconfig.diag.json is redundant because it duplicates
settings from tsconfig.json and isn’t referenced by CI or scripts; either delete
tsconfig.diag.json to avoid confusion, or wire it up by updating the
package.json "type-check" script (currently running "tsc --noEmit") and any
CI/IDE configurations to run "tsc -p tsconfig.diag.json" so the file is actually
used; ensure references to "tsconfig.json" remain correct if you remove or
change the diag file.
🪄 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: b3709313-ebe1-4cae-bda1-c5e5225e497a
📒 Files selected for processing (8)
deno.jsonsrc/@types/client.tssrc/@types/schemas.tssrc/@types/types.tssrc/assert.tssrc/context.tssrc/endpoint.tstsconfig.diag.json
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/context.ts (1)
131-165:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReturn
undefinedwhen there is no typed body.
RequestContext.bodyis published asundefinedfor schema-less endpoints, butgetBody()returnsnullon Line 133 and Line 165. Sincesrc/router.tsforwards this value directly, handlers can observe a runtime shape that contradicts the exported type.Proposed fix
export const getBody = async <Config extends EndpointConfig<any, any, any>>(request: Request, config: Config) => { if (!isSupportedBodyMethod(request.method)) { - return null + return undefined } @@ - return null + return undefined } catch { throw new RouterError("UNPROCESSABLE_ENTITY", "Invalid request body, the content-type does not match the body format") } }🤖 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/context.ts` around lines 131 - 165, getBody currently returns null for unsupported methods and when no body is parsed (seen in the early return after isSupportedBodyMethod and the final return), but RequestContext.body is typed/published as undefined for schema-less endpoints; update getBody to return undefined instead of null in both places so the runtime matches the exported type (adjust the Promise return type if needed), and ensure callers in router.ts that forward the value continue to accept undefined. Reference symbols: getBody, isSupportedBodyMethod, RequestContext.body and the router.ts forwarding logic.bench/router.bench.ts (1)
8-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix benchmark endpoint generation to use the index (not the element)
Array.from({ length: N })createsundefinedelements; with the callback signature((idx) => ...),idxis alwaysundefined, so the 100/1000-endpoint suites generate duplicate/endpoint-undefinedroutes (also affects the response payload).
Applies tobench/router.bench.tslines 8-15, 22-29, and 41-48.Proposed fix
- const endpoints = Array.from({ length: 100 }).map<RouteEndpoint<any, any, any, any>>((idx) => ({ + const endpoints = Array.from({ length: 100 }).map<RouteEndpoint<any, any, any, any>>((_, idx) => ({ @@ - const endpoints = Array.from({ length: 100 }).map<RouteEndpoint<any, any, any, any>>((idx) => ({ + const endpoints = Array.from({ length: 100 }).map<RouteEndpoint<any, any, any, any>>((_, idx) => ({ @@ - const endpoints = Array.from({ length: 1000 }).map<RouteEndpoint<any, any, any, any>>((idx) => ({ + const endpoints = Array.from({ length: 1000 }).map<RouteEndpoint<any, any, any, any>>((_, idx) => ({🤖 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 `@bench/router.bench.ts` around lines 8 - 15, The benchmark generates duplicate routes because the Array.from callback currently receives the element (undefined) as the first arg, so "idx" is undefined; change the mapping callback to accept the index (e.g., use (_, idx) => ...) so the index is used for route and response generation in the endpoints array; update occurrences inside the object created for each RouteEndpoint (route `/endpoint-${idx}`, handler response `Endpoint ${idx}`, and any config references) so each entry uses the actual numeric index rather than the undefined element.src/@types/client.ts (1)
55-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle readonly endpoint tuples in
Client.
Clientacceptsreadonlyendpoints, but the top-level conditional usesEndpoints extends unknown[], which only matches mutable arrays—soClient<readonly [...]>(e.g.,as constendpoint lists) can collapse to{}. Update the conditional toEndpoints extends readonly unknown[]so the recursive tuple mapping runs for readonly tuples.🤖 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 55 - 69, The Client type's top-level conditional uses "Endpoints extends unknown[]" which fails to match readonly tuples (e.g., as const) and causes Client<readonly [...]> to resolve to {}; change the conditional to "Endpoints extends readonly unknown[]" so the recursive tuple mapping runs for readonly endpoint tuples; update the Client declaration that references Endpoints and RouteEndpoint<...> accordingly to use the readonly check.src/middlewares.ts (1)
54-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve original
RouterErrors from middleware execution.Lines 68-69 currently swallow every
RouterErrorraised above and replace it with a generic"Handler threw an error". That means"Middleware must be a function"and any middleware-thrownRouterErrornever reach the caller intact.Suggested fix
export const executeMiddlewares = async < Route extends RoutePattern, Method extends HTTPMethod | HTTPMethod[], Config extends EndpointSchemas = EndpointSchemas, >( context: RequestContext<EndpointMeta<Route, Method, Config>>, use: MiddlewareFunction<Route, Method, Config>[] = [] ) => { - 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<EndpointMeta<Route, Method, 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") + 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<EndpointMeta<Route, Method, Config>> + } catch (error) { + if (error instanceof RouterError) throw error + throw new RouterError("BAD_REQUEST", "Handler threw an error", "MiddlewareError") + } } + return ctx }🤖 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 54 - 70, The outer catch currently swallows specific RouterError instances and replaces them with a generic error; modify the final catch to capture the thrown error (catch (error)) and rethrow it if it is an instance of RouterError, otherwise wrap it in a new RouterError. Locate the middleware execution block that uses variables ctx, context, use and the RouterError class; change the trailing catch { ... } to catch (error) { if (error instanceof RouterError) throw error; throw new RouterError("BAD_REQUEST", "Handler threw an error"); } so original RouterError messages like "Middleware must be a function" propagate unchanged.
🤖 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 167-182: The Config generic for RouteHandler and RouteEndpoint is
too loose and can be mismatched with Method; update both declarations so Config
is bound to the same Method generic (i.e. change Config extends
EndpointConfig<Route, any, any> to Config extends EndpointConfig<Route, Method,
any>) so middleware/use types in EndpointConfig are correctly inferred for the
endpoint method; update the RouteHandler and RouteEndpoint signatures
(referencing the RouteHandler, RouteEndpoint and EndpointConfig symbols) to use
this tightened constraint and keep other generics the same.
---
Outside diff comments:
In `@bench/router.bench.ts`:
- Around line 8-15: The benchmark generates duplicate routes because the
Array.from callback currently receives the element (undefined) as the first arg,
so "idx" is undefined; change the mapping callback to accept the index (e.g.,
use (_, idx) => ...) so the index is used for route and response generation in
the endpoints array; update occurrences inside the object created for each
RouteEndpoint (route `/endpoint-${idx}`, handler response `Endpoint ${idx}`, and
any config references) so each entry uses the actual numeric index rather than
the undefined element.
In `@src/`@types/client.ts:
- Around line 55-69: The Client type's top-level conditional uses "Endpoints
extends unknown[]" which fails to match readonly tuples (e.g., as const) and
causes Client<readonly [...]> to resolve to {}; change the conditional to
"Endpoints extends readonly unknown[]" so the recursive tuple mapping runs for
readonly endpoint tuples; update the Client declaration that references
Endpoints and RouteEndpoint<...> accordingly to use the readonly check.
In `@src/context.ts`:
- Around line 131-165: getBody currently returns null for unsupported methods
and when no body is parsed (seen in the early return after isSupportedBodyMethod
and the final return), but RequestContext.body is typed/published as undefined
for schema-less endpoints; update getBody to return undefined instead of null in
both places so the runtime matches the exported type (adjust the Promise return
type if needed), and ensure callers in router.ts that forward the value continue
to accept undefined. Reference symbols: getBody, isSupportedBodyMethod,
RequestContext.body and the router.ts forwarding logic.
In `@src/middlewares.ts`:
- Around line 54-70: The outer catch currently swallows specific RouterError
instances and replaces them with a generic error; modify the final catch to
capture the thrown error (catch (error)) and rethrow it if it is an instance of
RouterError, otherwise wrap it in a new RouterError. Locate the middleware
execution block that uses variables ctx, context, use and the RouterError class;
change the trailing catch { ... } to catch (error) { if (error instanceof
RouterError) throw error; throw new RouterError("BAD_REQUEST", "Handler threw an
error"); } so original RouterError messages like "Middleware must be a function"
propagate unchanged.
🪄 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: 46f4c49a-365e-42c2-9f46-e68d37f6333d
📒 Files selected for processing (12)
.vscode/extensions.json.vscode/settings.jsonbench/router.bench.tssrc/@types/client.tssrc/@types/types.tssrc/assert.tssrc/context.tssrc/endpoint.tssrc/middlewares.tstest/client.test.tstest/middlewares.test.tstest/type.test-d.ts
✅ Files skipped from review due to trivial changes (2)
- .vscode/extensions.json
- .vscode/settings.json
This was referenced Jun 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request optimizes deeply nested type computations across the library by reducing generic propagation and removing unnecessary uses of the
Prettifyutility type within endpoint, middleware, and request context definitions.This work was initially motivated by issues discovered in
@aura-stack/auth, where several consumers encountered the following TypeScript compiler error:The primary goal of this PR is to improve compiler performance, reduce type complexity, and make type inference more predictable when working with complex endpoint definitions and schema validators.
One of the first optimizations was reducing the usage of the
Prettifyutility type, which was heavily used to flatten complex inferred types such asRequestContextin handlers and middlewares. While this utility improves type readability, it also introduces additional type computations that contribute to excessive type instantiation.However, further investigation revealed that
Prettifywas only a small part of the problem.Key Changes
Prettifyutility typeHistory
While working on this PR, several important observations were made regarding schema validator support within Aura Router.
Over the last few releases, Aura Router has added support for multiple schema validation libraries, including:
During this process, we identified that some utility types provided by these libraries introduce significant compiler overhead. Previous investigations revealed issues with types such as:
InferOutputfrom ValibotStaticfrom TypeBoxTo mitigate the Valibot issue, we introduced custom inference utilities such as
InferValibotSchema, which was added in #45 and provides equivalent behavior with substantially lower type complexity.Initially, we believed these optimizations had resolved the majority of the performance issues. However, additional profiling revealed a much larger source of complexity.
Using:
we compared compiler metrics before and after introducing TypeBox's
Statictype and observed a dramatic increase in type-checking costs.Compiler Metrics Comparison
StaticStaticRaw Output
These results indicate that the TypeBox
Statictype is currently one of the largest contributors to type instantiation growth within the project.As a result, future work will focus on evaluating alternatives, reducing dependency on
Static, or potentially revisiting the current TypeBox integration strategy if a sustainable solution cannot be found.In short, while
Prettifycontributed to the issue, the investigation showed thatStaticis responsible for a significantly larger portion of the compiler overhead.Summary by CodeRabbit
Refactoring
Chores