Skip to content

Conversation

schiller-manuel
Copy link
Contributor

@schiller-manuel schiller-manuel commented Oct 6, 2025

…th server function

fixes #5384

Summary by CodeRabbit

  • New Features

    • Added a Middleware Request page at /middleware/request-middleware, linked from the Middleware section.
    • Displays request details from the loader and from a client-triggered server call; includes a button to re-fetch data.
  • Refactor

    • Improved server request context propagation for more consistent behavior across routes and middleware.
  • Tests

    • Added end-to-end tests verifying loader and client results match when using request middleware with server functions.

Copy link
Contributor

coderabbitai bot commented Oct 6, 2025

Walkthrough

Adds a new request-middleware demo route and navigation, implements request propagation through Start context, replaces a server-context getter with a server-only context accessor, updates server function middleware resolution, and adds tests validating that request is available in request middleware for both loader and client calls.

Changes

Cohort / File(s) Summary
Request propagation plumbing
packages/start-server-core/src/createStartHandler.ts, packages/start-storage-context/src/async-local-storage.ts, packages/start-client-core/src/getStartContextServerOnly.ts, packages/start-client-core/src/getServerContextAfterGlobalMiddlewares.ts, packages/start-client-core/src/createServerFn.ts
Propagates the current Request into Start context; extends StartStorageContext with request; introduces getStartContextServerOnly; removes getServerContextAfterGlobalMiddlewares; updates server function to read contextAfterGlobalMiddlewares and request from the new server-only context and clarifies client/server middleware selection.
Routes and UI
e2e/react-start/server-functions/src/routeTree.gen.ts, e2e/react-start/server-functions/src/routes/middleware/index.tsx, e2e/react-start/server-functions/src/routes/middleware/request-middleware.tsx
Registers new route /middleware/request-middleware; adds nav link; implements route with request-scoped middleware, server function, loader wiring, and component rendering loader/client results.
Tests
e2e/react-start/server-functions/tests/server-functions.spec.ts
Adds tests ensuring loader and client server-function calls see identical request data via request middleware.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant B as Browser
  participant R as Route Loader
  participant SF as Server Function
  participant MW as Request Middleware
  participant SC as Start Context
  participant S as Start Server

  Note over S,SC: Server initializes Start context per request
  B->>R: Navigate to /middleware/request-middleware
  R->>S: Invoke loader (server)
  S->>SC: runWithStartContext({ contextAfterGlobalMiddlewares, request })
  R->>SF: call serverFn()
  SF->>SC: getStartContextServerOnly()
  SC-->>SF: { contextAfterGlobalMiddlewares, request }
  SF->>MW: execute middleware({ request, next })
  MW-->>SF: next({ context: { requestParam, requestFunc }})
  SF-->>R: { requestParam, requestFunc }
  R-->>B: Render loader data

  Note over B,SF: Client-triggered call
  B->>SF: serverFn() via client
  SF->>SC: getStartContextServerOnly() (server execution)
  SC-->>SF: { contextAfterGlobalMiddlewares, request }
  SF->>MW: execute middleware({ request, next })
  MW-->>SF: next({ context: { requestParam, requestFunc }})
  SF-->>B: { requestParam, requestFunc }
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • chorobin
  • birkskyum

Poem

A hare with hops through routes anew,
It carried requests the whole way through.
Context in paw, no URL amiss,
Loader and client now match with bliss.
Thump-thump! I merge with cheer—
Middleware’s song is crystal clear. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the core fix—ensuring the request parameter is passed into the request middleware when used with a server function—and matches the change implemented without extraneous details.
Linked Issues Check ✅ Passed The modifications in core packages propagate the HTTP request into the middleware context as required by issue #5384, replacing the undefined parameter with the correct request object, and the added tests confirm that both loader and client invocations now receive the same URL, fully addressing the linked issue.
Out of Scope Changes Check ✅ Passed All code additions and modifications directly support the request-middleware fix or its end-to-end validation; no unrelated features or unrelated refactors have been introduced.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-5384

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d3478c and 28d5b99.

📒 Files selected for processing (9)
  • e2e/react-start/server-functions/src/routeTree.gen.ts (11 hunks)
  • e2e/react-start/server-functions/src/routes/middleware/index.tsx (1 hunks)
  • e2e/react-start/server-functions/src/routes/middleware/request-middleware.tsx (1 hunks)
  • e2e/react-start/server-functions/tests/server-functions.spec.ts (1 hunks)
  • packages/start-client-core/src/createServerFn.ts (4 hunks)
  • packages/start-client-core/src/getServerContextAfterGlobalMiddlewares.ts (0 hunks)
  • packages/start-client-core/src/getStartContextServerOnly.ts (1 hunks)
  • packages/start-server-core/src/createStartHandler.ts (1 hunks)
  • packages/start-storage-context/src/async-local-storage.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/start-client-core/src/getServerContextAfterGlobalMiddlewares.ts
🧰 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:

  • e2e/react-start/server-functions/src/routes/middleware/index.tsx
  • packages/start-server-core/src/createStartHandler.ts
  • packages/start-client-core/src/createServerFn.ts
  • packages/start-client-core/src/getStartContextServerOnly.ts
  • e2e/react-start/server-functions/src/routeTree.gen.ts
  • packages/start-storage-context/src/async-local-storage.ts
  • e2e/react-start/server-functions/src/routes/middleware/request-middleware.tsx
  • e2e/react-start/server-functions/tests/server-functions.spec.ts
