fix(pages): align prerender functional parity

[james-elicx]

Summary

• match Next deploy browser Cache-Control for Pages SSG/ISR HTML and _next/data responses
• hydrate fallback shells through the server-resolved Pages route URL and avoid hard reloads for failed internal _h hydration data updates
• gate Pages props serializability validation like Next.js production SSR, and add Pages API res.setPreviewData() cookies

Failure mapping

• Covered here: test/e2e/prerender.test.ts cache header rows for revalidate/fallback/no-revalidate pages, fallback dynamic SSG rewrite, invalid JSON from getStaticProps on SSR/CST, on-demand revalidate with preview cookie, and no prerender data fetch on mount.
• Covered by fix(pages): preserve prerender rewrite query parity #2218: should not supply query values to params or useRouter non-dynamic page SSR. Existing coverage comments are already present there.
• Covered by fix(pages): align on-demand ISR regeneration semantics #2027: should not revalidate when set to false. Added a coverage note: fix(pages): align on-demand ISR regeneration semantics #2027 (comment)

Validation

• vp test run tests/cache-control.test.ts tests/pages-page-response.test.ts tests/pages-page-data.test.ts tests/pages-node-compat.test.ts tests/entry-templates.test.ts
• vp check tests/cache-control.test.ts tests/pages-page-response.test.ts tests/pages-page-data.test.ts tests/pages-node-compat.test.ts tests/entry-templates.test.ts
• REPO="$(pwd)" NEXTJS_DIR="/Users/jamesanderson/Developer/vinext/.nextjs-ref" NEXT_TEST_CONCURRENCY=1 ./scripts/run-targeted-nextjs-e2e.sh test/e2e/prerender.test.ts
  • Result: 61 passed, 2 failed; the remaining failures are the rows mapped to fix(pages): preserve prerender rewrite query parity #2218 and fix(pages): align on-demand ISR regeneration semantics #2027 above.

Artifacts from the local E2E/build run were removed after validation.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

commit: 233fb2c

