refactor: extract pages page response helpers

[southpolesteve]

Summary

This PR starts the Pages Router side of the generated-entry refactor by extracting the page HTML shell / stream / response construction out of pages-server-entry.ts and into a typed runtime helper:

• adds packages/vinext/src/server/pages-page-response.ts
• moves document shell construction, font/style injection, __NEXT_DATA__ serialization, streamed HTML response shaping, and ISR cache fill HTML assembly into that helper
• keeps packages/vinext/src/entries/pages-server-entry.ts as thinner route/data wiring
• adds direct unit coverage in tests/pages-page-response.test.ts
• updates the Pages entry generator assertions and snapshot to check delegation instead of the old inline implementation details

This PR was written by Codex.

Why

After the App Router extraction series landed, pages-server-entry.ts is the next major generated module that still owns too much real runtime behavior. This is the first Pages Router slice: pull the HTML render/response lifecycle into normal typed code, then follow with the data/ISR and API/runtime seams on top of that.

Testing

• vp check packages/vinext/src/server/pages-page-response.ts packages/vinext/src/entries/pages-server-entry.ts tests/pages-page-response.test.ts tests/entry-templates.test.ts
• vp test run tests/pages-page-response.test.ts tests/entry-templates.test.ts -t 'pages page response|server entry delegates Pages HTML stream/response shaping to a typed helper'
• vp test run tests/pages-router.test.ts -t 'getServerSideProps headers and status are applied to the response|getServerSideProps calling res.end\(\) short-circuits the response|renders Suspense \+ React.lazy content via streaming SSR|Production Pages Router SSR streaming'
• vp test run tests/entry-templates.test.ts -u
• vp run vinext#build

[southpolesteve]

/bigbonk

[pkg-pr-new]

Open in StackBlitz

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

commit: e539f08

[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

[ask-bonk]

Good extraction — the generated entry is noticeably thinner and the new helper is well-typed. A few things to address before merging:

Behavioral change in applyGsspHeaders — non-primitive header values silently dropped

pages-page-response.ts:200-202

The old inline code accepted any non-null header value via String(hv):

} else if (hv != null) {
  responseHeaders.set(hk, String(hv));
}

The new code only accepts string | number | boolean:

if (typeof value === "string" || typeof value === "number" || typeof value === "boolean") {
  headers.set(key, String(value));
}

This silently drops non-set-cookie array values. Node.js res.setHeader() accepts readonly string[] for multi-value headers — e.g., res.setHeader("Vary", ["Accept", "Accept-Encoding"]) is idiomatic Node. The res facade's getHeaders() returns these as-is (raw arrays), so the old code would have set "Accept,Accept-Encoding" (via String()), while the new code drops it entirely.

I agree the old behavior was sloppy (especially the "[object Object]" case for plain objects), but the string[] case is legitimate Node.js API usage. Consider adding array handling:

if (Array.isArray(value)) {
  headers.set(key, value.map(String).join(", "));
} else if (typeof value === "string" || typeof value === "number" || typeof value === "boolean") {
  headers.set(key, String(value));
}

Or at minimum, match the old String() coercion behavior (which handles arrays via Array.prototype.toString() → join(",")) to avoid a silent parity break.

Test coverage gap: no test for array header values from gSSP

The new test at tests/pages-page-response.test.ts:81 only covers scalar gSSP header values ("content-type": "application/json", "x-test": "1"). Add a test case with an array-valued non-set-cookie header to cover the gap mentioned above and lock in whatever behavior you choose:

gsspRes: {
  statusCode: 200,
  getHeaders() {
    return {
      "vary": ["Accept", "Accept-Encoding"],
      "set-cookie": ["a=1", "b=2"],
      "x-custom": 42,
    };
  },
},

Nit: PagesGsspResponse.getHeaders return type is too loose

pages-page-response.ts:17-18

export interface PagesGsspResponse {
  statusCode: number;
  getHeaders(): Record<string, unknown>;
}

The unknown value type means applyGsspHeaders can't safely iterate values without narrowing. Consider Record<string, string | number | boolean | string[]> to match the actual possible values from the res facade, which would also make the type guard in applyGsspHeaders feel less arbitrary.

Overall

The extraction itself is clean and well-structured. The helper boundary (options bag with callbacks for rendering, ISR, fonts) is a good seam. The test file covers the key paths (document shell, ISR cache fill, gSSP header merge, streamed response marking). Just need to fix the header coercion behavioral change and add the corresponding test case.

[ask-bonk]

Review posted on PR #657. Here's a summary of the findings:

One behavioral change that needs fixing:

• applyGsspHeaders in the new helper silently drops non-primitive header values (arrays, objects) that the old inline code would have coerced via String(). This matters for legitimate Node.js res.setHeader("Vary", ["Accept", "Accept-Encoding"]) usage, where the res facade stores raw arrays and getHeaders() returns them as-is. The old code would set "Accept,Accept-Encoding", the new code drops them entirely.

One test gap:

• No test for array-valued non-set-cookie headers from gSSP, which would have caught the above.

One typing nit:

• PagesGsspResponse.getHeaders() returns Record<string, unknown> — tightening to Record<string, string | number | boolean | string[]> would make the type guard less arbitrary.

Otherwise, the extraction is clean — the helper boundary is well-designed with an options bag of callbacks, the generated entry is substantially thinner, and the existing test coverage for document shell, ISR cache fill, and streamed response marking is solid.

github run

[southpolesteve]

Addressed the requested changes in e539f08.

What changed:

• preserved array-valued non-set-cookie gSSP headers in applyGsspHeaders() instead of silently dropping them
• tightened PagesGsspResponse.getHeaders() to Record<string, string | number | boolean | string[]>
• added a regression test covering vary: ["Accept", "Accept-Encoding"], numeric headers, and multiple set-cookie values

Local verification:

• vp check packages/vinext/src/server/pages-page-response.ts tests/pages-page-response.test.ts
• vp test run tests/pages-page-response.test.ts
• vp test run tests/pages-router.test.ts -t 'getServerSideProps headers and status are applied to the response|getServerSideProps calling res.end\(\) short-circuits the response|Production Pages Router SSR streaming'

/bigbonk

[ask-bonk]

All three items from the previous review are addressed:

1. Array header coercion fixed — applyGsspHeaders now handles Array.isArray(value) with value.join(", ") before the primitive check, matching Node.js res.setHeader("Vary", ["Accept", "Accept-Encoding"]) semantics correctly.

2. Test added — "preserves array-valued non-set-cookie headers from gSSP responses" covers the array-valued Vary, numeric x-custom, and array set-cookie cases.

3. Type tightened — PagesGsspResponse.getHeaders() returns Record<string, string | number | boolean | string[]> instead of Record<string, unknown>.

The extraction itself is clean. The helper boundary (options bag with callbacks for rendering, ISR, fonts) is a good seam that makes the generated entry substantially thinner while keeping all behavior testable in isolation. The three test cases cover the key paths: document shell with gSSP headers, array header coercion, and ISR cache fill with query-string stripping.

One minor observation (not blocking): applyGsspHeaders uses key.toLowerCase() only for the set-cookie check but passes the original key to headers.set() for array values. This is fine because Headers.set() is case-insensitive per the Fetch spec, but it's a slight asymmetry with the set-cookie branch. Not worth changing.

LGTM.

[ask-bonk]

Review posted. All three items from the prior review (array header coercion, test coverage for array headers, tighter getHeaders return type) have been addressed. The extraction is clean and the generated entry is substantially thinner. Approved.

github run