fix: do not pass signal into server function #5470

schiller-manuel · 2025-10-14T00:15:22Z

after this is merged and released, srvx must be bumped here

Summary by CodeRabbit

New Features
- Added a Cloudflare Workers configuration scaffold to the React start-basic example for easier deployment.
Refactor
- Simplified server function APIs by removing the explicit AbortSignal parameter and related abort wiring.
Tests
- Consolidated and parameterized server-function tests across GET/POST methods for clearer coverage.
Chores
- Updated build tooling/dev dependencies across multiple example and e2e projects to the latest compatible versions.
- Adjusted snapshot-cleanup script to more reliably remove snapshot artifacts.

coderabbitai · 2025-10-14T00:15:46Z

Walkthrough

Removes AbortSignal propagation from server function plumbing: updates createServerFn API and codegen to drop signal, adjusts server handler to stop wiring request abort signals, and updates related tests and an e2e route/spec. Also bumps srvx versions across e2e packages, tweaks a plugin script, and adds a Wrangler config scaffold.

Changes

Cohort / File(s)	Summary
React e2e srvx bump `e2e/react-start/*/package.json`	DevDependency srvx updated from ^0.8.6 to ^0.8.16 across React-based e2e apps.
Solid e2e srvx bump `e2e/solid-start/*/package.json`	DevDependency srvx updated from ^0.8.6 to ^0.8.16 across Solid-based e2e apps.
Server function API (client core) `packages/start-client-core/src/createServerFn.ts`, `packages/start-client-core/src/tests/createServerFn.test-d.ts`	Removes signal from ServerFnCtx and __executeServer signature; updates type tests accordingly.
Server function handler (server core) `packages/start-server-core/src/server-functions-handler.ts`	Removes request AbortController wiring and stops passing signal to actions.
Plugin: createServerFn codegen `packages/start-plugin-core/src/create-server-fn-plugin/handleCreateServerFn.ts`	Generated server function now calls __executeServer(opts) without signal.
Plugin tests and snapshots `packages/start-plugin-core/tests/...createServerFn...`	Updates handlers from (opts, signal) to (opts) and drops signal in __executeServer calls across client/server snapshots and tests.
Plugin package config `packages/start-plugin-core/package.json`	Changes clean:snapshots glob to `*/snapshots*`; bumps dependency srvx to ^0.8.16.
E2E route and tests (server functions) `e2e/react-start/server-functions/src/routes/abort-signal.tsx`, `e2e/react-start/server-functions/tests/server-functions.spec.ts`	Refactors route to method-specific GET/POST handlers using createServerOnlyFn/getRequest; parameterizes tests over methods and updates test IDs.
Example config `examples/react/start-basic/wrangler.jsonc`	Adds Cloudflare Wrangler config scaffold with comments and observability enabled.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant C as Client App
  participant SF as ServerFn (client wrapper)
  participant H as Server Handler
  participant A as Action (handler)

  U->>C: Trigger server function
  C->>SF: call fn(opts)
  SF->>H: HTTP request (opts)
  Note over SF,H: No AbortSignal is created or forwarded
  H->>A: action(opts)
  A-->>H: result | error
  H-->>SF: HTTP response
  SF-->>C: resolve/reject
  C-->>U: render outcome

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

fix: basepath #5345 — Touches the same server-functions handler area, refactoring sanitization/regex; overlaps file and responsibility.
feat: serverfn abstraction #5040 — Modifies server-function plumbing (createServerFn, __executeServer, plugin codegen) similar to this PR’s API/signature changes.

Suggested reviewers

chorobin

Poem

