fix: parent param types in server routes #5482
Merged
+140
−29
Conversation
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
WalkthroughAdds two new server routes (/api/params/$foo and /api/params/$foo/$bar) with generated route tree updates. Refactors server route type generics to use TFullPath and TParams and switches param resolution to ResolveAllParamsFromParent. Adjusts type signatures across serverRoute.ts and updates a related type annotation in createStartHandler.ts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Router as React Router (Server)
participant Matcher as Route Matcher
participant Handler as Route Handler (GET)
note over Router,Handler: Param resolution now uses ResolveAllParamsFromParent
Client->>Router: HTTP GET /api/params/$foo/$bar
Router->>Matcher: Match route by fullPath
Matcher-->>Router: Matched route + parent chain
Router->>Handler: Invoke GET with ctx { params: resolved from parents }
Handler-->>Router: Response("hello, foo and bar")
Router-->>Client: HTTP 200 text/plain
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 2b52abe
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/start-client-core/src/serverRoute.ts (2)
134-186: Double-check TPath vs TFullPath handoffRouteServerOptions expects TFullPath, but FilebaseRouteOptionsInterface passes TPath into it. If router-core’s FilebaseRouteOptionsInterface exposes both TPath and TFullPath generics, consider passing TFullPath here; otherwise, confirm that TPath indeed represents the full path at this point.
115-132: Prefer unknown over any in conditional inference pointsUsing unknown instead of any in ExtractHandlersContext preserves type safety without losing inference.
Apply if desired:
-) => CustomHandlerFunctionsRecord< - any, - any, - any, - any, - any, - any, - infer TServerContext -> +) => CustomHandlerFunctionsRecord< + unknown, + unknown, + unknown, + unknown, + unknown, + unknown, + infer TServerContext +> @@ - RouteMethodHandler<any, any, any, any, any, any, infer TServerContext> + RouteMethodHandler<unknown, unknown, unknown, unknown, unknown, unknown, infer TServerContext>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
e2e/react-start/server-routes/src/routeTree.gen.ts(3 hunks)e2e/react-start/server-routes/src/routes/api/params/$foo/$bar.ts(1 hunks)e2e/react-start/server-routes/src/routes/api/params/$foo/route.ts(1 hunks)packages/start-client-core/src/serverRoute.ts(18 hunks)packages/start-server-core/src/createStartHandler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/start-server-core/src/createStartHandler.tse2e/react-start/server-routes/src/routes/api/params/$foo/$bar.tse2e/react-start/server-routes/src/routes/api/params/$foo/route.tspackages/start-client-core/src/serverRoute.tse2e/react-start/server-routes/src/routeTree.gen.ts
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/start-server-core/src/createStartHandler.tspackages/start-client-core/src/serverRoute.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/server-routes/src/routes/api/params/$foo/$bar.tse2e/react-start/server-routes/src/routes/api/params/$foo/route.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/server-routes/src/routes/api/params/$foo/$bar.tse2e/react-start/server-routes/src/routes/api/params/$foo/route.tse2e/react-start/server-routes/src/routeTree.gen.ts
🧬 Code graph analysis (4)
packages/start-server-core/src/createStartHandler.ts (1)
packages/start-client-core/src/serverRoute.ts (1)
RouteMethodHandlerFn(437-456)
e2e/react-start/server-routes/src/routes/api/params/$foo/$bar.ts (1)
e2e/react-start/server-routes/src/routes/api/params/$foo/route.ts (1)
Route(3-11)
e2e/react-start/server-routes/src/routes/api/params/$foo/route.ts (1)
e2e/react-start/server-routes/src/routes/api/params/$foo/$bar.ts (1)
Route(3-11)
e2e/react-start/server-routes/src/routeTree.gen.ts (2)
e2e/react-start/server-functions/src/routeTree.gen.ts (5)
FileRoutesByFullPath(149-172)FileRoutesByTo(173-196)FileRoutesById(197-221)FileRouteTypes(222-296)RootRouteChildren(297-320)examples/react/start-basic/src/routeTree.gen.ts (5)
FileRoutesByFullPath(117-133)FileRoutesByTo(134-148)FileRoutesById(149-168)FileRouteTypes(169-222)RootRouteChildren(223-233)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (5)
e2e/react-start/server-routes/src/routes/api/params/$foo/route.ts (1)
3-11: LGTM: parent param is consumed in handlerHandler reads params.foo as intended; good coverage for the fix path.
e2e/react-start/server-routes/src/routes/api/params/$foo/$bar.ts (1)
3-11: LGTM: nested param merging is exercisedHandler reads both params.foo and params.bar, matching the new ResolveAllParamsFromParent behavior.
e2e/react-start/server-routes/src/routeTree.gen.ts (1)
34-44: Generated route wiring looks consistent
- ApiParamsFooRouteRoute at /api/params/$foo under root.
- ApiParamsFooBarRoute with id/path '/$bar' under ApiParamsFooRouteRoute and fullPath '/api/params/$foo/$bar'.
- Public typings updated (fullPaths, to, id, RootRouteChildren).
No issues spotted.
Also applies to: 45-66, 121-134, 146-154
packages/start-client-core/src/serverRoute.ts (1)
484-486: Param typing fix approved; ResolveParams no longer used
Confirm TS compilation and that route handlers can access merged params (e.g.,params.foo/params.bar) without errors.packages/start-server-core/src/createStartHandler.ts (1)
455-456: Generic arity update is correct
All RouteMethodHandlerFn<> usages—including those in serverRoute.ts—now use seven type parameters; no further changes required.
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.
fixes #5212
Summary by CodeRabbit