-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: allow custom fetch impl for server functions #6327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds per-call Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant RPC as serverFnFetcher
participant Middleware as MiddlewareChain
participant Server as ServerFunction
Note over Client,RPC: Call-site may pass an optional fetch
Client->>RPC: invoke serverFn(payload, opts{ fetch? })
RPC->>Middleware: deliver payload including _callSiteFetch/fetch
alt Middleware modifies fetch
Middleware->>Middleware: set/override ctx.fetch
end
Middleware->>Server: call handler with ctx (ctx.fetch prioritized)
Server->>Server: perform fetch using resolved fetch impl
Server-->>Client: return response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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. Comment |
|
View your CI Pipeline Execution ↗ for commit 58d101f
☁️ Nx Cloud last updated this comment at |
af99c0e to
aa7aadc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/start/framework/react/guide/middleware.md (1)
545-545: Minor wording refinement.Per static analysis, "merged together" can be simplified to "merged" for conciseness while retaining clarity.
✏️ Proposed wording refinement
- When multiple middlewares set headers, they are **merged together**. Later middlewares can add new headers or override headers set by earlier middlewares: + When multiple middlewares set headers, they are **merged**. Later middlewares can add new headers or override headers set by earlier middlewares:
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/start/framework/react/guide/middleware.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use relative links to
docs/folder format (e.g.,./guide/data-loading) for internal documentation references
Files:
docs/start/framework/react/guide/middleware.md
🪛 LanguageTool
docs/start/framework/react/guide/middleware.md
[style] ~523-~523: Consider using a different adverb to strengthen your wording.
Context: ...ses the client method executes in a completely different client-side context than se...
(COMPLETELY_ENTIRELY)
[style] ~545-~545: ‘merged together’ might be wordy. Consider a shorter alternative.
Context: ...ple middlewares set headers, they are merged together. Later middlewares can add new header...
(EN_WORDINESS_PREMIUM_MERGED_TOGETHER)
⏰ 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 (1)
docs/start/framework/react/guide/middleware.md (1)
525-720: Comprehensive and well-structured documentation for custom fetch and header handling.The new sections introduce client-side request modification clearly, with well-organized subsections on headers, custom fetch, and precedence rules. The examples progress logically from simple to complex (single middleware, call-site override, chained middlewares), and the precedence tables effectively communicate priority order. The SSR caveat (line 718-719) is appropriately highlighted to prevent common misunderstandings.
I see the documentation imports
CustomFetchfrom'@tanstack/react-start'(lines 605, 627). Please verify this type is exported from the public API surface of the package, as indicated in the PR summary.
Summary by CodeRabbit
New Features
UI
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.