I nibbled the threads where signals once ran,
Snipped tidy paths, a clean little plan.
GET and POST hop in two neat lines,
Snapshots swept where moonlight shines.
With wrangler burrows mapped just right—
Ship it swift, then carrot delight! 🥕✨

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 (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title ‘fix: do not pass signal into server function’ clearly conveys the primary change of the PR, which is removing the signal parameter from server function invocations. It is concise, specific, and uses an imperative tone that aligns with conventional commit style. This phrasing makes the intent of the change immediately clear to reviewers and future readers.

✨ 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-signal

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

nx-cloud · 2025-10-14T00:15:56Z

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

_{To disable these notifications, a workspace admin can disable them in workspace settings.}

View your CI Pipeline Execution ↗ for commit 6a36d34

Command	Status	Duration	Result
`nx affected --targets=test:eslint,test:unit,tes...`	❌ Failed	2m 27s	View ↗
`nx run-many --target=build --exclude=examples/*...`	❌ Failed	54s	View ↗

☁️ Nx Cloud last updated this comment at 2025-10-14 00:18:55 UTC

coderabbitai

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/start-client-core/src/createServerFn.ts (1)
132-155: Fix the __executeServer contract after dropping signal.

The runtime now calls __executeServer(opts) with no signal, but FetcherBase['__executeServer'] still requires { signal: AbortSignal }. Any TS consumer (including the plugin-generated stubs) will fail type-checking because opts no longer has that property. Please drop the signal field (or make it optional) in the FetcherBase signature to keep the public types aligned with the new behavior.
 export interface FetcherBase {
   [TSS_SERVER_FUNCTION]: true
   url: string
-  __executeServer: (opts: {
-    method: Method
-    data: unknown
-    headers?: HeadersInit
-    context?: any
-    signal: AbortSignal
-  }) => Promise<unknown>
+  __executeServer: (opts: {
+    method: Method
+    data: unknown
+    headers?: HeadersInit
+    context?: any
+  }) => Promise<unknown>
 }
Also applies to: 258-264

🧹 Nitpick comments (1)

e2e/react-start/server-functions/src/routes/abort-signal.tsx (1)
42-106: LGTM!

The Test component correctly implements method-specific testing with proper state management, abort scenarios, and error handling. The method-specific test IDs (run-with-abort-btn-${method}, result-${method}, etc.) align well with e2e test expectations.

The union type for the fn prop (line 47) works correctly since both handlers wrap the same underlying function. If desired, you could define a common type alias for cleaner typing, but this is optional.

Optional: Consider extracting a common type for the server function.

For improved type clarity, you could define a common type:
+type AbortableServerFn = typeof abortableServerFnPost
+
 function Test({
   method,
   fn,
 }: {
   method: string
-  fn: typeof abortableServerFnPost | typeof abortableServerFnGet
+  fn: AbortableServerFn
 }) {
However, this is a minor improvement and the current approach is acceptable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5436818 and 6a36d34.

⛔ Files ignored due to path filters (1)

pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (40)

e2e/react-start/basic-react-query/package.json (1 hunks)
e2e/react-start/basic-tsr-config/package.json (1 hunks)
e2e/react-start/basic/package.json (1 hunks)
e2e/react-start/custom-basepath/package.json (1 hunks)
e2e/react-start/scroll-restoration/package.json (1 hunks)
e2e/react-start/selective-ssr/package.json (1 hunks)
e2e/react-start/serialization-adapters/package.json (1 hunks)
e2e/react-start/server-functions/package.json (1 hunks)
e2e/react-start/server-functions/src/routes/abort-signal.tsx (3 hunks)
e2e/react-start/server-functions/tests/server-functions.spec.ts (1 hunks)
e2e/react-start/server-routes/package.json (1 hunks)
e2e/react-start/virtual-routes/package.json (1 hunks)
e2e/react-start/website/package.json (1 hunks)
e2e/solid-start/basic-tsr-config/package.json (1 hunks)
e2e/solid-start/basic/package.json (1 hunks)
e2e/solid-start/custom-basepath/package.json (1 hunks)
e2e/solid-start/scroll-restoration/package.json (1 hunks)
e2e/solid-start/selective-ssr/package.json (1 hunks)
e2e/solid-start/server-functions/package.json (1 hunks)
e2e/solid-start/server-routes/package.json (1 hunks)
e2e/solid-start/website/package.json (1 hunks)
examples/react/start-basic/wrangler.jsonc (1 hunks)
packages/start-client-core/src/createServerFn.ts (1 hunks)
packages/start-client-core/src/tests/createServerFn.test-d.ts (0 hunks)
packages/start-plugin-core/package.json (2 hunks)
packages/start-plugin-core/src/create-server-fn-plugin/handleCreateServerFn.ts (1 hunks)
packages/start-plugin-core/tests/createServerFn/createServerFn.test.ts (4 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/client/createServerFnDestructured.tsx (1 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/client/createServerFnDestructuredRename.tsx (1 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/client/createServerFnStarImport.tsx (1 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/client/createServerFnValidator.tsx (1 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/client/factory.tsx (1 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/client/isomorphic-fns.tsx (1 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/server/createServerFnDestructured.tsx (2 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/server/createServerFnDestructuredRename.tsx (2 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/server/createServerFnStarImport.tsx (3 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/server/createServerFnValidator.tsx (1 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/server/factory.tsx (1 hunks)
packages/start-plugin-core/tests/createServerFn/snapshots/server/isomorphic-fns.tsx (1 hunks)
packages/start-server-core/src/server-functions-handler.ts (3 hunks)

💤 Files with no reviewable changes (1)

packages/start-client-core/src/tests/createServerFn.test-d.ts

🧰 Additional context used

📓 Path-based instructions (6)

**/package.json