[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
workers-cache	preview	production
web	preview	production

[james-elicx]

/bigbonk review for issues

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #2471 (fix(pages): align prerender functional parity)

Reviewed all 13 changed files against Next.js canary source. The three main changes are well-founded and match Next.js behavior. One incomplete-feature concern and a couple of minor notes below.

Verified correct

• Fallback hydration routeUrl (pages-client-entry.ts + pages-page-handler.ts). Passing the server-resolved routeUrl as the navigation url and the browser location as as matches Next.js's fallback hydration in packages/next/src/client/index.tsx:130-153, where it calls router.replace(router.pathname + '?' + query, asPath, { _h: 1 }). Previously vinext used the browser location for both, which fetched data for the wrong URL under rewrites. Good fix.

• Non-fatal _h hydration data failures (shims/router.ts). Returning early on !res.ok / invalid JSON when isHydrationQueryUpdate matches Next.js's router.ts:2123-2131 ("we don't hard error during query updating ... return null"). Verified the early return still lets performNavigation run onStateUpdate?.() (via runNavigateClient → "completed"), so isReady is still flipped and the router isn't left in a broken state.

• VINEXT_NEXT_DEPLOY_CACHE_CONTROL gate (cache-control.ts). The env flag is deliberately set only in scripts/e2e-deploy.sh:640 to match Vercel-deploy-style browser Cache-Control for SSG/ISR HTML and _next/data when running the Next.js E2E harness. This is an intentional, documented divergence, not dead code — good.

• Props-serialization gating (pages-page-data.ts + handler). validatePropsSerialization: NODE_ENV !== "production" || VINEXT_PRERENDER === "1" mirrors Next.js only validating serializable props during dev/build, not production SSR. Matches render.tsx.

Concern — res.setPreviewData() writes cookies nothing consumes

setPreviewData (pages-node-compat.ts:338-354) emits __prerender_bypass and __next_preview_data cookies, but no code path reads __next_preview_data to populate context.preview / context.previewData in getStaticProps. The getStaticProps context built at pages-page-data.ts:980-990 has no preview/previewData fields, and a repo-wide search finds no reader for __next_preview_data. So calling res.setPreviewData() sets the cookies but preview mode never actually activates on subsequent requests.

Two sub-points:

1. This is a partial feature. If the intent is "align functional parity," calling setPreviewData() and then loading the page should skip the SSG cache and re-run getStaticProps with context.preview === true. That half isn't wired up. Consider either wiring the reader in this PR or filing a tracking issue and noting the limitation.
2. Payload format differs from Next.js. Next.js signs __next_preview_data as an encrypted JWT (api-resolver.ts:187-201); vinext stores raw base64url(JSON). That's fine as an internal representation as long as the future reader decodes the same way — but it means these cookies are not interchangeable with a real Next.js deployment, and the 2KB limit check operates on a smaller payload than Next.js's signed token. Worth a comment documenting the deliberate difference (similar to the getRevalidateSecret ↔ previewModeId note in isr-cache.ts).

Minor

• Header-merge key casing (pages-page-handler.ts:715-717). Iterating a Headers object yields lowercased keys (cache-control), which are merged into init.headers. Here it's safe because in the else if (isStaticPropsRoute) branch gsspRes is null so init.headers starts empty — no pre-existing capitalized Cache-Control to leave stranded. Fine as-is, but fragile if the branch conditions ever change; a short comment noting the empty-headers precondition would help.

• Test coverage. tests/pages-node-compat.test.ts only asserts the two preview cookies are appended and existing cookies preserved. It doesn't assert HttpOnly/Secure/SameSite/Path/Max-Age attributes or the 2KB-limit throw. Consider adding those, plus a test that setPreviewData actually enables preview mode end-to-end once the reader exists.

Overall the cache-control, hydration-URL, and props-validation changes are solid and match Next.js. The setPreviewData addition is the one piece that is functionally incomplete (cookies with no consumer) and should be flagged as such.

[ask-bonk]

Review posted to PR #2471. Summary of findings:

Review Summary

Verified correct (3 core changes match Next.js):

• Fallback hydration routeUrl — passing server-resolved route URL as url and browser location as as matches Next.js client/index.tsx:130-153. Fixes data-fetch under rewrites.
• Non-fatal _h hydration failures — matches Next.js router.ts:2123-2131 ("don't hard error during query updating"). Confirmed the early return still runs onStateUpdate(), so isReady isn't left stuck.
• VINEXT_NEXT_DEPLOY_CACHE_CONTROL gate — intentional flag set in scripts/e2e-deploy.sh:640 for the Next.js E2E harness, not dead code.
• Props-serialization gating — correctly mirrors Next.js only validating in dev/build, not prod SSR.

Main concern:

• res.setPreviewData() writes cookies nothing consumes. No code reads __next_preview_data to populate context.preview/previewData in getStaticProps, so preview mode never activates. Also the payload format (raw base64url(JSON)) diverges from Next.js's signed JWT without a documenting comment. Recommended wiring the reader or filing a tracking issue.

Minor:

• Header-merge key casing in the handler is safe only because init.headers starts empty in that branch — worth a comment.
• Test coverage for setPreviewData doesn't assert cookie attributes or the 2KB throw.

All relevant tests pass (117 + 40) and type check is clean.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #2471 (fix(pages): align prerender functional parity)

Re-reviewed against the current head commit (b75d46b). Note: the previous bonk review was against an earlier version where res.setPreviewData() wrote cookies with no consumer. That gap is now closed — this PR wires the reader (getPagesPreviewDataFromCookieHeader / getPagesPreviewData) into both the dev path (dev-server.ts) and the prod path (pages-page-handler.ts → resolvePagesPageData), and populates context.preview / context.previewData for getStaticProps and getServerSideProps. Preview mode now actually activates. Good — and dev/prod parity is maintained.

Verified correct

• Preview reader end-to-end. getPagesPreviewData(request, { isOnDemandRevalidate }) is threaded through createPagesPageHandler (pages-page-handler.ts:602) and the dev handler (dev-server.ts:933, 1094). The previewData === false guards consistently skip SSG/ISR cache hits, stale serving, generatedFromDataRequest reuse, isrRevalidateSeconds capture, and shouldPersistFallbackData writes — matching Next.js, which bypasses the ISR cache entirely under preview. Empty-object {} (bypass cookie present, no payload) correctly activates preview with previewData: {}.

• On-demand-revalidate disables preview. getPagesPreviewDataFromCookieHeader returns false when isOnDemandRevalidate is set (pages-node-compat.ts:211), so preview cookies can't poison a regenerated ISR entry. Matches Next.js. Well-commented.

• Cookie round-trip is symmetric. serializePreviewCookie encodeURIComponents the value; parseCookieHeader decodeURIComponents on read (utils/parse-cookie.ts:7). base64url payloads survive intact.

• Fallback hydration routeUrl. Passing server-resolved routeUrl as url and browser location as as (pages-client-entry.ts:298-303) matches Next.js client/index.tsx fallback replace, fixing data-fetch under rewrites.

• Non-fatal _h hydration failures (shims/router.ts:2172-2174, 2197-2199) match Next.js router.ts "don't hard error during query updating". performNavigation still runs onStateUpdate so isReady isn't stranded.

• VINEXT_NEXT_DEPLOY_CACHE_CONTROL gate is an intentional E2E-harness flag, not dead code.

• Props-serialization gating (validatePropsSerialization: NODE_ENV !== "production" || VINEXT_PRERENDER === "1") mirrors Next.js only validating in dev/build, not prod SSR.

All 121 relevant tests pass; vp check (format + type + lint) is clean.

Concern — non-constant-time secret comparison

getPagesPreviewDataFromCookieHeader compares the bypass cookie against the revalidate secret with plain !==:

if (bypass !== getRevalidateSecret()) return false;   // pages-node-compat.ts:217

isr-cache.ts deliberately exposes safeEqual (constant-time) and uses it for the same secret in isOnDemandRevalidateRequest specifically to avoid a timing side-channel. Using !== here reintroduces that channel for the preview-bypass path. Recommend importing and using safeEqual for parity with the existing on-demand-revalidate check.

(For context: App Router draft mode's isDraftModeRequest also uses === on a per-request random UUID — a weaker target — but the Pages preview cookie is checked against the long-lived build-baked secret, which is the more sensitive comparison. Worth fixing here even if the App-router one is pre-existing.)

Minor

• Shared __prerender_bypass cookie, different secrets. App Router draft mode signs __prerender_bypass with a per-context draftModeSecret (random UUID); Pages preview signs it with getRevalidateSecret() (build-baked). Next.js uses one previewModeId for both. Since a route is either App or Pages this doesn't misfire in practice, but the two systems can't share the cookie. Pre-existing divergence — worth a note but not a blocker for this PR.

• setPreviewData(false) / non-object payloads. decodePagesPreviewPayload returns String(value) for non-objects, so setPreviewData(false) round-trips to the string "false". Preview data is almost always an object, so low-impact, but a short comment or normalizing to {} for primitives would avoid surprise.

• Header-merge casing (pages-page-handler.ts:717-718). Merging the lowercased cache-control key from applyCdnResponseHeaders back into init.headers is safe only because init.headers starts empty in the else if (isStaticPropsRoute) branch (gSSP null → no capitalized Cache-Control). Confirmed safe today; a one-line comment noting the empty-headers precondition would guard against future edits.

• Test coverage. tests/pages-node-compat.test.ts now asserts the round-trip read (good), but doesn't assert cookie attributes (HttpOnly/Secure/SameSite/Path/Max-Age), the 2KB-limit throw, or the wrong-secret rejection (bypass !== secret → false). Adding a wrong-secret case would also lock in the constant-time fix above.

Overall: the functional-parity changes (preview mode wiring, cache-control, hydration URL, props validation) are solid and match Next.js. The one thing I'd change before merge is the constant-time secret comparison.

[ask-bonk]

Review posted to PR #2471.

Review Summary

I re-reviewed against the current head commit. Key difference from the prior bonk review: the PR has since been updated — the previous review's main concern ("setPreviewData() writes cookies nothing consumes") is now resolved. The reader is fully wired into both dev and prod Pages paths, so preview mode actually activates and gates ISR caching.

Verified correct:

• Preview reader threaded through dev + prod (dev/prod parity maintained), with consistent previewData === false guards bypassing SSG/ISR cache
• On-demand-revalidate disables preview (can't poison regenerated entries)
• Cookie encode/decode round-trip is symmetric
• Fallback hydration routeUrl, non-fatal _h failures, cache-control gate, props-validation gating all match Next.js
• 121 tests pass, vp check clean

Main concern (only pre-merge change I'd request):

• Non-constant-time secret comparison at pages-node-compat.ts:217 (bypass !== getRevalidateSecret()). The codebase deliberately exposes safeEqual and uses it for the same build-baked secret in isOnDemandRevalidateRequest. Using !== reintroduces a timing side-channel. Recommend safeEqual.

Minor: shared __prerender_bypass cookie name with different secrets between App/Pages (pre-existing), setPreviewData(false) primitive round-trip quirk, header-merge casing precondition comment, and missing wrong-secret/attribute test coverage.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #2471 (fix(pages): align prerender functional parity)

Re-reviewed against the current head (816050c, which added fix(pages): harden preview bypass comparison on top of b75d46b). The prior bonk review's one pre-merge concern — the non-constant-time secret comparison — is now resolved. All changes match Next.js behavior. No blocking issues found.

Concern from prior review is fixed

The previous review flagged bypass !== getRevalidateSecret() as a timing side-channel. The new commit extracts a shared isRevalidateSecret() helper in isr-cache.ts that routes through the existing constant-time safeEqual, and both getPagesPreviewDataFromCookieHeader and isOnDemandRevalidateRequest now use it. Preview-bypass and on-demand-revalidate auth are now checked identically and in constant time. tests/pages-node-compat.test.ts adds a wrong-secret rejection case that locks this in.

Verified correct

• Preview reader wired dev + prod (parity). getPagesPreviewData is threaded through createPagesPageHandler (pages-page-handler.ts:602), and getPagesPreviewDataFromCookieHeader through the dev gSP/gSSP paths (dev-server.ts:933, 1094). The prod path is the same code the Cloudflare worker and Node prod server delegate to via pages-server-entry.ts → createPagesPageHandler, so all three request paths (dev, prod, worker) inherit the reader. Confirmed no separate Pages SSR logic exists in worker-entry.ts or prod-server.ts.
• previewData === false guards are consistent. Fresh-hit, stale-hit, generatedFromDataRequest reuse, isrRevalidateSeconds capture, and shouldPersistFallbackData writes all bypass SSG/ISR when preview is active, in both pages-page-data.ts and dev-server.ts (incl. the staticPropsPreviewData === false guard on the dev _next/data persist path at dev-server.ts:1528). Empty-object {} (bypass cookie, no payload) correctly activates preview without hitting === false.
• On-demand-revalidate disables preview. getPagesPreviewDataFromCookieHeader returns false when isOnDemandRevalidate is set (pages-node-compat.ts:184) so preview cookies can't poison a regenerated ISR entry. Well-commented, matches Next.js.
• Non-fatal _h hydration failures (shims/router.ts:2173-2174, 2198-2199). Traced the flow: on early return, navigateClient resolves normally → runNavigateClient returns "completed" → performNavigation still runs onStateUpdate?.() at router.ts:3136, so isReady is flipped and the router isn't stranded. Matches Next.js "don't hard error during query updating".
• Fallback hydration routeUrl (pages-client-entry.ts:298-303, vinext-next-data.ts). Passing server-resolved routeUrl as url and browser location as as matches Next.js client/index.tsx, fixing data-fetch under rewrites.
• VINEXT_NEXT_DEPLOY_CACHE_CONTROL gate (cache-control.ts) is set only in scripts/e2e-deploy.sh:640 (prod/deploy harness). The dev-server not applying BROWSER_REVALIDATE_CACHE_CONTROL on _next/data is therefore not a dev/prod parity bug — the flag never runs in dev. Intentional E2E divergence.
• Props-serialization gating (validatePropsSerialization: NODE_ENV !== "production" || VINEXT_PRERENDER === "1") mirrors Next.js validating serializable props in dev/build only, not prod SSR.

All 162 relevant tests pass (pages-node-compat, cache-control, pages-page-data, pages-page-response, entry-templates); vp check (format + lint + type) is clean.

Minor (non-blocking)

• setPreviewData(false) / non-object payloads. decodePagesPreviewPayload returns String(value) for primitives, so setPreviewData(false) round-trips to the string "false" (which is !== false, so preview stays active rather than being disabled). Preview data is almost always an object, so low-impact, but normalizing primitives to {} — or a short comment — would avoid the surprise.
• Header-merge casing (pages-page-handler.ts:717-719). Merging the lowercased cache-control key from the Headers iterator back into init.headers is safe only because the else if (isStaticPropsRoute) branch runs when gsspRes is null, so init.headers starts empty (the gSSP header-copy block at 687-693 is skipped) — no capitalized Cache-Control to strand. Confirmed safe today; a one-line precondition comment would guard against future edits to the branch conditions.
• Shared __prerender_bypass cookie, different secrets. App Router draft mode signs it with a per-context random UUID; Pages preview signs it with getRevalidateSecret(). Next.js uses one previewModeId for both. A route is only ever App or Pages so it doesn't misfire, but the two systems can't interop. Pre-existing divergence — worth a note, not a blocker.

Overall: solid PR. The functional-parity changes match Next.js, dev/prod/worker parity is maintained, and the constant-time comparison from the prior review is fixed. No changes required before merge.

[ask-bonk]

Review posted to PR #2471.

Review Summary

I re-reviewed against the current head (816050c), which added a commit on top of the previously-reviewed b75d46b.

Prior concern resolved: The non-constant-time secret comparison flagged in the last bonk review (bypass !== getRevalidateSecret()) is fixed. A shared isRevalidateSecret() helper now routes both the preview-bypass check and isOnDemandRevalidateRequest through the constant-time safeEqual, and a wrong-secret test case locks it in.

Verified correct:

• Preview reader wired into dev + prod + worker paths (all three delegate to the same createPagesPageHandler) — parity maintained
• previewData === false guards consistently bypass SSG/ISR cache hits, stale serving, data-request reuse, and fallback persistence
• On-demand-revalidate disables preview (can't poison regenerated entries)
• Non-fatal _h hydration failures still run onStateUpdate, so isReady isn't stranded (traced the call flow)
• Fallback hydration routeUrl, cache-control gate, and props-validation gating all match Next.js
• The VINEXT_NEXT_DEPLOY_CACHE_CONTROL dev/prod difference is not a parity bug — the flag only runs in the E2E deploy harness

Verification: 162 relevant tests pass; vp check clean.

Minor (non-blocking): setPreviewData(false) primitive round-trip quirk, header-merge casing precondition comment, and the pre-existing shared __prerender_bypass cookie name with different secrets between App/Pages.

No changes required before merge.

github run

[james-elicx]

Deploy-suite run 28478866791 / job 84413308650 triage: this PR appears to cover these test/e2e/prerender.test.ts residual Pages prerender failures:

• Prerender > should use correct caching headers for a revalidate page
• Prerender > should use correct caching headers for a fallback-true page (prerendered)
• Prerender > should use correct caching headers for a fallback-true page (lazy)
• Prerender > should not error when rewriting to fallback dynamic SSG page
• Prerender > should use correct caching headers for a no-revalidate page
• Prerender > should not show error for invalid JSON returned from getStaticProps on SSR
• Prerender > should revalidate on-demand revalidate with preview cookie
• Prerender > should not fetch prerender data on mount

[github-actions]

Performance benchmarks

Compared 233fb2c against base 239595e using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	125.6 KB	125.6 KB	⚫ -0.0%
Client entry size (gzip)	vinext	119.8 KB	119.8 KB	⚫ 0.0%
Dev server cold start	vinext	2.38 s	2.39 s	⚫ +0.6%
Production build time	vinext	2.73 s	2.72 s	⚫ -0.4%
RSC entry closure size (gzip)	vinext	96.8 KB	96.9 KB	⚫ +0.1%
Server bundle size (gzip)	vinext	163.2 KB	163.2 KB	⚫ +0.1%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}