fix(pages): pass req res to document initial props

[james-elicx]

Summary

• thread the shared Pages req/res into _document.getInitialProps during production HTML renders
• create a document req/res for plain Pages routes that do not already run gSSP, _app.getInitialProps, or page getInitialProps
• mirror the normal dev render path and preserve status/headers set through _document

Validation

• vp test run tests/pages-page-response.test.ts -t "passes req/res into _document"
• vp test run tests/pages-router.test.ts -t "passes req/res into _document"
• vp test run tests/pages-router.test.ts -t "Pages _document renderPage enhancers"
• vp test run tests/pages-page-response.test.ts
• vp check packages/vinext/src/server/dev-server.ts packages/vinext/src/server/pages-document-initial-props.ts packages/vinext/src/server/pages-page-data.ts packages/vinext/src/server/pages-page-handler.ts packages/vinext/src/server/pages-page-response.ts tests/pages-page-response.test.ts tests/pages-router.test.ts

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

commit: 8e637ad

[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]

Review: fix(pages): pass req res to document initial props

Reviewed the changes across dev-server.ts, pages-document-initial-props.ts, pages-page-data.ts, pages-page-handler.ts, pages-page-response.ts, and the two test files. The core change is correct and well-scoped: threading the shared Pages req/res into _document.getInitialProps, falling back to a freshly-created req/res for plain routes, and folding the document res into header/status application.

I verified:

• gsspRes and documentReqRes.res are the same object on gSSP/gIP routes (gsspRes = res at pages-page-data.ts:815, both sourced from getSharedReqRes()), so gsspRes ?? documentReqRes?.res in applyGsspHeaders never drops headers set by _document on those routes.
• The statusCode = 200 -> statusCode ?? res.statusCode change in dev-server.ts is sound: Node's ServerResponse.statusCode defaults to 200, so no regression, and it now lets _document.getInitialProps influence status in dev to match prod.
• vp check is clean and the cited tests pass in both dev and prod.

A few issues worth addressing:

1. Dev/prod parity gap: dev does not honor a response sent from _document.getInitialProps

In prod, pages-page-response.ts:531-533 returns early if _document.getInitialProps sent a response via res:

if (options.documentReqRes?.res.headersSent && options.documentReqRes.responsePromise) {
  return options.documentReqRes.responsePromise;
}

The dev path (dev-server.ts streamPageToResponse) has no equivalent guard. In dev the res passed in documentContext (dev-server.ts:1734-1735) is the real Node ServerResponse. If a user's _document.getInitialProps calls res.end()/res.json()/res.writeHead(), dev will still fall through to res.writeHead(statusCode ?? res.statusCode, headers) at dev-server.ts:359 and throw ERR_HTTP_HEADERS_SENT, while prod returns the already-sent response.

This is an edge case (sending a response from _document.getInitialProps is unusual, and the new tests only exercise setHeader/statusCode, not res.end()), but per the repo's dev/prod parity rule it should either be handled in dev too or explicitly documented as a known divergence. Worth confirming Next.js behavior here first.

2. Missing integration coverage for the headline "plain Pages route" path

The PR summary's main new behavior is "create a document req/res for plain Pages routes that do not already run gSSP, _app.getInitialProps, or page getInitialProps" — i.e. the pageDataResult.documentReqRes ?? createPageReqRes() fallback at pages-page-handler.ts:647.

Neither new test exercises that fallback:

• tests/pages-router.test.ts uses an index.tsx fixture that defines getServerSideProps (tests/pages-router.test.ts:5715), so documentReqRes is always the shared gSSP req/res — createPageReqRes() is never hit.
• tests/pages-page-response.test.ts passes documentReqRes directly, bypassing the handler entirely.

Consider adding a plain static page (no gSSP/gIP/_app.getInitialProps) whose _document.getInitialProps reads ctx.req.cookies and sets ctx.res.setHeader/statusCode, to lock in the new fallback path end-to-end.

3. Stale comment wording

