fix(app-router): respect reactMaxHeadersLength for preload Link header (#1552)

[james-elicx]

Problem

reactMaxHeadersLength (the cap on the total length of the preload Link header React inlines during App Router SSR) was not respected — vinext never forwarded the option into the React renderer. Fixes #1552.

Fix

Thread reactMaxHeadersLength from next.config through the App Router render pipeline and into React's renderToReadableStream:

• Config (config/next-config.ts): accept reactMaxHeadersLength, default to 6000 (matching Next.js's defaultConfig).
• Codegen (entries/app-rsc-entry.ts): embed the resolved value and pass it through the page dispatch.
• Render pipeline (app-page-dispatch.ts → app-page-render.ts → app-page-stream.ts → app-ssr-entry.ts): forward the value, wire React's onHeaders + maxHeadersLength so React caps and emits its preload Link header. 0 disables emission entirely (no onHeaders wired, no Link header set).
• Response (buildAppPageLinkHeader): combine vinext's font preload Link header with React's emitted Link header and cap the combined value to reactMaxHeadersLength, dropping whole entries once the limit is exceeded (mirrors React's behavior, where every preload flows through a single capped onHeaders).

Test

tests/nextjs-compat/react-max-headers-length.test.ts (ported from test/e2e/app-dir/react-max-headers-length) verifies the cap contract against the emitted Link header: 0 emits no header, small caps truncate to whole entries, larger caps emit more entries, and the default stays within 6000. Backed by a minimal route fixture under app-basic whose reactMaxHeadersLength is env-driven (matching the upstream TEST_REACT_MAX_HEADERS_LENGTH switch).

Caveat

vinext's App Router SSR currently surfaces its preload Link header from the font pipeline. RSC-originated ReactDOM.preload() hints from Server Components flow through the embedded Flight stream and are applied client-side rather than captured by the SSR Fizz onHeaders pass (the app tree renders behind the Flight-stream Suspense, so user preloads resolve after the shell completes and React's onHeaders has already fired). Making those preloads reach the SSR header is a separate, larger change; this PR forwards the config option into the renderer and makes the emitted Link header respect reactMaxHeadersLength, which is the behavior the issue calls for.

Closes #1552

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

commit: d6a7bc2

[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

[ask-bonk]

Review: reactMaxHeadersLength support (#1552)

Solid, well-scoped change. The threading from next.config → codegen → dispatch → render → stream → SSR entry is consistent, both handleSsr paths (main render at app-page-render.ts:828 and the revalidated path at app-page-dispatch.ts:618) forward the value, and both buildAppPageHtmlResponse call sites (app-page-render.ts:964, :1029) use the combined header. I verified the new test passes and confirmed the default-wiring behavior matches Next.js, which always wires onHeaders + maxHeadersLength for App Router SSR (app-render.tsx:4323/4462). The 0-disables-everything semantics (including the font header, since the response layer guards on a truthy value) is also correct parity. Type/lint/format all clean.

A few non-blocking observations:

1. Missing unit test for resolveNextConfig parity (test gap)

tests/next-config.test.ts has an established describe("resolveNextConfig expireTime", …) block (line 1415) that tests default + configured + fallback for the sibling expireTime option, but there's no equivalent for reactMaxHeadersLength. The new behavior in next-config.ts:1480 (default 6000, numeric passthrough, non-number fallback) is only covered transitively by the integration test. A small unit block mirroring the expireTime one would lock the resolution contract directly and is the cheapest place to catch a regression:

describe("resolveNextConfig reactMaxHeadersLength", () => {
  it("defaults to 6000", async () => {
    expect((await resolveNextConfig(null)).reactMaxHeadersLength).toBe(6000);
  });
  it("uses the configured value (including 0)", async () => {
    expect((await resolveNextConfig({ reactMaxHeadersLength: 0 })).reactMaxHeadersLength).toBe(0);
    expect((await resolveNextConfig({ reactMaxHeadersLength: 200 })).reactMaxHeadersLength).toBe(200);
  });
});

2. DEFAULT_REACT_MAX_HEADERS_LENGTH = 6000 is duplicated in 4 files

It's defined independently in next-config.ts:494, app-rsc-entry.ts:26, app-page-stream.ts:74, and app-ssr-entry.ts:67. This is parity-critical (the comments explicitly note "both caps must agree"), so four independent literals is a drift risk — if Next.js ever changes the default, three of these silently disagree. I acknowledge DEFAULT_EXPIRE_TIME is already duplicated this way in the repo, so this matches existing convention and isn't a new anti-pattern. Worth considering a single exported constant (e.g. from next-config.ts), but not a blocker.

3. Combine ordering: fonts win the cap budget over React's preloads

buildAppPageLinkHeader (app-page-stream.ts:81-99) concatenates fontLinkHeader entries first, then reactLinkHeader entries, then drops whole entries from the tail once the cap is hit. Under a tight cap, this means React's module/script preloads (the entries that actually drive the initial paint) are the first to be dropped, while font preloads always survive. Next.js routes every preload through a single onHeaders pass in render order, so this ordering is a deliberate vinext divergence — which the PR description already documents as a known limitation. Flagging only so it's a conscious choice: if anything, React's render-critical preloads arguably deserve priority over fonts. Not a correctness bug given the documented caveat.

4. Minor: parameter naming

The combined value is computed as linkHeader (app-page-render.ts:859) but passed via the fontLinkHeader property of buildAppPageHtmlResponse. The property now carries more than fonts; a follow-up rename (e.g. linkHeader) on AppPageHtmlResponseOptions would reduce confusion. Cosmetic.

Overall this looks correct and matches the behavior the issue calls for. The caveat about RSC-originated ReactDOM.preload() hints not reaching the SSR header is honest and appropriately deferred. None of the above blocks merge — item 1 (the unit test) is the one I'd most encourage adding.

[ask-bonk]

Review posted to PR #1841. Summary of findings:

Review summary

The PR is correct and well-scoped. I verified:

• The config → codegen → dispatch → render → stream → SSR threading is complete; both handleSsr paths and both response-build sites use the combined header.
• New default wiring (onHeaders + maxHeadersLength always on) matches Next.js's App Router SSR behavior.
• 0-disables-everything semantics (including font header) is correct parity.
• The new test passes; type/lint/format are clean; existing next-config and app-router config tests still pass.

Non-blocking observations posted:

1. Test gap — No unit test in tests/next-config.test.ts for resolveNextConfig resolution of reactMaxHeadersLength, despite an established expireTime precedent. Suggested a small block. (The one I'd most encourage adding.)
2. 4× duplicated 6000 default across next-config.ts, app-rsc-entry.ts, app-page-stream.ts, app-ssr-entry.ts — parity-critical magic number, drift risk. Matches existing DEFAULT_EXPIRE_TIME convention, so not a new anti-pattern.
3. Combine ordering — fonts get cap-budget priority over React's render-critical preloads. A documented divergence from Next.js's single-pass ordering; flagged as a conscious choice.
4. Minor naming — combined value passed via fontLinkHeader property; cosmetic follow-up.

No correctness bugs or regressions found. The documented RSC-preload caveat is honest and appropriately deferred.

github run

[james-elicx]

Thanks for the review. Addressed the actionable items:

1. Test gap — added a resolveNextConfig reactMaxHeadersLength unit block in tests/next-config.test.ts (default 6000, configured value, and 0 passthrough), plus direct unit coverage for buildAppPageLinkHeader in tests/app-page-stream.test.ts.
2. Duplicated 6000 — kept four local constants, matching the existing DEFAULT_EXPIRE_TIME convention to avoid pulling config-loading code into the runtime/codegen bundles (the SSR-entry, codegen, and pure-util sites must not import from next-config.ts). Each is comment-anchored as parity-critical.
3. Combine ordering — flipped buildAppPageLinkHeader to emit React's render-critical preloads first, then font preloads, so trailing fonts are the first dropped under a tight cap.
4. Naming — renamed the buildAppPageHtmlResponse option from fontLinkHeader to linkHeader (it now carries the combined header).

/bigbonk review for issues

[ask-bonk]

Re-review: addressed items verified

Re-reviewed after the three follow-up commits (92c082e, d6a7bc2). All four items from the prior review are resolved and the change is clean. vp check passes (format + lint + type) on every touched file, and tests/app-page-stream.test.ts (22) + tests/next-config.test.ts (173) are green.

1. Test gap — resolved

resolveNextConfig reactMaxHeadersLength block (tests/next-config.test.ts:1427) covers default 6000, a configured value, and the 0 passthrough — mirroring the sibling expireTime block exactly. Direct buildAppPageLinkHeader unit coverage (tests/app-page-stream.test.ts:426) covers combine ordering, 0 → empty, default cap, whole-entry truncation, fonts-dropped-first, and empty sources. This is the coverage I most wanted; good.

2. Duplicated 6000 — acknowledged, accepted

Kept as four comment-anchored local constants (next-config.ts:494, app-rsc-entry.ts, app-page-stream.ts:78, app-ssr-entry.ts). The rationale (SSR-entry/codegen/pure-util sites must not import config-loading code into their bundles) is sound and matches the existing DEFAULT_EXPIRE_TIME convention. Each is parity-anchored with a comment. Fine to leave as-is.

3. Combine ordering — resolved correctly

buildAppPageLinkHeader (app-page-stream.ts:86) now iterates [reactLinkHeader, fontLinkHeader], so render-critical React preloads win the cap budget and trailing fonts drop first. The new test at app-page-stream.test.ts:454 locks this behavior. This is the better default and what I'd hoped for.

4. Naming — resolved

BuildAppPageHtmlResponseOptions.linkHeader (app-page-response.ts:74) now reflects that it carries the combined header; both call sites (app-page-render.ts:964, :1029) updated, and tests/app-page-response.test.ts follows. Note the separate renderAppPageHtmlResponse in app-page-stream.ts:173 still uses fontLinkHeader — correctly, since the error-boundary path (app-page-boundary-render.ts:271) only ever feeds font preloads there and does not go through buildAppPageLinkHeader. The naming split is accurate, not an oversight.

Verification

• 0-disable path traced end-to-end: app-ssr-entry.ts:457 skips onHeaders/maxHeadersLength (avoids React's non-positive warning) → buildAppPageLinkHeader returns "" → truthy guard in buildAppPageHtmlResponse omits the Link header. Integration test react-max-headers-length.test.ts:59 confirms no header at 0.
• Both handleSsr paths forward reactMaxHeadersLength; both response-build sites consume the combined header.

No correctness issues. LGTM.

[ask-bonk]

Posted the re-review to PR #1841. All four items from the prior review are resolved and verified:

1. Test gap — resolveNextConfig reactMaxHeadersLength block (default/configured/0) plus direct buildAppPageLinkHeader unit coverage are present and pass.
2. Duplicated 6000 — kept as four comment-anchored local constants; rationale (no config-loading imports in SSR/codegen/pure-util bundles) is sound and matches the existing DEFAULT_EXPIRE_TIME convention.
3. Combine ordering — buildAppPageLinkHeader now emits React preloads first, fonts drop first under a tight cap, locked by a new test.
4. Naming — BuildAppPageHtmlResponseOptions.linkHeader renamed; both call sites and the response test updated. I confirmed the separate renderAppPageHtmlResponse keeping fontLinkHeader is correct (error-boundary path only carries fonts).

Additional verification:

• vp check clean on all touched files; app-page-stream.test.ts (22) + next-config.test.ts (173) green.
• Traced the 0-disable path end-to-end (skip onHeaders → empty combined header → omitted Link header), matching the integration test.

No correctness issues found — LGTM.

github run