feat(web): let an app supply the fetch server function calls go through - #3080
feat(web): let an app supply the fetch server function calls go through#3080frenzzy wants to merge 6 commits into
Conversation
🦋 Changeset detectedLatest commit: fa55a14 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will not alter performance
Comparing Footnotes
|
c1e8b68 to
d095e43
Compare
ryansolid
left a comment
There was a problem hiding this comment.
Reviewed with the dispatch map in hand: the funnel claim checks out. Every call shape — plain POST, GET-declared reads, the long-URL POST fallback, and live including its reconnect — reaches the wire through fetchServerFunction → createRequest, and after this change that function has exactly one exit. The observer-path normalization is right too: a wrapper written against the documented shape must not misroute the moment devtools attach, and read-only observers seeing a reconstructed Request instead of the dispatched object costs nothing.
The design also lands where the addressing question settled in #3072: the app owns its URL shape by rewriting into the canonical address, and no downstream gate learns a second format. Good.
One change request, one suggestion:
Type the seam as what it promises. The option is typeof globalThis.fetch, but the contract this PR documents — "always called as (address, init)" — is narrower and is the whole point of the seam. As typed, a wrapper must handle Request | URL inputs it will never receive, which is why the specs need address as string casts. Something like
fetch?: ((address: string, init: RequestInit) => Promise<Response>) | null;keeps every existing fetch-shaped function assignable (wider parameters, contravariance) while letting a hand-written wrapper use the address without casting. The type is the second place the guarantee lives; right now it contradicts the doc.
Doc nit: "what a wrapper owes the transport" names init.signal, but the per-call options from invoke (keepalive, priority) ride on the same init now that the invocation channel standardizes them. Rather than growing the list one field at a time, state it as: forward init wholesale — everything the transport decided about the call travels on it. Worth one more line that the seam is the client transport's exit: in-process server-side calls (invoke on the server) never fetch and never see it.
CodSpeed is the same fork artifact as on #3088 — not yours.
d095e43 to
9ed85bc
Compare
|
Both applied: the type is Changes since your review, mostly from an edge-case pass:
|
The transport sends every call with the global `fetch`.
`configureServerFunctionsClient({ fetch })` replaces it: retries, telemetry, a
test double, or a route of the app's own.
An app-shaped url — an argument in a path segment, a tenant in a prefix —
cannot be a second built-in address without teaching every gate that
recognises a call: the runtime, the plugin's dev middleware, the generated
dispatch gate, the router's action-url interception. It does not have to be
one. The handler takes a web `Request`, so an app route that rewrites into the
canonical address dispatches like any other call, and the client's side of it
was the only piece missing.
The seam is typed and called as `(address, init)`, including on the path where
call observers are installed — that path used to hand the global `fetch` a
`Request`, and keeping the difference would mean a wrapper written against the
documented shape silently misrouting the moment devtools attached. Observers
receive a reconstruction of the dispatched request. `null` restores the
global, and a wrapper that answers with anything but a `Response` is told so
by name rather than through a property read on undefined.
Also tidies the `endpoint` docs on both entries, which the path-addressing
change left saying the same thing twice.
9ed85bc to
df5b25e
Compare
Review of the seam turned up two things the observed path got wrong, both introduced by giving it the same `(address, init)` shape as the path without observers. The reconstructed `Request` was built from the live `init`, so a streaming body was disturbed before the send got it — a call that worked without devtools failed with them attached. It is now built without a streaming body, and skipped altogether if the init will not make a `Request` at all: an init the configured fetch would have tolerated must not fail the call because something was watching. The return guard was `instanceof Response`, which refuses a mock, a polyfill and another realm's response for their identity rather than their shape. It now duck-types, and also catches the likelier mistake of handing back a response the wrapper already read. Along with it: `init` forwarding is stated as the contract it is — dropping `signal` voids both the caller's abort and a live source's teardown — the option refuses a non-callable value where it is set rather than per call, the declared return admits a synchronous `Response` the code already accepted, and the server entry carries a no-op configurator so a shared config module resolves on both builds.
The previous commit added two runtime checks the declared option already rules out: a guard rejecting a value that is not callable, and a per-call guard on what the wrapper answered with. Both describe author mistakes a `((address, init) => Response | Promise<Response>) | null` catches at the call site, and one of them ran on every request to do it. What stays is the part types cannot express: the observed request is reconstructed without a streaming body, because reconstructing one consumes it before the send can use it.
It filled a gap this PR did not open: `configureServerFunctionsClient` has no server mirror on `next` either, and the option's own documentation points at the client entry as the place to call it. A no-op there also swallows the call silently, which is the wrong answer if someone reaches for it from shared code — that deserves its own issue and its own decision, not a line in a PR about the transport's exit.
Reverting each part of this change one at a time showed the tests did not watch two of them. Nothing failed when the reconstruction for observers was allowed to consume a streaming body — the regression the previous commit fixed — so there is a test for it now, and it is the only one that fails when that guard goes. Nothing failed either when `null` stopped resetting the option, because `config.fetch || fetch` already answers for a null. The normalisation on the way in was dead; it is gone. One test went with them: with the streaming case pinned, the FormData one died on exactly the same reverts as the shape test above it.
The hook's init is loosely typed enough to take `duplex` without complaint, so the directive above it had nothing to suppress and `tsc --project tsconfig.test.json` failed on the directive itself.
Follow-up to #3076, and the userland half of the addressing question in #3072.
configureServerFunctionsClient({ fetch })replaces the function the transport sends every server-function request with — retries, telemetry, a test double, or a route of the app's own.nullrestores the global.Why a seam and not a second address
An app that wants a cache-shaped url — an argument in a path segment, a tenant in a prefix — can't get one from a second built-in address format: that has to be taught to every gate that recognises a call (the runtime, the plugin's dev middleware, the generated dispatch gate, the router's action-url interception). It doesn't have to be one. The handler takes a web
Request, so an app route that rewrites into the canonical address dispatches like any other call:That already works on
next. The client's side of it didn't exist —prepareRequestsees everything about a request except the part that addresses it — and this adds it, without any gate downstream learning a second format.The shape
Always
(address, init), the address relative to the document, so an ordinary fetch wrapper drops in andparseServerFunctionUrlreads the id back out for telemetry.One behaviour change: the path taken when call observers are installed used to hand the global
fetchaRequestinstead. Leaving that difference would mean a wrapper written against the documented shape misrouting the moment devtools attach. Observers now see a reconstruction of the dispatched request, built without a streaming body — reconstructing one consumes it before the send can use it.Three things a type can't say, so the doc does: forward
init(the call'ssignalrides on it; dropping it voids the caller's abort and a live source's teardown), keep the call same-origin, hand the response back unread.Checked by hand
Every dispatch path goes through the seam with a
stringaddress the id parses back out of: plain POST, GET-declared reads, the long-url POST fallback, boundaction.withcalls, an explicitbase, andlive()including reconnect. A retrying wrapper re-sends string, FormData, Blob and File bodies, which handing over aRequestcould not have done.