refactor(server): replace prerender HTML buffering with metadataReady contract by NathanDrake2406 · Pull Request #1809 · cloudflare/vinext

NathanDrake2406 · 2026-06-07T08:51:27Z

Triage / Issue Context

This PR resolves Issue #1795.

Previously, renderAppPageLifecycle() implicitly buffered the entire HTML stream using readStreamAsText(htmlStream) during prerendering in order to discover metadata (such as cache life revalidation and headers) generated during component rendering. While this worked, it forced full stream buffering of the body.

Proposed Changes

We introduce an explicit metadataReady promise contract between the SSR handler (app-ssr-entry.ts) and the app-page rendering lifecycle (app-page-render.ts).

AppSsrRenderResult:
Introduced a unified return result for handleSsr() under app-page-stream.ts:

export type AppSsrRenderResult = {
  htmlStream: ReadableStream<Uint8Array>;
  metadataReady: Promise<void>;
  capturedRscData: Promise<ArrayBuffer> | null;
};

SSR Entry Contract (handleSsr):
- Modified handleSsr() to return AppSsrRenderResult.
- For dynamic requests: It returns immediately with metadataReady: Promise.resolve(), ensuring fully unbuffered progressive streaming.
- For static/prerender requests (waitForAllReady: true): It blocks and awaits htmlStream.allReady inside handleSsr() before piping the HTML stream through the tick-buffered transform pipeline. This guarantees that Fizz HTML is not pulled or transformed before the React rendering tree is fully ready, preventing suspense fallback leaks. The returned metadataReady promise is set to Promise.resolve() since allReady is already resolved.
Lifecycle Integration:
Modified renderAppPageLifecycle() to:
- Await htmlRender.metadataReady and settleCapturedRscRenderForCacheMetadata(htmlRender.capturedRscData) when options.isPrerender === true.
- Completely remove the readStreamAsText and createBufferedHtmlStream logic from the rendering flow.
- Serve the HTML response to the build/prerender engine as a live, unbuffered ReadableStream.
- Perform the route-level Suspense check (capturing redirect() / notFound() thrown during the full SSR pass) after awaiting metadataReady to ensure those errors are correctly captured and processed before the response is returned.
ISR Revalidation:
- Updated app-page-dispatch.ts to pass waitForAllReady: true when rendering fresh cache output to block stream transformations until React is fully ready, preserving correct cache state.
Refactored Compatibility Glue:
- Added a private normalizeAppSsrRenderResult helper and a shared resolvedMetadataReady constant in app-page-stream.ts to normalize return results cleanly, avoiding duplicate compatibility code in renderAppPageHtmlStream and renderAppPageHtmlStreamWithRecovery.

Verification Plan

Added behavior test "streams runtime HTML responses progressively without buffering the body" verifying that runtime requests stream chunks progressively.
Added test "waits for metadataReady to resolve before returning the response in prerender mode".
Added test "captures special errors thrown during the full prerender SSR pass and converts them to 307/404 response".
Ran full toolchain check: vp check (passed with zero lint/formatting/type/knip errors).
Built package: pnpm run build (successful).

Next.js Reference: matches Next.js static rendering flow where allReady is awaited asynchronously for static/ISR targets: Next.js app-render.tsx

… contract

pkg-pr-new · 2026-06-07T08:52:39Z

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@1809

npm i https://pkg.pr.new/vinext@1809

commit: c5f95ea

…ansform when waitForAllReady is true

…ataReady promise resolution

james-elicx · 2026-06-07T20:49:19Z

/bigbonk review for issues

ask-bonk

Review: replace prerender HTML buffering with `metadataReady` contract

Reviewed the diff, traced all handleSsr / renderAppPageHtmlStream callers, and ran the relevant suites locally — tests/app-page-render.test.ts (46), tests/app-page-stream.test.ts (16), and tests/isr-cache.test.ts (64) all pass, and vp check is clean on the four changed source files.

This is a solid change. Removing readStreamAsText + createBufferedHtmlStream from the prerender path means the HTML body is no longer fully buffered inside the lifecycle; it's returned as a live stream and consumed by the prerender driver (build/prerender.ts:781 does await response.text()). The cache-life/metadata mechanism is preserved because:

handleSsr still awaits htmlStream.allReady when waitForAllReady is set (prerender), so the RSC tree is fully rendered and rscErrorTracker is populated before the hasLoadingBoundary special-error check runs.
settleCapturedRscRenderForCacheMetadata(htmlRender.capturedRscData) drives the captured RSC buffer to completion before applyRequestCacheLife, same as the old capturedRscDataRef.value await.

The allReady-await reordering in app-ssr-entry.ts (moved from before the CSS/font setup block to immediately before pipeThrough) is behavior-preserving — the intervening code is synchronous closure/string setup that never pulls from htmlStream.

The type guard isAppSsrRenderResult correctly distinguishes a raw ReadableStream (no htmlStream/metadataReady props) from the new result object, and the normalize* helpers keep the union contained. No callers outside the App Router page path consume handleSsr, so the contract change is well-scoped.

A few minor / non-blocking observations below.

ask-bonk · 2026-06-07T20:53:05Z

+
+        return {
+          htmlStream: finalStream,
+          metadataReady: Promise.resolve(),


The real handleSsr always returns metadataReady: Promise.resolve() — the actual prerender blocking happens via await htmlStream.allReady a few lines up. So await htmlRender.metadataReady in renderAppPageLifecycle (app-page-render.ts:850) is effectively a no-op for the production path; the only thing that exercises a pending metadataReady is the test mock.

That's fine as a contract (it keeps the lifecycle agnostic to where the blocking happens), but it's worth a short comment here explaining that metadataReady resolves eagerly precisely because allReady was already awaited above — otherwise a future reader may assume metadataReady is load-bearing in prod and try to defer the allReady await onto it.

ask-bonk · 2026-06-07T20:53:05Z

                basePath: options.basePath,
                clientTraceMetadata: options.clientTraceMetadata,
                rootParams: options.rootParams,
+                waitForAllReady: true,


Good catch adding waitForAllReady: true to the ISR revalidation render — previously this path did not wait for allReady, so Suspense fallback content could leak into the regenerated cache entry. This is a real correctness improvement and brings revalidation in line with the prerender path.

However, it's a behavioral change that isn't covered by a dedicated test. The new tests in app-page-render.test.ts cover the prerender lifecycle, but not the dispatch revalidation branch. Consider adding a focused test (or extending an isr-cache test) that asserts a revalidated entry does not contain fallback content / that allReady is awaited before the cache write, so this guarantee doesn't silently regress.

ask-bonk · 2026-06-07T20:53:05Z

 } from "./client-reuse-manifest.js";
 import { NO_STORE_CACHE_CONTROL } from "./cache-control.js";
-import { readStreamAsText } from "../utils/text-stream.js";
+


Stray blank line left behind where the readStreamAsText import was removed. Minor, but worth cleaning up:

Suggested change

import { NO_STORE_CACHE_CONTROL } from "./cache-control.js";