pages-document-initial-props.ts:202-203 now says req/res are provided "in SSR paths". With this PR, req/res are passed on all Pages render paths (including plain static pages via the createPageReqRes() fallback), not just SSR. Suggest tweaking the comment to avoid implying the fallback path is excluded.

Minor

• The responsePromise field on PagesDocumentReqRes is optional, so the guard at pages-page-response.ts:531 is headersSent && responsePromise. A headersSent-but-no-responsePromise res silently re-renders. This is only reachable via a test mock today (real producers always set responsePromise), so it's not a bug — just noting the guard's behavior.

Overall the implementation logic is correct; the main actionable items are the dev-path parity check (#1) and integration coverage for the plain-route fallback (#2).

[ask-bonk]

Review posted to PR #2402. Summary of what I found:

Review summary

The core change is correct and well-scoped. I verified the key invariant that makes it safe: on gSSP/gIP routes, gsspRes and documentReqRes.res are the same object (pages-page-data.ts:815), so the gsspRes ?? documentReqRes?.res fallback in applyGsspHeaders never drops document-set headers. The statusCode = 200 → statusCode ?? res.statusCode change is sound (Node defaults res.statusCode to 200). vp check is clean and the cited tests pass in dev and prod.

Issues flagged:

1. Dev/prod parity gap (most important) — Prod returns early when _document.getInitialProps sends a response (pages-page-response.ts:531-533), but dev has no equivalent guard. In dev the res is the real Node ServerResponse, so a _document that calls res.end() would crash dev with ERR_HTTP_HEADERS_SENT while prod succeeds. Edge case, but violates the repo's dev/prod parity rule.

2. Missing integration coverage for the headline feature — The "plain Pages route" fallback (createPageReqRes() at pages-page-handler.ts:647) isn't actually exercised: the integration fixture uses getServerSideProps, and the unit test passes documentReqRes directly.

3. Stale comment — pages-document-initial-props.ts:202 says req/res are provided "in SSR paths", but they're now passed on all render paths.

Plus a minor note about the optional responsePromise guard behavior.

No blocking correctness bugs in the logic itself — the actionable items are the dev-path parity check and the integration test gap.

github run

[github-actions]

Performance benchmarks

Compared 8e637ad against base 69e0051 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	124.0 KB	124.0 KB	⚫ +0.0%
Client entry size (gzip)	vinext	118.8 KB	118.8 KB	⚫ -0.0%
Dev server cold start	vinext	2.58 s	2.59 s	⚫ +0.4%
Production build time	vinext	3.17 s	3.13 s	⚫ -1.1%
RSC entry closure size (gzip)	vinext	95.2 KB	95.2 KB	⚫ -0.0%
Server bundle size (gzip)	vinext	161.5 KB	161.4 KB	⚫ -0.0%

View detailed results and traces

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

[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]

Review: fix(pages): pass req res to document initial props

Re-reviewed at HEAD 8e637ad. This commit is more complete than the version the earlier bot review saw — it already resolves all three substantive items that prior review flagged. Verified by reading every changed file, tracing the data flow, and running the cited tests + vp check.

Verified correct

• Dev/prod parity for early responses (prior issue #1 — now fixed). dev-server.ts:251 adds if (res.headersSent || res.writableEnded) return; right after runDocumentRenderPage, mirroring the prod early-return at pages-page-response.ts:531-533. A _document.getInitialProps that calls res.end() now leaves the already-sent response intact in both dev and prod instead of throwing ERR_HTTP_HEADERS_SENT in dev. The new test "honors _document.getInitialProps responses that end early in %s" exercises res.end("DOCUMENT ENDED") + statusCode = 203 in both modes and passes.

• Integration coverage for the new paths (prior issue #2 — now fixed). Added fixtures static.tsx (auto-export) and static-gsp.tsx (getStaticProps), plus dev+prod tests:

  • "passes req/res into _document.getInitialProps in %s" (gSSP route)
  • "passes req/res into _document.getInitialProps for getStaticProps pages in %s" — this is the path that actually hits createPageReqRes() (pages-page-handler.ts:650), since a gSP page served from a generatedFromDataRequest cache HIT never calls getSharedReqRes(), so pageDataResult.documentReqRes is null.
  • "omits req/res from _document.getInitialProps for auto-export pages in %s" — locks in the autoExport === true → null gate (has-res vs missing-res assertion).

• Stale comment (prior issue #3 — now fixed). pages-document-initial-props.ts comment rewritten to say renderers pass "the request-scoped context they can provide (pathname/query/asPath plus req/res when available)".

• Header/status fallback is safe. On gSSP/gIP routes gsspRes and documentReqRes.res are the same object (gsspRes = res at pages-page-data.ts:815, both from getSharedReqRes()), so options.gsspRes ?? options.documentReqRes?.res in applyGsspHeaders (pages-page-response.ts:604-608) never drops document-set headers. The finalStatus correctly feeds the ISR cache write.

• statusCode ?? res.statusCode is sound. Node's ServerResponse.statusCode defaults to 200, so dropping the statusCode = 200 default preserves behavior while letting _document influence status in dev (matching prod).

• autoExport gating is consistent across dev (dev-server.ts:1735) and prod (pages-page-handler.ts:648). Both pass req/res only when the page has a data method (!autoExport), matching Next.js, which renders auto-export documents without request-scoped req/res.

• vp check clean on all five source files; all 22 _document tests in tests/pages-router.test.ts and the targeted pages-page-response.ts test pass.

Non-blocking notes

• PR summary wording is slightly inaccurate. It says the fallback covers "plain Pages routes that do not already run gSSP, _app.getInitialProps, or page getInitialProps". But such pure-static pages are autoExport === true, which short-circuits documentReqRes to null before the ?? createPageReqRes() fallback. The fallback is actually reached for gSP pages served from a data-request-generated cache HIT (where getSharedReqRes() was never invoked). Behavior is correct; only the description is misleading. Worth tightening for the changelog.

• Minor (carried from prior review, still accurate): the prod guard at pages-page-response.ts:531 is headersSent && responsePromise. A headersSent-but-no-responsePromise res would silently re-render, but createPagesReqRes() always sets responsePromise (pages-node-compat.ts:455), so this is only reachable via test mocks — not a real-world bug.

Verdict

No blocking issues. Logic is correct, dev/prod parity holds, and coverage now exercises gSSP, gSP, auto-export, and early-res.end() paths in both runtimes. Recommend tightening the PR/commit description's "plain Pages route" wording before merge.

[ask-bonk]

Review posted to PR #2402. Summary of my findings:

Review summary

I re-reviewed at HEAD 8e637ad, which is more complete than the commit the earlier bot review evaluated. The current code already resolves all three substantive issues that the prior ask-bonk review flagged:

1. Dev/prod parity gap (prior Virtual module imports break esbuild dependency optimization when vinext is installed from npm #1) — fixed. dev-server.ts:251 now guards if (res.headersSent || res.writableEnded) return;, mirroring the prod early-return. A new dev+prod test exercises res.end() from _document.getInitialProps.

2. Integration coverage (prior Published npm package ships sourcemaps pointing to missing source files #2) — fixed. Added static.tsx and static-gsp.tsx fixtures with dev+prod tests covering gSSP, getStaticProps (the path that actually hits createPageReqRes()), and auto-export omission.

3. Stale comment (prior fix: exclude vinext from optimizeDeps to prevent virtual module resolution errors #3) — fixed. The pages-document-initial-props.ts comment was rewritten.

I verified the header/status fallback is safe (gsspRes and documentReqRes.res are the same object on gSSP/gIP routes), the statusCode ?? res.statusCode change is sound, and the autoExport gate is consistent across dev and prod. vp check is clean and all 22 _document tests pass.

Non-blocking note: The PR summary's "plain Pages routes" description is slightly inaccurate — pure-static pages are autoExport === true and short-circuit to null before the fallback. The createPageReqRes() fallback is actually reached for gSP pages served from a data-request-generated cache HIT. Behavior is correct; only the wording is misleading.

Verdict: No blocking issues.

github run