fix(web): unwrap respond() on direct calls; make its envelope a Response

.changeset/respond-envelope-is-a-response.md

@@ -0,0 +1,9 @@
+---
+"@solidjs/web": patch
+---
+
+`respond()` is typed as its value and its envelope is a real `Response`; direct calls unwrap it.
+
+- **Direct calls unwrap envelopes.** A server function called in-process during SSR that returned `respond(value)` handed its caller the `ResponseEnvelope` itself, while an HTTP caller decoded `value` — and a server component wrapped for its headers (`respond(View, { headers })`) reached the render as an object, so the frames policy never branded it and the hydration serializer failed on the envelope. The direct leg now resolves with the value (a thrown envelope rejects with it) before `transformDirectResult` runs. Of the metadata, `Set-Cookie` is appended to the render's response head; other headers describe the function's own address and the status is the page's, so neither is applied to the document.
+- **The envelope is a `Response`.** `ResponseEnvelope` now extends `Response`, carrying the given response's body, status and headers, so a consumer with no Solid knowledge (a fetch-style API dispatch) answers with it as-is instead of JSON-encoding the wrapper. `new ResponseEnvelope(response, value)` keeps its signature; `response` is the envelope itself, or `undefined` when constructed without one. Solid's own consumers still recognize it with `isResponseEnvelope()`.
+- **The helpers are typed by what they mean to the caller.** `respond<T>()` returns `T`, and `redirect()`/`reload()` are `<T = never>(…): T` — control flow, not values, so they vanish from the returning function's inferred type, while an annotated return or a `Response`-typed request handler supplies `T` (a literal `never` would also mark code after a bare call unreachable). A bare `"use server"` function's own signature is therefore its callers' type, with no wrapper narrowing (`NarrowResponse`) needed. At runtime the helpers are unchanged.

documentation/solid-2.0/10-server-functions.md

