feat(types): speed up typed routes #3941
Conversation
|
@dannote is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughRestructures the route typing system from a flat, static approach to a hierarchical tree-based model. Introduces path normalization, exact and loose matching strategies, and nested route schemas to support better type safety and improved code organization. Changes span type generation, route resolution logic, and test infrastructure. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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
🤖 Fix all issues with AI agents
In @src/build/types.ts:
- Around line 146-152: The generated method entries for node.$ are missing
trailing semicolons; update the code that pushes method lines (the loop using
lines.push(`${childIndent}'${method}': ${types.join(" | ")}`)) to append a
semicolon so each entry becomes `${childIndent}'${method}': ${types.join(" |
")};`, ensuring valid TypeScript in the emitted declarations. Ensure the change
is applied where indent/childIndent and node.$ are handled so all rendered
method lines include the semicolon.
- Around line 171-185: The generateRoutes function currently injects a local
Awaited type declaration that shadows TypeScript's built-in Awaited; remove the
redundant "type Awaited..." line from the generated output in generateRoutes so
the module augmentation relies on the built-in utility instead—update
generateRoutes (which builds routeTree via buildRouteTree and renders with
renderRouteTree) to omit the manual Awaited declaration while keeping the rest
of the declared module and exported empty module.
🧹 Nitpick comments (2)
test/fixture/typed-routes.types.ts (1)
39-53: Consider adding a test fordefaultmethod handling.The test coverage is solid for explicit HTTP methods, but the
AvailableRouterMethodlogic infetch.tshas special handling for the"default"method case. Consider adding a test case with a route that has adefaultmethod to verify theRouterMethodfallback behavior.src/types/fetch/fetch.ts (1)
11-29: DuplicateJoinPathandRootPathtype utilities.
JoinPath(lines 13-15) andRootPath(line 17) are duplicated insrc/types/fetch/_match.ts(lines 38-42). While this may be intentional to avoid circular dependencies (since_match.tsimports fromfetch.ts), consider extracting these shared utilities to a common module like_utils.tsto improve maintainability.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/build/types.tssrc/types/fetch/_match.tssrc/types/fetch/fetch.tstest/fixture/typed-routes.types.ts
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,js,tsx,jsx}: Usepathefor cross-platform path operations instead of Node.jsnode:path
Use ESM and modern JavaScript
Do not add comments explaining what the line does unless prompted
Files:
src/build/types.tssrc/types/fetch/fetch.tssrc/types/fetch/_match.ts
src/{build,dev,runner,cli}/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use
consolafor logging in build/dev code; usenitro.loggerwhen available
Files:
src/build/types.ts
src/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use
unstoragefor storage abstraction
Files:
src/build/types.tssrc/types/fetch/fetch.tssrc/types/fetch/_match.ts
src/build/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
Virtual modules must be registered in
src/build/virtual.ts
Files:
src/build/types.ts
🧠 Learnings (1)
📚 Learning: 2025-12-24T11:45:17.435Z
Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/runtime/**/*.{ts,js} : In runtime code, prefer Web APIs (fetch, Request, Response, URL, etc.) over Node.js APIs
Applied to files:
src/types/fetch/fetch.ts
🧬 Code graph analysis (3)
test/fixture/typed-routes.types.ts (1)
src/types/fetch/fetch.ts (3)
InternalApi(6-6)TypedInternalResponse(45-59)AvailableRouterMethod(63-70)
src/types/fetch/fetch.ts (1)
src/types/fetch/_match.ts (1)
MatchedRouteMethods(134-137)
src/types/fetch/_match.ts (2)
src/presets/cloudflare/wrangler/environment.ts (1)
Route(30-34)src/types/fetch/fetch.ts (1)
InternalApi(6-6)
🪛 Biome (2.1.2)
src/types/fetch/fetch.ts
[error] 6-6: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🔇 Additional comments (12)
src/build/types.ts (2)
111-136: LGTM!The
buildRouteTreefunction correctly constructs a nested route tree from the flattypes.routesmap. The handling of the root path/as an empty segments array and the method bucket$at each leaf node aligns well with the PR's objective of O(path depth) matching.
154-166: Missing semicolons after nested object properties.Similar to the method entries, the nested segment properties should have trailing semicolons for valid TypeScript interface syntax.
🐛 Proposed fix
for (const [segment, child] of Object.entries(node)) { if (segment === "$" || !child || typeof child !== "object") { continue; } lines.push(`${indent}'${segment}': {`); lines.push( ...renderRouteTree( child as ReturnType<typeof buildRouteTree>, level + 1 ) ); - lines.push(`${indent}}`); + lines.push(`${indent}};`); }Also update the
$object closing (line 151):- lines.push(`${indent}}`); + lines.push(`${indent}};`);Likely an incorrect or invalid review comment.
test/fixture/typed-routes.types.ts (1)
8-31: LGTM!The module augmentation correctly demonstrates the new nested
InternalApistructure with the$method bucket pattern. The test cases comprehensively cover exact routes, parameterized routes (:id), and catch-all routes (**), which validates the matching logic in_match.ts.src/types/fetch/fetch.ts (3)
5-6: Empty interface is intentional for module augmentation.The static analysis tool flagged this as equivalent to
{}. However, this empty interface is intentionally designed as an extension point for module augmentation (as demonstrated in the test fixture). This pattern is idiomatic for Nitro's type system. No change needed.
19-29: LGTM!The
RoutePathstype correctly extracts all valid route paths from the nestedInternalApistructure. The recursive logic withStripMethodsKeyproperly excludes the$method bucket from path segments while still recognizing nodes with methods as valid endpoints.
38-43: LGTM!The
MiddlewareOfandAvailableRouterMethodtypes correctly use the newMatchedRouteMethods<Route>instead of the previousInternalApi[MatchedRoutes<Route>]pattern. The conditional logic for method extraction and default fallback is preserved correctly.Also applies to: 62-70
src/types/fetch/_match.ts (6)
3-9: LGTM!The
MatchResultandRouteMethodstypes provide a clean foundation for the matching system. Theexactboolean flag inMatchResultenables prioritization of exact matches, andRouteMethodscorrectly extracts the method bucket using conditional type inference.
11-29: LGTM!The path normalization utilities correctly strip query strings, hash fragments, and leading/trailing slashes in the proper order. The
NormalizePathcomposition ensures consistent path handling for matching.
47-57: LGTM!The
ExactMatchtype correctly implements strict segment-by-segment matching. It only succeeds when all path segments exist as literal keys in the route tree and the final node contains a$method bucket.
59-125: LGTM!The
LooseMatchandLooseMatchSteptypes implement a sophisticated fallback matching algorithm. The approach correctly:
- Tracks potential fallback matches (nodes with
$at intermediate levels)- Accumulates catch-all matches as the algorithm descends
- Prioritizes exact segment matches over params over catch-alls
- Returns a union of all valid matches for TypeScript to resolve
The priority ordering (exact → param → catch-all) aligns with the PR objectives.
127-132: LGTM!The
MatchRoutetype correctly orchestrates the matching strategy by tryingExactMatchfirst and falling back toLooseMatchonly when no exact match exists. The[Exact] extends [never]pattern is the idiomatic way to check for thenevertype in TypeScript.
134-139: LGTM!The
MatchedRouteMethodsexport provides the new API whileMatchedRoutesis retained as an alias for backward compatibility. This is a clean approach that allows gradual migration.
| if (node.$) { | ||
| lines.push(`${indent}'$': {`); | ||
| for (const [method, types] of Object.entries(node.$)) { | ||
| lines.push(`${childIndent}'${method}': ${types.join(" | ")}`); | ||
| } | ||
| lines.push(`${indent}}`); | ||
| } |
There was a problem hiding this comment.
Missing semicolon in generated TypeScript code.
The rendered method entries are missing trailing semicolons, which will produce invalid TypeScript syntax in the generated nitro-routes.d.ts file.
🐛 Proposed fix
if (node.$) {
lines.push(`${indent}'$': {`);
for (const [method, types] of Object.entries(node.$)) {
- lines.push(`${childIndent}'${method}': ${types.join(" | ")}`);
+ lines.push(`${childIndent}'${method}': ${types.join(" | ")};`);
}
lines.push(`${indent}}`);
}📝 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.
| if (node.$) { | |
| lines.push(`${indent}'$': {`); | |
| for (const [method, types] of Object.entries(node.$)) { | |
| lines.push(`${childIndent}'${method}': ${types.join(" | ")}`); | |
| } | |
| lines.push(`${indent}}`); | |
| } | |
| if (node.$) { | |
| lines.push(`${indent}'$': {`); | |
| for (const [method, types] of Object.entries(node.$)) { | |
| lines.push(`${childIndent}'${method}': ${types.join(" | ")};`); | |
| } | |
| lines.push(`${indent}}`); | |
| } |
🤖 Prompt for AI Agents
In @src/build/types.ts around lines 146 - 152, The generated method entries for
node.$ are missing trailing semicolons; update the code that pushes method lines
(the loop using lines.push(`${childIndent}'${method}': ${types.join(" | ")}`))
to append a semicolon so each entry becomes `${childIndent}'${method}':
${types.join(" | ")};`, ensuring valid TypeScript in the emitted declarations.
Ensure the change is applied where indent/childIndent and node.$ are handled so
all rendered method lines include the semicolon.
| const generateRoutes = () => { | ||
| const routeTree = buildRouteTree(); | ||
| return [ | ||
| "// Generated by nitro", | ||
| 'import type { Serialize, Simplify } from "nitro/types";', | ||
| 'declare module "nitro/types" {', | ||
| " type Awaited<T> = T extends PromiseLike<infer U> ? Awaited<U> : T", | ||
| " interface InternalApi {", | ||
| ...renderRouteTree(routeTree, 2), | ||
| " }", | ||
| "}", | ||
| // Makes this a module for augmentation purposes | ||
| "export {}", | ||
| ]; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file to see the full context
head -200 src/build/types.ts | tail -100Repository: nitrojs/nitro
Length of output: 2816
🏁 Script executed:
# Check TypeScript version requirements in package.json
cat package.json | grep -A 5 '"typescript"'Repository: nitrojs/nitro
Length of output: 207
🏁 Script executed:
# Check tsconfig.json for TypeScript version target
cat tsconfig.json | grep -E '(compilerOptions|target)' -A 5Repository: nitrojs/nitro
Length of output: 254
🏁 Script executed:
# Search for any comments or documentation about the Awaited type
rg "Awaited" --context 5Repository: nitrojs/nitro
Length of output: 2411
Remove redundant Awaited type declaration.
The local Awaited type at line 177 shadows TypeScript's built-in utility type. Since the project requires TypeScript ^5.9.3 (which includes the built-in Awaited), this local redeclaration is unnecessary and should be removed. The type will remain available within the module augmentation scope without the redundant definition.
🤖 Prompt for AI Agents
In @src/build/types.ts around lines 171 - 185, The generateRoutes function
currently injects a local Awaited type declaration that shadows TypeScript's
built-in Awaited; remove the redundant "type Awaited..." line from the generated
output in generateRoutes so the module augmentation relies on the built-in
utility instead—update generateRoutes (which builds routeTree via buildRouteTree
and renders with renderRouteTree) to omit the manual Awaited declaration while
keeping the rest of the declared module and exported empty module.
1 participant
Summary
This switches typed route generation + matching to a nested route tree, so
$fetchtypes walk path segments instead of scoring every route key. It keeps method narrowing intact while avoiding the deep type instantiation that slows or breaks TypeScript on larger APIs.What changed
InternalApiwith a$method bucket at each leaf (src/build/types.ts).src/types/fetch/_match.ts).NitroFetchRequestas a union of nested route paths and use matched method maps for$fetchnarrowing (src/types/fetch/fetch.ts).test/fixture/typed-routes.types.ts).Why this helps
The previous flat route map required comparing every route on each
$fetchcall (O(n x path depth)). The nested tree resolves in O(path depth) and dramatically reduces TypeScript instantiations.Related issues
Excessive stack depth comparing typeswhen trying to wrap$fetch#470fetch/$fetchand routes #2758Elysia/Eden inspiration
The nested route tree idea is inspired by Elysia's
CreateEdenand Eden's path mapping utilities:Tests
pnpm lint:fixpnpm test:typespnpm test(fails intest:rollupwith Vitest unhandledECONNRESET/ "Server is not running";test:rolldowndid not run)