**/src/routes/**

📄 CodeRabbit inference engine (AGENTS.md)

Place file-based routes under src/routes/ directories

Files:

  • e2e/react-start/server-functions/src/routes/middleware/index.tsx
  • e2e/react-start/server-functions/src/routes/middleware/request-middleware.tsx
e2e/**

📄 CodeRabbit inference engine (AGENTS.md)

Store end-to-end tests under the e2e/ directory

Files:

  • e2e/react-start/server-functions/src/routes/middleware/index.tsx
  • e2e/react-start/server-functions/src/routeTree.gen.ts
  • e2e/react-start/server-functions/src/routes/middleware/request-middleware.tsx
  • e2e/react-start/server-functions/tests/server-functions.spec.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.ts
  • packages/start-client-core/src/createServerFn.ts
  • packages/start-client-core/src/getStartContextServerOnly.ts
  • packages/start-storage-context/src/async-local-storage.ts
🧬 Code graph analysis (4)
e2e/react-start/server-functions/src/routes/middleware/index.tsx (1)
e2e/react-start/server-functions/src/routes/middleware/request-middleware.tsx (1)
  • Route (23-26)
packages/start-client-core/src/createServerFn.ts (1)
packages/start-client-core/src/getStartContextServerOnly.ts (1)
  • getStartContextServerOnly (4-4)
packages/start-client-core/src/getStartContextServerOnly.ts (1)
packages/start-storage-context/src/async-local-storage.ts (1)
  • getStartContext (22-34)
e2e/react-start/server-functions/src/routes/middleware/request-middleware.tsx (2)
packages/start-server-core/src/request-response.ts (1)
  • getRequest (72-75)
packages/start-client-core/src/createServerFn.ts (1)
  • createServerFn (49-170)
⏰ 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: Preview
  • GitHub Check: Test
🔇 Additional comments (11)
e2e/react-start/server-functions/src/routes/middleware/index.tsx (1)

27-35: LGTM! Navigation link properly configured.

The new link to the request-middleware route is correctly set up with appropriate test ID and reloadDocument={true} to ensure full page reload for testing.

e2e/react-start/server-functions/tests/server-functions.spec.ts (1)

386-410: LGTM! Comprehensive test coverage for the fix.

The test properly validates that:

  1. The request parameter in middleware is defined and non-empty
  2. request.url equals getRequest().url for both loader (SSR) and client-side calls

This effectively verifies the fix for issue #5384.

e2e/react-start/server-functions/src/routes/middleware/request-middleware.tsx (3)

6-15: LGTM! Middleware properly demonstrates the fix.

The request middleware correctly captures both request.url (from the parameter) and getRequest().url (from the helper function) to validate they're equivalent. This directly addresses issue #5384 where the request parameter was previously undefined.


17-26: LGTM! Server function and route properly configured.

The server function correctly applies the request middleware and the route loader invokes it appropriately for testing both SSR and client-side scenarios.


28-83: LGTM! Component properly displays test data.

The component clearly displays both loader data (SSR) and client data (after button click), making it easy to verify the fix visually and via automated tests.

e2e/react-start/server-functions/src/routeTree.gen.ts (1)

30-30: LGTM! Auto-generated route registration.

The new route /middleware/request-middleware has been properly registered across all route type mappings by TanStack Router's code generation.

Also applies to: 125-130, 165-165, 189-189, 214-214, 240-240, 264-264, 288-288, 313-313, 450-456, 497-497

packages/start-server-core/src/createStartHandler.ts (1)

173-173: LGTM! Request is now propagated through Start context.

The addition of the request parameter to runWithStartContext enables downstream middleware and handlers to access the HTTP request via the Start context. This directly addresses the issue where request middleware received undefined for the request parameter.

packages/start-storage-context/src/async-local-storage.ts (1)

6-6: LGTM! Type definition correctly extends the context interface.

The addition of the request: Request property to StartStorageContext properly types the request that is now propagated through the Start context, enabling type-safe access in downstream code.

packages/start-client-core/src/getStartContextServerOnly.ts (1)

1-4: LGTM! Clean server-only wrapper for context access.

The server-only wrapper ensures that Start context (which now includes the request) is only accessible in the server environment, preventing accidental client-side access. This is a clean abstraction that maintains proper environment boundaries.

packages/start-client-core/src/createServerFn.ts (2)

133-144: LGTM! Request is now correctly extracted from Start context.

The server function execution context now receives the request from startContext.request, fixing the issue where the request parameter was undefined in request middleware when used with server functions. The change maintains backward compatibility by preserving the existing contextAfterGlobalMiddlewares structure.


204-213: Approve middleware resolution refactor
The added 'client'/'server' existence checks are semantically equivalent to the prior ternary and safer—every middleware in the codebase supplies the appropriate .client or .server.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

nx-cloud bot commented Oct 6, 2025

View your CI Pipeline Execution ↗ for commit 28d5b99

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 6m 42s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 20s View ↗

☁️ Nx Cloud last updated this comment at 2025-10-06 22:30:03 UTC

Copy link

pkg-pr-new bot commented Oct 6, 2025

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5390

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@5390

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@5390

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5390

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@5390

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@5390

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@5390

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@5390

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@5390

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@5390

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@5390

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@5390

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@5390

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@5390

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@5390

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@5390

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@5390

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@5390

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@5390

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@5390

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@5390

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@5390

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@5390

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@5390

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@5390

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@5390

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@5390

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@5390

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@5390

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@5390

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@5390

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5390

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@5390

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@5390

commit: 28d5b99

@schiller-manuel schiller-manuel merged commit 776d3d5 into main Oct 6, 2025
6 checks passed
@schiller-manuel schiller-manuel deleted the fix-5384 branch October 6, 2025 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Start: request param is undefined in request middleware when used in server function
1 participant