@@ -94,7 +94,9 @@ return reload({ revalidate: "todos" });
 return respond(item, { status: 201, revalidate: "items" });
 ```
 
-`respond()` produces a `ResponseEnvelope` — HTTP metadata paired with an in-memory value. The handler forwards the envelope’s headers and status and encodes the value as the body, while scripted callers receive the value transparently. Crucially, the carried response holds a **real JSON body**, so progressive-enhancement consumers (no-JS form posts, direct HTTP) get real JSON while scripted calls get the in-memory value — no reparse. Thrown envelopes ride the same path with an error tag (`X-Server-Function-Error`) and their status forwarded. Check with `isResponseEnvelope()` (a registered-symbol brand, correct across duplicated bundles — always prefer it over `instanceof`).
+`respond()` produces a `ResponseEnvelope` — HTTP metadata paired with an in-memory value. The handler forwards the envelope’s headers and status and encodes the value as the body, while scripted callers receive the value transparently. Crucially, the envelope **is a real `Response`** with a JSON body, so progressive-enhancement consumers (no-JS form posts, direct HTTP) get real JSON while scripted calls get the in-memory value — no reparse — and a consumer that knows nothing of Solid (a filesystem router's API dispatch, any fetch-style handler) answers with it as-is. Thrown envelopes ride the same path with an error tag (`X-Server-Function-Error`) and their status forwarded. Check with `isResponseEnvelope()` (a registered-symbol brand, correct across duplicated bundles — always prefer it over `instanceof`).
+
+The helpers are typed by what they mean to the caller, not as the objects they build: `respond(value)` is typed as `value`, and `redirect()`/`reload()` — control flow for the integration, not values — never show up in the returning function's type (`<T = never>(…): T`: the `never` vanishes from the inferred union, and where the context names a type, such as an annotated return or a `Response`-typed request handler, `T` takes it). So a bare `"use server"` function returning `respond(item)` on one branch and `redirect("/login")` on another is typed `Promise<Item>`, with no wrapper needed to narrow it. A direct call during SSR receives `respond()`'s value too (a thrown envelope rejects with it), and the envelope's `Set-Cookie` headers reach the page's response; its other headers and its status describe the function's own address and are not applied to the document.
 
 **Redirects to scripted callers ride a dedicated carrier.** fetch follows the redirect statuses (301/302/303/307/308) before the transport can read them, so a scripted answer masks the 3xx to 200 and carries the redirect in `X-Server-Function-Redirect`: the author’s status plus the target **resolved against the request URL** — exactly the meaning HTTP assigns the `Location` a form post would have received. Resolving server-side means `redirect("/")` and `redirect(new URL("/", url).href)` arrive identical, so an integration compares origins on a real URL instead of guessing navigation strategy from how the author spelled the target (#3102, #3107); decode with `decodeRedirectHeaderValue`. `Location` itself never rides a masked answer — on a 200 it has no HTTP meaning, and an authored `Location` on a forwarding status (a 201’s created-at) stays what it is: data. Unscripted callers get the real 3xx, and the non-followable 3xx band (304) forwards untouched for everyone.
 

packages/web/server-functions/src/server.ts

@@ -1364,20 +1364,34 @@ export function createServerReference({ id, fn, name }) {
       // provideEventOnce): a broken hook used to double-commit or skip the
       // body silently during a render, where there is no status line to
       // notice it by.
-      let result = provideEventOnce(provideEvent, evt, () => {
-        const run = () => fn.apply(thisArg, args);
-        // The wrapper must return run()'s value (this path stays
-        // synchronous for synchronous functions). Observed as a whole —
-        // policy included — as the `"invocation"` record on `OBSERVE.records`;
-        // a no-op with no listener and outside observe builds.
-        return observeInvocation({ id, direct: true, event: evt, args }, () =>
-          reportDirectFailure(
-            () => (wrap ? wrap(run, { id, args, event: evt, direct: true }) : run()),
-            id,
-            hook
-          )
-        );
-      });
+      let result;
+      try {
+        result = provideEventOnce(provideEvent, evt, () => {
+          const run = () => fn.apply(thisArg, args);
+          // The wrapper must return run()'s value (this path stays
+          // synchronous for synchronous functions). Observed as a whole —
+          // policy included — as the `"invocation"` record on `OBSERVE.records`;
+          // a no-op with no listener and outside observe builds.
+          return observeInvocation({ id, direct: true, event: evt, args }, () =>
+            reportDirectFailure(
+              () => (wrap ? wrap(run, { id, args, event: evt, direct: true }) : run()),
+              id,
+              hook
+            )
+          );
+        });
+      } catch (error) {
+        throw directEnvelopeValue(error, evt);
+      }
+      result =
+        result && typeof result.then === "function"
+          ? result.then(
+              value => directEnvelopeValue(value, evt),
+              error => {
+                throw directEnvelopeValue(error, evt);
+              }
+            )
+          : directEnvelopeValue(result, evt);
       // A generator or stream body runs when the caller pulls it, after the
       // call-time scope above has gone. Bind the WRAPPER'S result (not merely
       // fn's) so a deferred wrapInvocation keeps the same semantics.
@@ -3446,6 +3460,28 @@ function reportDirectFailure(run, id, hook) {
   return result && typeof result.then === "function" ? result.then(undefined, report) : result;
 }
 
+/**
+ * A `respond()` envelope a direct call returned or threw, as its caller
+ * receives it: the value, exactly as an HTTP caller decodes it — the
+ * in-process leg has no Response to hand over. Of the metadata, only
+ * `Set-Cookie` reaches the render's response head: a cookie is state the
+ * function established, and the browser must receive it whichever leg ran
+ * the call. The rest describes the function's own address (a GET's
+ * `Cache-Control` is about that url, not the page composed from it) and the
+ * status is the page's, so neither is applied to the document.
+ */
+function directEnvelopeValue(result, event) {
+  if (!isResponseEnvelope(result)) return result;
+  const { response, value } = result;
+  const stub = event.response;
+  if (response && stub && stub.headers && response.headers.getSetCookie) {
+    for (const cookie of response.headers.getSetCookie()) {
+      stub.headers.append("Set-Cookie", cookie);
+    }
+  }
+  return value;
+}
+
 export function sanitizeServerError(value) {
   if (DEV) return value;
   if (isSafeError(value)) return value;

packages/web/src/response.ts

@@ -22,35 +22,48 @@ const ENVELOPE = Symbol.for("solid.ResponseEnvelope");
  * payloads); the HTTP handler forwards `response`'s headers and
  * (non-redirect) status and encodes `value` as the body through the codec,
  * while client-only integrations read `value` directly — no reparse.
+ *
+ * The envelope IS a `Response` — the given response's body, status and
+ * headers — so a consumer that knows nothing of Solid (a filesystem
+ * router's API dispatch, any fetch-style handler) answers with it as-is.
+ * `response` is the envelope itself, or `undefined` when constructed
+ * without one.
  */
-// PURE-annotated factory (same convention as solid's MockPromise): the brand
-// lives on the prototype, but a bare top-level `C.prototype[X] = true` is a
-// module side effect that pins the class into every bundle including this
-// module — client bundles that never construct or brand-check an envelope
-// were retaining it. Wrapping the declaration and the brand assignment in one
-// pure expression lets the whole thing shake when unreferenced. (A `static {}`
-// block would NOT work: bundlers treat static blocks as side-effectful.)
-export interface ResponseEnvelope<T = unknown> {
+export interface ResponseEnvelope<T = unknown> extends Response {
   response: Response | undefined;
   value: T;
 }
 
+// Built on first construction: `extends Response` evaluated at module load
+// would throw wherever the global is missing, and a top-level class with a
+// prototype write is a side effect that pins it into every bundle importing
+// this module.
+let EnvelopeClass: any;
+
 export const ResponseEnvelope: {
   new <T>(response: Response | undefined, value: T): ResponseEnvelope<T>;
-} = /* @__PURE__ */ (() => {
-  class ResponseEnvelope {
-    response: Response | undefined;
-    value: unknown;
-    constructor(response: Response | undefined, value: unknown) {
-      this.response = response;
-      this.value = value;
-    }
+} = function ResponseEnvelope(response: Response | undefined, value: unknown) {
+  if (!EnvelopeClass) {
+    EnvelopeClass = class extends Response {
+      response: Response | undefined;
+      value: unknown;
+      constructor(response: Response | undefined, value: unknown) {
+        super(
+          response ? response.body : null,
+          response && {
+            status: response.status,
+            statusText: response.statusText,
+            headers: copyHeaders(response.headers)
+          }
+        );
+        this.response = response ? this : undefined;
+        this.value = value;
+      }
+    };
+    EnvelopeClass.prototype[ENVELOPE] = true;
   }
-  (ResponseEnvelope.prototype as any)[ENVELOPE] = true;
-  return ResponseEnvelope;
-})() as {
-  new <T>(response: Response | undefined, value: T): ResponseEnvelope<T>;
-};
+  return new EnvelopeClass(response, value);
+} as any;
 
 /** Whether `value` is a `ResponseEnvelope` (robust across module copies). */
 export function isResponseEnvelope(value: unknown): value is ResponseEnvelope {
@@ -167,25 +180,25 @@ export interface ResponseHelperInit extends ResponseInit {
 /** @internal */
 export const RESPONSE_HEADER_VALUE_LIMIT = 4096;
 
+// Copy preserving multiple Set-Cookie values: Headers-to-Headers copying
+// through the constructor folds them into one comma-joined entry on some
+// runtimes (a folded Set-Cookie is corrupt). Plain-object inits cannot
+// carry duplicates and pass through as-is.
+function copyHeaders(init: HeadersInit | undefined): Headers {
+  const source = init as Headers | undefined;
+  if (!source || !source.getSetCookie) return new Headers(init);
+  const headers = new Headers();
+  source.forEach((value, key) => {
+    if (key !== "set-cookie") headers.append(key, value);
+  });
+  for (const cookie of source.getSetCookie()) headers.append("Set-Cookie", cookie);
+  return headers;
+}
+
 function initWithRevalidate(init: number | ResponseHelperInit = {}) {
   const resolved: any = typeof init === "number" ? { status: init } : init;
   const { revalidate, ...responseInit } = resolved;
-  // Copy preserving multiple Set-Cookie values: Headers-to-Headers copying
-  // through the constructor folds them into one comma-joined entry on some
-  // runtimes (a folded Set-Cookie is corrupt). Plain-object inits cannot
-  // carry duplicates and pass through as-is.
-  let headers: Headers;
-  if (responseInit.headers && responseInit.headers.getSetCookie) {
-    headers = new Headers();
-    responseInit.headers.forEach((value: string, key: string) => {
-      if (key !== "set-cookie") headers.append(key, value);
-    });
-    for (const cookie of responseInit.headers.getSetCookie()) {
-      headers.append("Set-Cookie", cookie);
-    }
-  } else {
-    headers = new Headers(responseInit.headers);
-  }
+  const headers = copyHeaders(responseInit.headers);
   if (revalidate !== undefined) {
     const list = Array.isArray(revalidate) ? revalidate : [revalidate];
     if (list.length > 1 && list.includes(REVALIDATE_ALL)) {
@@ -213,8 +226,19 @@ function initWithRevalidate(init: number | ResponseHelperInit = {}) {
 /**
  * Response redirecting to `url` (default 302). `revalidate` names the
  * cache keys the mutation invalidated.
+ *
+ * Typed so it never shows up in the type of the function returning it: a
+ * redirect is control flow for the integration to act on, not a value.
+ * `T` defaults to `never`, which vanishes from the inferred return type's
+ * union; where the context expects a type (an annotated return, a
+ * `Response`-typed request handler) `T` takes it. A literal `never` return
+ * would also mark code after a bare call unreachable; a type parameter
+ * does not. At runtime it is a real `Response`.
  */
-export function redirect(url: string | Href, init: number | ResponseHelperInit = 302) {
+export function redirect<T = never>(
+  url: string | Href,
+  init: number | ResponseHelperInit = 302
+): T {
   if (typeof url !== "string" && !isHref(url)) {
     throw new TypeError(
       "redirect() expects a string URL or an Href-branded value (Symbol.for('solid.Href'))."
@@ -256,16 +280,19 @@ export function redirect(url: string | Href, init: number | ResponseHelperInit =
     );
   }
   headers.set("Location", encoded);
-  return new Response(null, { ...responseInit, headers });
+  return new Response(null, { ...responseInit, headers }) as T;
 }
 
 /**
  * Empty response requesting revalidation of the named cache keys (all of
  * them when omitted).
+ *
+ * Typed as `redirect` is, for the same reason: it never shows up in the
+ * type of the function returning it. At runtime it is a real `Response`.
  */
-export function reload(init: ResponseHelperInit = {}) {
+export function reload<T = never>(init: ResponseHelperInit = {}): T {
   const { responseInit, headers } = initWithRevalidate(init);
-  return new Response(null, { ...responseInit, headers });
+  return new Response(null, { ...responseInit, headers }) as T;
 }
 
 /**
@@ -284,8 +311,13 @@ export const NULL_BODY_STATUSES: ReadonlySet<number> = new Set([204, 205, 304]);
  * stays invisible: the carried response holds a plain JSON body so
  * consumers without the client runtime (no-JS form posts, direct HTTP)
  * get real JSON, while integrations read `value` — no reparse.
+ *
+ * Typed as `value`'s type: every caller of the function returning it —
+ * over HTTP or in-process — receives the value, never the envelope. At
+ * runtime it is a `ResponseEnvelope`, a real `Response` that integrations
+ * recognize with `isResponseEnvelope()`.
  */
-export function respond<T>(value: T, init: ResponseHelperInit = {}) {
+export function respond<T>(value: T, init: ResponseHelperInit = {}): T {
   const { responseInit, headers } = initWithRevalidate(init);
   // A null-body status cannot carry the passthrough JSON body — building it
   // would throw right here, at 200, masking the author's intent (#3095).
@@ -296,11 +328,14 @@ export function respond<T>(value: T, init: ResponseHelperInit = {}) {
   // else — and on a null-body status there is no body at all (#3197).
   for (const header of COMPOSED_BODY_FRAMING) headers.delete(header);
   if (NULL_BODY_STATUSES.has(responseInit.status)) {
-    return new ResponseEnvelope(new Response(null, { ...responseInit, headers }), value);
+    return new ResponseEnvelope(
+      new Response(null, { ...responseInit, headers }),
+      value
+    ) as unknown as T;
   }
   headers.set("Content-Type", "application/json");
   return new ResponseEnvelope(
     new Response(JSON.stringify(value), { ...responseInit, headers }),
     value
-  );
+  ) as unknown as T;
 }

packages/web/test/response-helpers.type-tests.ts

@@ -0,0 +1,58 @@
+// The response helpers are typed by what they mean to the caller of the
+// function returning them: `respond(value)` is `value`, and a `redirect()`
+// or `reload()` — control flow, not a value — never shows up. That holds
+// for a bare `"use server"` function, typed by its own signature, as much
+// as through `GET()`. Compile-only, under `test-types`.
+import { GET as clientGET } from "../server-functions/src/client.js";
+import { GET as serverGET } from "../server-functions/src/server.js";
+import { redirect, reload, respond } from "../src/response.js";
+
+type Equal<X, Y> =
+  (<T>() => T extends X ? 1 : 2) extends <T>() => T extends Y ? 1 : 2 ? true : false;
+function assertType<T extends true>(): T | void {}
+type Resolved<F extends (...args: any[]) => any> = Awaited<ReturnType<F>>;
+type User = { id: string };
+
+assertType<Equal<ReturnType<typeof respond<{ id: number }>>, { id: number }>>();
+const redirected = () => redirect("/login");
+const reloaded = () => reload({ revalidate: "users" });
+assertType<Equal<ReturnType<typeof redirected>, never>>();
+assertType<Equal<ReturnType<typeof reloaded>, never>>();
+// A type parameter defaulting to `never`, not a literal `never` return: the
+// latter would also mark code after a bare call unreachable.
+assertType<Equal<ReturnType<typeof redirect<Response>>, Response>>();
+assertType<Equal<ReturnType<typeof reload<Response>>, Response>>();
+
+// A bare server function's type is the caller's type.
+async function getStory(id: number) {
+  "use server";
+  return respond({ id }, { headers: { "cache-control": "public, max-age=60" } });
+}
+assertType<Equal<Resolved<typeof getStory>, { id: number }>>();
+
+// Control flow on any branch leaves only the value.
+async function rename(id: string) {
+  "use server";
+  if (!id) return redirect("/login");
+  if (id === "stale") return reload({ revalidate: "users" });
+  return respond({ id, renamed: true }, { status: 201 });
+}
+assertType<Equal<Resolved<typeof rename>, { id: string; renamed: boolean }>>();
+
+// Where the context names a type, the helper takes it: an annotated
+// return, or a request handler that must answer with a Response.
+async function annotated(id: string): Promise<User> {
+  "use server";
+  if (!id) return redirect("/login");
+  return { id };
+}
+annotated;
+const handler: (request: Request) => Response = request =>
+  request.headers.has("cookie") ? Response.json({ ok: true }) : redirect("/login");
+handler;
+
+// A component wrapped for its headers is still a component to the caller.
+for (const GET of [clientGET, serverGET]) {
+  const getView = GET(async () => respond(() => "view"));
+  assertType<Equal<Resolved<typeof getView>, () => "view">>();
+}