fix(web): dispose renderToString's root synchronously, committing the response head first (#3385)

.changeset/render-to-string-sync-dispose.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/web": patch
+---
+
+`renderToString` now disposes its reactive root synchronously before returning instead of via `setTimeout`, so a synchronous loop of renders no longer retains every graph until the next macrotask (#3385). The request event's response head is committed right before that dispose — the same head-freeze point an awaited `renderToStream` already uses — so `httpStatus`/`httpHeader` declarations still reach `createSSRResponse`. A render that throws leaves the head uncommitted and retracts its declarations as before.

packages/web/src/index.server.ts

@@ -322,11 +322,11 @@ const headerLedgers = /* @__PURE__ */ new WeakMap<ResponseStub, Map<string, Head
  * cleanup are no-ops once the response head is `committed` (head
  * derived/sent — status can no longer change). The head commits when it
  * freezes: at shell flush for a piped `renderToStream` (`createSSRResponse`
- * commits on the first write), at completion for an awaited one (the render
- * commits right before its final dispose, so the declarations still live at
- * that point are the ones the consumer derives the head from), and when
- * `createSSRResponse` receives a `renderToString` result. On the client this
- * is a no-op.
+ * commits on the first write), and at completion for an awaited one or for
+ * `renderToString` (the render commits right before its final dispose, so
+ * the declarations still live at that point are the ones the consumer
+ * derives the head from; `createSSRResponse` then passes the committed stub
+ * through). On the client this is a no-op.
  */
 export function httpStatus(code: number, text?: string): void {
   // `response` is an integration-augmented field (see core's ResponseStub);
@@ -380,9 +380,8 @@ export function httpStatus(code: number, text?: string): void {
  * header). Both the write and the cleanup are no-ops once the response head
  * is `committed` (head derived/sent — headers can no longer change); the
  * head commits when it freezes — shell flush for a piped `renderToStream`,
- * completion (right before the final dispose) for an awaited one,
- * `createSSRResponse` for a `renderToString` result — see `httpStatus`. On
- * the client this is a no-op.
+ * completion (right before the final dispose) for an awaited one and for
+ * `renderToString` — see `httpStatus`. On the client this is a no-op.
  */
 export function httpHeader(name: string, value: string, options?: { append?: boolean }): void {
   const event = getRequestEvent() as (RequestEvent & { response?: ResponseStub }) | undefined;

packages/web/src/index.ts

@@ -548,8 +548,8 @@ export function clientOnly<T extends Component<any>>(
  * then recovered retracts its write instead of stomping a status a
  * surviving part of the tree legitimately set. Once the response head is
  * `committed` (head derived/sent — the shell flush of a piped
- * `renderToStream`, the completion of an awaited one, `createSSRResponse`
- * for a `renderToString` result), writes and retractions are no-ops.
+ * `renderToStream`, the completion of an awaited one or of
+ * `renderToString`), writes and retractions are no-ops.
  */
 export function httpStatus(_code: number, _text?: string): void {}
 
@@ -569,8 +569,7 @@ export function httpStatus(_code: number, _text?: string): void {}
  * write time and restored when the owning scope is disposed (deleted if
  * there was none) — a boundary that errors or recovers retracts its writes.
  * Once the response head is `committed` (head derived/sent — the shell
- * flush of a piped `renderToStream`, the completion of an awaited one,
- * `createSSRResponse` for a `renderToString` result), writes and
- * retractions are no-ops.
+ * flush of a piped `renderToStream`, the completion of an awaited one or
+ * of `renderToString`), writes and retractions are no-ops.
  */
 export function httpHeader(_name: string, _value: string, _options?: { append?: boolean }): void {}

packages/web/src/server.ts

@@ -1562,41 +1562,59 @@ export function renderToString(code, options = {}) {
   registerEntryAssets(manifest);
   // The trace this render belongs to (see `getTraceContext`): the request's
   // under a request scope, the render's own otherwise — cleared with the
-  // render's deferred dispose so a later read outside any render does not
-  // find a stale one on the lingering context.
+  // render's dispose so a later read outside any render does not find a
+  // stale one on the lingering context.
   const context = sharedConfig.context;
   const requestEvent = peekRequestEvent();
   context.trace = requestEvent ? traceForEvent(requestEvent) : traceFor(context, undefined);
-  let html = root(
-    d => {
-      setTimeout(() => {
-        context.trace = undefined;
-        d();
-      });
-      return resolveSSRSync(escape(code()));
-    },
-    { id: renderId }
-  );
-  serializeFragmentAssets("", tracking.boundaryModules, sharedConfig.context, renderId);
-  sharedConfig.context.noHydrate = true;
-  serializer.close();
-  const head = renderShellHead(
-    headRegistry,
-    nonce,
-    null,
-    noScripts,
-    traceMetaMarkup(context.trace)
-  );
-  return assembleDocument(
-    resolveSSRSelectValues(html),
-    tracking.emittedAssets,
-    tracking.preloadLinks,
-    tracking.inlineStyles,
-    scripts.length ? scripts : "",
-    nonce,
-    head,
-    onHead
-  );
+  let dispose;
+  try {
+    const html = root(
+      d => {
+        dispose = d;
+        return resolveSSRSync(escape(code()));
+      },
+      { id: renderId }
+    );
+    serializeFragmentAssets("", tracking.boundaryModules, sharedConfig.context, renderId);
+    sharedConfig.context.noHydrate = true;
+    serializer.close();
+    const head = renderShellHead(
+      headRegistry,
+      nonce,
+      null,
+      noScripts,
+      traceMetaMarkup(context.trace)
+    );
+    const document = assembleDocument(
+      resolveSSRSelectValues(html),
+      tracking.emittedAssets,
+      tracking.preloadLinks,
+      tracking.inlineStyles,
+      scripts.length ? scripts : "",
+      nonce,
+      head,
+      onHead
+    );
+    // Head-freeze point: the request's response head commits right before
+    // the render's final dispose — the same order as an awaited
+    // `renderToStream`'s completion — so the `httpStatus`/`httpHeader`
+    // declarations still live at completion survive into
+    // `createSSRResponse(html, event)`, which passes the committed stub
+    // through. A render that threw leaves the head open: its declarations
+    // retract with the dispose below, and the handler's error path may
+    // still write.
+    if (requestEvent && requestEvent.response) {
+      commitResponseStub(requestEvent.response, { event: requestEvent });
+    }
+    return document;
+  } finally {
+    // Release the graph before returning (#3385): a deferred dispose held
+    // every root — and every memo under it — until the next macrotask, so
+    // nothing was freed across a synchronous loop of renders.
+    context.trace = undefined;
+    if (dispose) dispose();
+  }
 }
 export function renderToStream<T>(
   fn: () => T,
@@ -5083,11 +5101,12 @@ export function createSSRResponse(
  *
  * - String results commit the stub and return a `Response` synchronously;
  *   a `Location` on the stub becomes a real redirect
- *   (`getExpectedRedirectStatus`) instead of an HTML response. An awaited
- *   `renderToStream(...)` result arrives with its stub ALREADY committed —
- *   the render froze the head at completion, before its final dispose, so
- *   `httpStatus`/`httpHeader` declarations survive into the derived head —
- *   and the commit here is an idempotent pass-through for it.
+ *   (`getExpectedRedirectStatus`) instead of an HTML response. A
+ *   `renderToString(...)` or awaited `renderToStream(...)` result rendered
+ *   under this event's request scope arrives with its stub ALREADY
+ *   committed — the render froze the head at completion, before its final
+ *   dispose, so `httpStatus`/`httpHeader` declarations survive into the
+ *   derived head — and the commit here is an idempotent pass-through for it.
  * - Stream results (`renderToStream(...)`) resolve at shell flush — the
  *   moment the head freezes: the stub is committed there (post-commit
  *   header writes fail loudly — see `commitResponseStub`), its

packages/web/test/server/http-components.spec.tsx

@@ -77,8 +77,8 @@ describe("httpStatus (server primitive)", () => {
     storage.run(event, () => {
       renderToString(() => <Page />);
     });
-    // Read synchronously after render — renderToString defers its dispose to
-    // a macrotask, so the write is still in place for the integration.
+    // Read synchronously after render — renderToString commits the head right
+    // before its dispose, so the declaration survives for the integration.
     expect(event.response!.status).toBe(404);
     expect(event.response!.statusText).toBe("Not Found");
   });

packages/web/test/server/render-to-string-dispose.spec.tsx

@@ -0,0 +1,96 @@
+/**
+ * @jsxImportSource @solidjs/web
+ *
+ * `renderToString` releases its reactive graph before it returns (#3385).
+ * The root used to be disposed via `setTimeout`, so every render in a
+ * synchronous loop (benchmarks, batch pre-rendering, `Promise.all` over
+ * many renders) was retained until the task yielded. The scope-tied
+ * response primitives must still survive the render: the request event's
+ * head freezes right before the dispose, as an awaited `renderToStream`
+ * already does, so `httpStatus`/`httpHeader` declarations are not retracted
+ * and `createSSRResponse` sees them.
+ */
+import { AsyncLocalStorage } from "node:async_hooks";
+import { afterAll, beforeAll, describe, expect, test } from "vitest";
+import {
+  createRequestEvent,
+  createSSRResponse,
+  getTraceContext,
+  httpHeader,
+  httpStatus,
+  renderToString
+} from "@solidjs/web";
+import type { RequestEvent, ResponseStub } from "@solidjs/web";
+import { onCleanup } from "solid-js";
+
+type HttpEvent = RequestEvent & { response: ResponseStub };
+
+const RequestContext = Symbol.for("solid.RequestContext");
+let storage: AsyncLocalStorage<HttpEvent>;
+
+beforeAll(() => {
+  storage = new AsyncLocalStorage();
+  (globalThis as any)[RequestContext] = storage;
+});
+
+afterAll(() => {
+  delete (globalThis as any)[RequestContext];
+});
+
+describe("renderToString disposes its root synchronously (#3385)", () => {
+  test("every render's cleanup has run before the next synchronous render starts", () => {
+    let disposed = 0;
+    const Page = () => {
+      onCleanup(() => disposed++);
+      return <div>page</div>;
+    };
+    for (let i = 0; i < 5; i++) {
+      expect(disposed).toBe(i);
+      const html = renderToString(() => <Page />);
+      expect(html).toContain("page");
+      expect(disposed).toBe(i + 1);
+    }
+    expect(disposed).toBe(5);
+  });
+
+  test("a render that throws still disposes what it created", () => {
+    let disposed = 0;
+    const Page = () => {
+      onCleanup(() => disposed++);
+      throw new Error("render failed");
+    };
+    expect(() => renderToString(() => <Page />)).toThrow("render failed");
+    expect(disposed).toBe(1);
+  });
+
+  test("the render's own trace is gone as soon as the render returns", () => {
+    let inside: unknown;
+    const Reader = () => {
+      inside = getTraceContext();
+      return <span>t</span>;
+    };
+    renderToString(() => <Reader />);
+    expect(inside).toBeDefined();
+    expect(getTraceContext()).toBeUndefined();
+  });
+
+  test("httpStatus/httpHeader declarations survive the synchronous dispose", async () => {
+    const evt = createRequestEvent(new Request("https://app.example/")) as HttpEvent;
+    let disposed = 0;
+    const Page = () => {
+      onCleanup(() => disposed++);
+      httpStatus(404, "Not Found");
+      httpHeader("cache-control", "no-store");
+      return <div>not found</div>;
+    };
+    const html = storage.run(evt, () => renderToString(() => <Page />));
+    expect(disposed).toBe(1);
+    expect(evt.response.status).toBe(404);
+    expect(evt.response.statusText).toBe("Not Found");
+    expect(evt.response.headers.get("cache-control")).toBe("no-store");
+    const response = createSSRResponse(html, evt);
+    expect(response.status).toBe(404);
+    expect(response.headers.get("cache-control")).toBe("no-store");
+    expect(await response.text()).toContain("not found");
+  });
+});