fix: server middleware type in solid-router#7260
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdjusts TypeScript typing for file-route middleware inference by making Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
View your CI Pipeline Execution ↗ for commit bc40c09
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview1 package(s) bumped directly, 3 bumped as dependents. 🟩 Patch bumps
|
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/solid-router/tests/fileRoute.test-d.tsx`:
- Around line 152-163: Remove the Git conflict markers and the createMiddleware
variant and keep the mocked middleware from the "Stashed changes" branch: update
the test named test(...) to the "when creating a file route with middleware
options" variant, define the local Middleware type and the const middleware as
in the stashed version (so the later assertion toEqualTypeOf<readonly
[Middleware]>() matches), and delete any references to createMiddleware or the
server middleware branch; ensure there are no remaining <<<<<<<, =======, or
>>>>>>> markers so the file compiles.
🪄 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: e2038ed4-6e22-493c-8e1b-c9241aaa5870
📒 Files selected for processing (1)
packages/solid-router/tests/fileRoute.test-d.tsx
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We resolved unresolved git merge conflict markers left in packages/solid-router/tests/fileRoute.test-d.tsx that were causing a SyntaxError at parse time, failing the unit test run. Our fix selects the correct conflict side — a self-contained type-level test using a simple as const object — which directly validates the const TMiddlewares type parameter fix introduced in fileRoute.ts.
Tip
✅ We verified this fix by re-running @tanstack/solid-router:test:eslint, @tanstack/solid-router:test:unit.
Suggested Fix changes
diff --git a/packages/solid-router/tests/fileRoute.test-d.tsx b/packages/solid-router/tests/fileRoute.test-d.tsx
index 46f74e2128..6c32156b2d 100644
--- a/packages/solid-router/tests/fileRoute.test-d.tsx
+++ b/packages/solid-router/tests/fileRoute.test-d.tsx
@@ -149,18 +149,9 @@ test('when creating a folder group', () => {
expectTypeOf<'/protected'>(protectedRoute.id)
})
-<<<<<<< Updated upstream
-test('when creating a file route with server middleware', () => {
- const middleware = createMiddleware({ type: 'request' }).server(
- ({ next }) => {
- return next({ context: { supportsGzip: true } })
- },
- )
-=======
test('when creating a file route with middleware options', () => {
type Middleware = { readonly middleware: 'middleware' }
const middleware = { middleware: 'middleware' } as const
->>>>>>> Stashed changes
const route = createFileRoute('/invoices')({
server: {
@@ -168,28 +159,29 @@ test('when creating a file route with middleware options', () => {
},
})
- type ExtractMiddlewares<TRoute> = TRoute extends Route<
- any,
- any,
- any,
- any,
- any,
- any,
- any,
- any,
- any,
- any,
- any,
- any,
- any,
- any,
- any,
- any,
- infer TMiddlewares,
- any
- >
- ? TMiddlewares
- : never
+ type ExtractMiddlewares<TRoute> =
+ TRoute extends Route<
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ any,
+ infer TMiddlewares,
+ any
+ >
+ ? TMiddlewares
+ : never
expectTypeOf<ExtractMiddlewares<typeof route>>().toEqualTypeOf<
readonly [Middleware]
Or Apply changes locally with:
npx nx-cloud apply-locally VFRC-IpPd
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/solid-router/tests/fileRoute.test-d.tsx (2)
152-160: Consider also asserting the original issue's runtime shape (context propagation).Issue
#7092is specifically aboutcontextreturned from a server middleware being inferred asundefinedin a child route's server handler. This test only verifies that themiddlewarearray type round-trips throughRoute— it does not exercise a middleware whosenext({ context: {...} })output should flow into a child handler'scontextparameter, which is the actual symptom users reported. A complementaryexpectTypeOfon the inferredcontextparameter of a child route's server handler (usingcreateMiddleware(...).server(({ next }) => next({ context: { supportsGzip: true } }))) would more directly prevent regression of the linked issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/solid-router/tests/fileRoute.test-d.tsx` around lines 152 - 160, Add a complementary type-level assertion that verifies context propagation from middleware into a child route handler: create a middleware using createMiddleware(...).server(({ next }) => next({ context: { supportsGzip: true } })), attach it to createFileRoute('/invoices') via the server.middleware array, then use expectTypeOf on the child route's server handler context parameter to assert it is inferred as { supportsGzip: boolean } (or the exact type you used); this will directly test that the middleware's next({ context: ... }) output flows into the child route's server handler context and prevent regression of issue `#7092`.
162-183: Position-basedRoute<...>inference is brittle, but the proposed refactor isn't feasible.
ExtractMiddlewaresrelies on positional extraction, which is fragile: any reordering or insertion of generics inRoutecould silently break the type. However, the current implementation is correct—TServerMiddlewaresis the 17th generic (with 16anys before it, not 17), and the assertion works as intended.The suggested refactor using
T['types']['middleware']isn't viable becauseRouteTypesdoesn't expose amiddlewareproperty, despite carrying theTServerMiddlewarestype parameter. While the brittleness concern is valid, a safer approach would be to document the dependency clearly or consider whether this level of type-testing complexity is necessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/solid-router/tests/fileRoute.test-d.tsx` around lines 162 - 183, ExtractMiddlewares relies on positional inference from Route to pull out the TServerMiddlewares generic and the current implementation is correct (TMiddlewares is the 17th generic after 16 `any`s); revert any refactor that replaced this with T['types']['middleware'] and instead add an inline comment/documentation near the ExtractMiddlewares type and/or Route definition explaining the positional dependency, referencing Route, ExtractMiddlewares, RouteTypes and the TServerMiddlewares generic so future maintainers understand why positional inference is used and why RouteTypes does not expose a middleware property.
🤖 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/solid-router/tests/fileRoute.test-d.tsx`:
- Around line 5-50: The test declares module augmentations for
FilebaseRouteOptionsInterface and RouteTypes which hides a missing production
augmentation; move these declarations out of the test and into the solid-router
source (e.g., add them to packages/solid-router/src/fileRoute.ts or a new
augmentation file in that package) so that the server?: { middleware?:
TServerMiddlewares } property on FilebaseRouteOptionsInterface and the
middleware: TServerMiddlewares field on RouteTypes are exported as real library
type augmentations and available to consumers; update exports or add a .d.ts
augmentation file within the solid-router package containing the same interface
augmentations and remove the test-local declarations.
---
Nitpick comments:
In `@packages/solid-router/tests/fileRoute.test-d.tsx`:
- Around line 152-160: Add a complementary type-level assertion that verifies
context propagation from middleware into a child route handler: create a
middleware using createMiddleware(...).server(({ next }) => next({ context: {
supportsGzip: true } })), attach it to createFileRoute('/invoices') via the
server.middleware array, then use expectTypeOf on the child route's server
handler context parameter to assert it is inferred as { supportsGzip: boolean }
(or the exact type you used); this will directly test that the middleware's
next({ context: ... }) output flows into the child route's server handler
context and prevent regression of issue `#7092`.
- Around line 162-183: ExtractMiddlewares relies on positional inference from
Route to pull out the TServerMiddlewares generic and the current implementation
is correct (TMiddlewares is the 17th generic after 16 `any`s); revert any
refactor that replaced this with T['types']['middleware'] and instead add an
inline comment/documentation near the ExtractMiddlewares type and/or Route
definition explaining the positional dependency, referencing Route,
ExtractMiddlewares, RouteTypes and the TServerMiddlewares generic so future
maintainers understand why positional inference is used and why RouteTypes does
not expose a middleware property.
🪄 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: 288496e4-f6ea-4eb3-a1ae-b5bef4152827
📒 Files selected for processing (1)
packages/solid-router/tests/fileRoute.test-d.tsx
| declare module '@tanstack/router-core' { | ||
| interface FilebaseRouteOptionsInterface< | ||
| TRegister, | ||
| TParentRoute, | ||
| TId, | ||
| TPath, | ||
| TSearchValidator, | ||
| TParams, | ||
| TLoaderDeps, | ||
| TLoaderFn, | ||
| TRouterContext, | ||
| TRouteContextFn, | ||
| TBeforeLoadFn, | ||
| TRemountDepsFn, | ||
| TSSR, | ||
| TServerMiddlewares, | ||
| THandlers, | ||
| > { | ||
| server?: { | ||
| middleware?: TServerMiddlewares | ||
| } | ||
| } | ||
|
|
||
| interface RouteTypes< | ||
| TRegister, | ||
| TParentRoute, | ||
| TPath, | ||
| TFullPath, | ||
| TCustomId, | ||
| TId, | ||
| TSearchValidator, | ||
| TParams, | ||
| TRouterContext, | ||
| TRouteContextFn, | ||
| TBeforeLoadFn, | ||
| TLoaderDeps, | ||
| TLoaderFn, | ||
| TChildren, | ||
| TFileRouteTypes, | ||
| TSSR, | ||
| TServerMiddlewares, | ||
| THandlers, | ||
| > { | ||
| middleware: TServerMiddlewares | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether FilebaseRouteOptionsInterface / RouteTypes augmentations exist in production solid-router sources.
fd -t f -e ts -e tsx . packages/solid-router/src | xargs rg -nP "declare module '@tanstack/router-core'|FilebaseRouteOptionsInterface|RouteTypes\b" -C2Repository: TanStack/router
Length of output: 5244
🏁 Script executed:
# Read the full module augmentation in fileRoute.ts
cat -n packages/solid-router/src/fileRoute.ts | sed -n '182,220p'Repository: TanStack/router
Length of output: 1419
🏁 Script executed:
# Search for FilebaseRouteOptionsInterface augmentation in production
rg -nP "FilebaseRouteOptionsInterface" packages/solid-router/srcRepository: TanStack/router
Length of output: 41
🏁 Script executed:
# Search for FilebaseRouteOptionsInterface augmentation in router-core package
fd -type f -e ts -e tsx . packages/router-core | xargs rg -nP "FilebaseRouteOptionsInterface|declare module.*@tanstack/router-core" -C3Repository: TanStack/router
Length of output: 14521
🏁 Script executed:
# Also check what's exported from packages/router-core/src/fileRoute.ts if it exists
cat -n packages/router-core/src/fileRoute.ts 2>/dev/null | head -100Repository: TanStack/router
Length of output: 3102
🏁 Script executed:
# Read the full FilebaseRouteOptionsInterface definition in router-core
cat -n packages/router-core/src/route.ts | sed -n '944,1000p'Repository: TanStack/router
Length of output: 1750
🏁 Script executed:
# Check if FilebaseRouteOptionsInterface has any augmentation in solid-router production code
rg -nP "declare module|server\?" packages/solid-router/src/fileRoute.ts packages/solid-router/src/route.tsx -C5Repository: TanStack/router
Length of output: 1785
🏁 Script executed:
# Check what the start-client-core augmentation looks like for FilebaseRouteOptionsInterface
cat -n packages/start-client-core/src/serverRoute.ts | sed -n '15,80p'Repository: TanStack/router
Length of output: 2161
Move module augmentations to solid-router production code.
The FilebaseRouteOptionsInterface and RouteTypes augmentations in this test file are test-only declarations. While start-client-core (the full-stack framework) includes these augmentations in production, solid-router (the routing library) does not. This means:
- The test passes because it declares the types locally
- End users of
@tanstack/solid-routerwon't receiveserver.middlewaretyping support - The test doesn't actually validate what consumers experience
Move these augmentations to packages/solid-router/src/fileRoute.ts or a dedicated augmentation file so that users of solid-router get proper type support for server middleware.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/solid-router/tests/fileRoute.test-d.tsx` around lines 5 - 50, The
test declares module augmentations for FilebaseRouteOptionsInterface and
RouteTypes which hides a missing production augmentation; move these
declarations out of the test and into the solid-router source (e.g., add them to
packages/solid-router/src/fileRoute.ts or a new augmentation file in that
package) so that the server?: { middleware?: TServerMiddlewares } property on
FilebaseRouteOptionsInterface and the middleware: TServerMiddlewares field on
RouteTypes are exported as real library type augmentations and available to
consumers; update exports or add a .d.ts augmentation file within the
solid-router package containing the same interface augmentations and remove the
test-local declarations.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/solid-router/tests/fileRoute.test-d.tsx (1)
5-50:⚠️ Potential issue | 🔴 CriticalModule augmentations belong in solid-router production sources, not the test file.
The
FilebaseRouteOptionsInterfaceandRouteTypesaugmentations declared here are what enableserver.middlewareto be inferred and surfaced on the route type. Because they live in the test file, the test will pass but consumers of@tanstack/solid-routerwill not get this typing — which is precisely the bug reported in#7092(compare withstart-client-core/src/serverRoute.ts, which augments these interfaces in production). The const-generic change toTMiddlewaresinfileRoute.tsonly fixes inference; without the augmentations being shipped, end-userRoutetypes still won't expose the middleware-derivedcontext.Please move these
declare module '@tanstack/router-core'blocks intopackages/solid-router/src/fileRoute.ts(or a dedicated.d.ts) and keep the test file consuming them as a real library user would. Otherwise this PR validates a fix that consumers won't actually receive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/solid-router/tests/fileRoute.test-d.tsx` around lines 5 - 50, Move the module augmentation that declares FilebaseRouteOptionsInterface and RouteTypes out of the test and into the library sources so consumers get the types: add the declare module '@tanstack/router-core' block (the augmentations that add server?: { middleware?: TServerMiddlewares } to FilebaseRouteOptionsInterface and middleware: TServerMiddlewares to RouteTypes) into packages/solid-router/src/fileRoute.ts (or a new .d.ts in the package), ensure it lives alongside the fileRoute.ts changes that introduced the const-generic TMiddlewares and references server.middleware, and keep the test file unchanged so it consumes the augmentation as an external user would.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/solid-router/tests/fileRoute.test-d.tsx`:
- Around line 5-50: Move the module augmentation that declares
FilebaseRouteOptionsInterface and RouteTypes out of the test and into the
library sources so consumers get the types: add the declare module
'@tanstack/router-core' block (the augmentations that add server?: {
middleware?: TServerMiddlewares } to FilebaseRouteOptionsInterface and
middleware: TServerMiddlewares to RouteTypes) into
packages/solid-router/src/fileRoute.ts (or a new .d.ts in the package), ensure
it lives alongside the fileRoute.ts changes that introduced the const-generic
TMiddlewares and references server.middleware, and keep the test file unchanged
so it consumes the augmentation as an external user would.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c18c985-7d53-4628-bc0d-f35ae8d07db2
📒 Files selected for processing (1)
packages/solid-router/tests/fileRoute.test-d.tsx
2 participants
Closes #7092
Summary by CodeRabbit
Bug Fixes
Tests
Chores