import {

ask-bonk · 2026-06-07T20:53:05Z

+      loadSsrHandler: async () => ({
+        async handleSsr(_rscStream, _navContext, _fontData, _options) {
+          // Trigger the captured onError callback representing a component throw
+          if (capturedOnError) {


let capturedOnError: any and _options use any/loose typing. Minor nit for consistency with the rest of the file's typed mocks — consider typing capturedOnError as ((error: unknown, ...args: unknown[]) => void) | null. Not blocking.

ask-bonk · 2026-06-07T20:53:14Z

Review submitted successfully. Here's a summary of my review of PR #1809.

Summary

The PR replaces full HTML stream buffering during prerendering with an explicit metadataReady contract. I reviewed the diff, traced all callers, and ran the relevant test suites locally.

Verdict: solid, well-scoped change. No blocking issues found.

What I verified

Correctness of the core change: Removing readStreamAsText/createBufferedHtmlStream is safe because handleSsr still awaits htmlStream.allReady for prerender (so the RSC tree is fully rendered and the error tracker is populated before the special-error check), and settleCapturedRscRenderForCacheMetadata still drives the captured RSC buffer to completion before applyRequestCacheLife.
allReady reordering in app-ssr-entry.ts is behavior-preserving — the intervening code is synchronous setup that never pulls from the stream.
Type guard / union containment: isAppSsrRenderResult correctly distinguishes raw streams from the new result object; no callers outside the App Router page path consume handleSsr.
Tests: app-page-render.test.ts (46), app-page-stream.test.ts (16), isr-cache.test.ts (64) all pass; vp check clean on changed files.

Feedback posted (all non-blocking)

metadataReady is always Promise.resolve() in the real path (vestigial; actual blocking is via allReady) — suggested a clarifying comment.
The waitForAllReady: true addition to ISR revalidation is a genuine correctness improvement (prevents fallback leaks into cache) but lacks dedicated test coverage.
Stray blank line where the readStreamAsText import was removed (with suggestion).
Minor any typing nit in the new test.

github run

…nits - Add focused test asserting stale HTML cache regeneration passes waitForAllReady: true to handleSsr so Suspense fallbacks never leak into the cache entry - Document why metadataReady resolves eagerly (allReady is already awaited for the prerender path) in app-ssr-entry.ts - Remove stray blank line left by the readStreamAsText import removal - Type capturedOnError in the prerender special-error test instead of any

james-elicx · 2026-06-07T21:18:44Z

/bigbonk review for issues

The dts bundle build (vp pack) failed because pages-page-handler.ts imports PagesPageModule from pages-page-data.js, but the type was not exported, so rolldown-plugin-dts could not resolve it.

james-elicx · 2026-06-07T21:24:37Z

/bigbonk review for issues

refactor(server): replace prerender HTML buffering with metadataReady…

defa703

… contract

NathanDrake2406 marked this pull request as draft June 7, 2026 08:59

NathanDrake2406 added 2 commits June 7, 2026 19:07

fix(server): wait for allReady before piping through tick-buffered tr…

3a8490a

…ansform when waitForAllReady is true

refactor(server): clean up AppSsrRenderResult normalization and metad…

7b90b26

…ataReady promise resolution

NathanDrake2406 marked this pull request as ready for review June 7, 2026 09:19

ask-bonk Bot reviewed Jun 7, 2026

View reviewed changes

james-elicx added 2 commits June 7, 2026 22:21

Merge remote-tracking branch 'origin/main' into fix/issue-1795

db7a51c

fix(pages): export PagesPageModule type consumed by pages-page-handler

c5f95ea

The dts bundle build (vp pack) failed because pages-page-handler.ts imports PagesPageModule from pages-page-data.js, but the type was not exported, so rolldown-plugin-dts could not resolve it.

james-elicx enabled auto-merge (squash) June 7, 2026 21:26

james-elicx merged commit 7d280ed into cloudflare:main Jun 7, 2026
39 checks passed

NathanDrake2406 deleted the fix/issue-1795 branch June 13, 2026 13:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(server): replace prerender HTML buffering with metadataReady contract#1809

refactor(server): replace prerender HTML buffering with metadataReady contract#1809
james-elicx merged 6 commits into
cloudflare:mainfrom
NathanDrake2406:fix/issue-1795

NathanDrake2406 commented Jun 7, 2026 •

edited

Loading

Uh oh!

pkg-pr-new Bot commented Jun 7, 2026 •

edited

Loading

Uh oh!

james-elicx commented Jun 7, 2026

Uh oh!

ask-bonk Bot left a comment

Uh oh!

ask-bonk Bot Jun 7, 2026

Uh oh!

ask-bonk Bot Jun 7, 2026

Uh oh!

ask-bonk Bot Jun 7, 2026

Uh oh!

ask-bonk Bot Jun 7, 2026

Uh oh!

ask-bonk Bot commented Jun 7, 2026

Uh oh!

james-elicx commented Jun 7, 2026

Uh oh!

james-elicx commented Jun 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


	import { NO_STORE_CACHE_CONTROL } from "./cache-control.js";
	import {

Uh oh!

Conversation

NathanDrake2406 commented Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Triage / Issue Context

Proposed Changes

Verification Plan

Uh oh!

pkg-pr-new Bot commented Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

james-elicx commented Jun 7, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Review: replace prerender HTML buffering with metadataReady contract

Uh oh!

ask-bonk Bot Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Jun 7, 2026

Summary

What I verified

Feedback posted (all non-blocking)

Uh oh!

james-elicx commented Jun 7, 2026

Uh oh!

james-elicx commented Jun 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NathanDrake2406 commented Jun 7, 2026 •

edited

Loading

pkg-pr-new Bot commented Jun 7, 2026 •

edited

Loading

Review: replace prerender HTML buffering with `metadataReady` contract