fix: don't await createFromReadableStream before hydrateRoot (#695)

[southpolesteve]

Summary

Fixes #695 -- useEffect callbacks never firing after RSC hydration on Cloudflare Workers.

• createFromReadableStream returns a React thenable, not a resolved component tree. Awaiting it before passing to hydrateRoot blocked hydration until the entire RSC stream was consumed, which prevented passive effects from ever being scheduled.
• In production on Workers, RSC chunks arrive progressively via injected <script> tags, so the stream takes longer to close -- making the bug reliably reproducible there but not locally (where RSC data is available synchronously in window.__VINEXT_RSC__).
• The fix is one line: remove the await so the thenable is passed directly to hydrateRoot, matching the React RSC contract and the pattern already used in app-ssr-entry.ts.

Changes

• packages/vinext/src/server/app-browser-entry.ts -- remove await on createFromReadableStream (line 182)
• tests/fixtures/cf-app-basic/app/effect-test/page.tsx -- new "use client" fixture page that uses useEffect to flip a status flag
• tests/e2e/cloudflare-workers/hydration.spec.ts -- regression test that verifies useEffect fires after RSC hydration on the Workers e2e project

[pkg-pr-new]

Open in StackBlitz

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

commit: 96e238e

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original

[southpolesteve]

/bigbonk review please

[ask-bonk]

Review

Core fix: correct and well-motivated

The one-line fix at app-browser-entry.ts:182 is correct. createFromReadableStream returns a React thenable, not a resolved component tree. The RSC contract requires passing this thenable directly to hydrateRoot, which suspends on it internally. Awaiting it first blocks the entire hydration pipeline until the RSC stream finishes consuming, which prevents React from ever scheduling passive effects (useEffect).

This matches the pattern already used in app-ssr-entry.ts:174:

flightRoot = createFromReadableStream(ssrStream); // no await

The regression test (e2e + fixture page) is well-targeted: a "use client" component with useEffect(() => setFired(true), []) that flips a data-testid from "effect-pending" to "effect-fired".

Refactor: clean extraction following AGENTS.md guidance

The bulk of the diff extracts inline generated code from pages-server-entry.ts into typed helper modules (pages-node-compat.ts, pages-api-route.ts), which is exactly what AGENTS.md recommends:

generated code may describe route-specific imports, manifests, and thin closures, but it should not own substantial request lifecycle logic

Positive details:

• ISR cache helpers now delegate to the shared isr-cache.ts instead of duplicating fnv1a64, pendingRegenerations, and getCacheHandler() calls inline
• Uses the existing addQueryParam utility from utils/query.ts instead of reimplementing the duplicate-key promotion logic
• Buffer → new Uint8Array(buffer) in send() is more portable to Workers
• pages-api-route.test.ts has solid coverage: query merging, JSON parse errors, urlencoded bodies, Buffer sends, error reporting, body size limits

Minor nits (non-blocking)

1. Fragile string-based error matching (pages-api-route.ts:76): (error as Error).message === "Request body too large" is brittle. A dedicated error class (like PagesApiBodyParseError) would be more robust. This matches the pre-existing behavior though, so not a blocker.

2. parsePagesApiBody discards stack trace (pages-node-compat.ts:108-109): The catch re-throws new Error("Request body too large") instead of re-throwing the original error. Since this is a fresh extraction, it'd be a good time to preserve the original error, but again non-blocking.

3. Thin ISR wrappers in the generated entry are technically redundant indirection (function isrGet(key) { return __sharedIsrGet(key); }), but I assume they're intentional to minimize changes to existing call sites within the template. Fine as-is.

LGTM -- the fix is clearly correct, the regression test covers the exact failure mode, and the refactor improves maintainability.

[ask-bonk]

Review posted and approved on PR #700. Here's a summary of the review:

Verdict: APPROVED

The PR has two parts:

1. Core fix (app-browser-entry.ts:182): Removes await from createFromReadableStream() so the React thenable is passed directly to hydrateRoot(). This matches the RSC contract and the existing pattern in app-ssr-entry.ts:174. The bug caused useEffect callbacks to never fire on Workers because awaiting the stream blocked hydration until the entire RSC payload was consumed.

2. Refactor: Extracts ~250 lines of inline code from the generated pages-server-entry.ts into typed helper modules (pages-node-compat.ts, pages-api-route.ts), following the AGENTS.md guidance that generated entries should stay thin. ISR cache helpers now delegate to the shared isr-cache.ts module instead of duplicating the implementation.

Three non-blocking nits noted in the review body:

• Fragile string-based error matching for "Request body too large" in pages-api-route.ts:76
• Stack trace discarded when re-throwing body-too-large errors in pages-node-compat.ts:108-109
• Thin ISR wrapper functions in the generated entry are technically redundant but likely intentional

github run