fix(pages): preserve styled-jsx CSS ordering in SSR

[james-elicx]

Summary

• buffer Pages Router HTML through React allReady in dev and production, matching Next.js and preventing later global CSS resources from overriding styled-jsx rules
• collect and flush each request-local styled-jsx registry once after Suspense settles, then emit its deduplicated rules after React body stylesheet resources
• keep ISR regeneration ordering consistent with fresh renders while preserving style-looking literals in every HTML raw-text container
• remove stale getServerSideProps Content-Length values before Worker response merging
• retain Pages data exports through the styled-jsx transform and provide the portable ESM styled-jsx runtime needed by generated and relocated static-export entries

Pages Router behavior change

Pages Router Suspense HTML is intentionally no longer exposed progressively: body bytes wait for renderStream.allReady. This trades Pages Router TTFB and progressive reveal for deterministic CSS ordering and Next.js parity. App Router streaming is unchanged, and Pages HEAD requests retain their headers-only fast path.

Validation

• vp check on all changed source and test files
• focused Pages response and data tests: 139/139 passed
• production Pages response integration block: 10/10 passed
• focused header merge and response-close tests: 11/11 passed
• targeted Next.js test/e2e/streaming-ssr/index.test.ts: 5/5 passed
• targeted Next.js test/e2e/app-dir/scss/with-styled-jsx/with-styled-jsx.test.ts: 2/2 passed across both Sass variants
• targeted Next.js scripts rebuilt all vinext packages successfully before the upstream tests

Review

Two independent cumulative reviews returned no findings after the raw-text, Worker header, duplicate styled-jsx registry, behavior-contract, and dead HTML streaming-marker findings were addressed. The PR description was also replaced and verified from a literal body file so the documented commands and behavior remain intact.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

npm i https://pkg.pr.new/create-vinext-app@2894

npm i https://pkg.pr.new/@vinext/types@2894

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

commit: a76fb59

[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

[github-actions]

Performance benchmarks

Compared a76fb59 against base a5f0bb4 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	136.2 KB	136.2 KB	⚫ -0.0%
Client entry size (gzip)	vinext	123.6 KB	123.6 KB	⚫ +0.0%
Dev server cold start	vinext	2.97 s	2.93 s	⚫ -1.4%
Production build time	vinext	3.13 s	3.11 s	⚫ -0.9%
RSC entry closure size (gzip)	vinext	114.9 KB	114.9 KB	⚫ -0.0%
Server bundle size (gzip)	vinext	195.3 KB	195.3 KB	⚫ +0.0%

View detailed results and traces

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

[ask-bonk]

Review: fix(pages): preserve styled-jsx during streaming SSR

Reviewed the full diff plus dev/prod parity across the four request-handling paths. This is a solid, well-tested change. Verified locally: styled-jsx, pages-page-response, pages-page-data tests pass (13 + 137), the two new pages-router streaming/ISR tests pass, and vp check is clean on all six changed source files. No blocking issues found. A few notes below.

Verified correct

• ESM conversion regexes match the real runtime. Ran convertStyledJsxRuntimeToEsm's three replacements against styled-jsx@5.1.6/dist/index/index.js — all three match, and React__default["default"] still works after swapping var React = require(...) for import * as React. The fail-loud throw when any replacement is a no-op is a reasonable guard against a future runtime layout change.
• getRuntimeDistPath() path is correct. require.resolve("styled-jsx") resolves to .../styled-jsx/index.js, so dirname(...) + dist/index/index.js lands on the real file. Coupling to the internal dist layout is acceptable given the wrapper-shape guard.
• Double-collect + flush is not duplicative. Initial collectPagesStyleRegistryHtml reads shell styles and flushes; the bodyAllReady continuation collects only post-flush (Suspense) styles. No double-emit.
• disableNextSsg: true is legitimate (next/dist/build/swc/options.d.ts:27) and the new test asserts getStaticProps/getServerSideProps survive without __N_SSG/__N_SSP. Good catch — this was silently stripping Pages data exports.
• ISR tee ordering holds. bodyStream.allReady is captured before tee(); both branches are drained (writePagesIsrCache reads the cache branch, composite stream reads the response branch), so finalShellSuffix resolves and late styles land in the cached HTML.
• renderErrorPage refactor is behavior-preserving. The removed non-Document branch is subsumed by streamPageToResponse's default-shell path (dev-server.ts:513-529), and preserveExistingContentType: statusCode === 404 mirrors the old conditional Content-Type. Bonus: error pages now get styled-jsx too.

Non-blocking observations

1. Dev/prod parity in how styled-jsx is loaded is structurally different. The prod entry (pages-server-entry.ts:212) statically imports styled-jsx and will hard-fail if it can't resolve, while dev (dev-server.ts:365) wraps the import in try/catch and treats it as optional. In practice this is fine because styled-jsx@5.1.6 is a hard dependency of next and vinext always requires next, so the module is always resolvable. But the two paths encode different assumptions about optionality. Worth a one-line comment on the prod static import noting why it's safe to import unconditionally (i.e., "always present via next's deps"), so a future reader doesn't "fix" the asymmetry by making prod also optional (which would silently drop styles).

2. ISR regeneration relocates styles from before-</body> to <head>. On the initial streaming write, late/Suspense styles are injected before </body>; on regeneration, STYLED_JSX_STYLE_PATTERN strips all __jsx- styles (head and body) and refreshCachedHeadTags re-inserts the freshly collected combined run into <head> (regen uses the non-streaming renderToStringAsync, so there's no late split). Both placements are valid and the custom-document ISR test passes, so this is correct — just noting the placement is intentionally not identical between first render and regen.

3. STYLED_JSX_STYLE_PATTERN lookahead assumes id appears before any >. <style\b(?=[^>]*\bid=["']__jsx-) relies on styled-jsx emitting id as the first attribute, which mapRulesToStyle does. Fine for the pinned runtime; just flagging it's coupled to attribute ordering.

Nothing here blocks merge. Items 1-2 are optional clarifying comments for future maintainers.

[ask-bonk]

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

Review Summary

The PR is well-implemented and thoroughly tested. No blocking issues found. I verified locally that all relevant tests pass (13 styled-jsx, 137 pages-response/data, 5 pages-router streaming/ISR) and vp check is clean on all changed source files.

Verified correct

• The convertStyledJsxRuntimeToEsm regexes match the real styled-jsx@5.1.6 runtime; the fail-loud guard against a future layout change is sound.
• getRuntimeDistPath() correctly resolves .../styled-jsx/dist/index/index.js.
• The double-collect + flush pattern (shell styles then post-flush Suspense styles) does not duplicate output.
• disableNextSsg: true is a legitimate SWC option that fixes silent stripping of getStaticProps/getServerSideProps — a real bug the PR correctly addresses.
• The ISR tee() ordering holds (allReady captured before tee; both branches drained).
• The renderErrorPage refactor is behavior-preserving and additionally brings styled-jsx to error pages.

Non-blocking observations (optional clarifying comments)

1. Structural dev/prod parity difference: prod statically imports styled-jsx (hard-fail), dev treats it as optional (try/catch). Safe in practice since styled-jsx is always a next dependency, but worth a comment explaining why the prod static import is safe.
2. ISR regen relocates styles from before-</body> (initial streaming write) to <head> (regeneration). Both valid; noting it's intentional.
3. STYLED_JSX_STYLE_PATTERN lookahead assumes id is the first attribute — true for the pinned runtime.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(pages): preserve styled-jsx during streaming SSR

Reviewed the full diff, dev/prod parity across all four request-handling paths, and verified the styled-jsx runtime shape against the pinned styled-jsx@5.1.6. Ran the relevant suites locally — all pass (14 styled-jsx, 137 pages-data/response, 5 pages-router streaming/ISR/static-export) — and vp check is clean on all six changed source files. No blocking issues found.

Verified correct

• CodeQL alert on pages-page-data.ts is a false positive. The flagged freshStyledJsxHTML/styledJsxHTML strings originate from renderToStringAsync(registry.styles()) — styled-jsx's style() renders developer-authored CSS through React's dangerouslySetInnerHTML. No request-derived/user data flows into the style content, and the injection targets (shellSuffix, cached <head>) are trusted document templates, not the untrusted style payload. This matches Next.js's own styled-jsx emission exactly. Safe to dismiss.
• disableNextSsg: true fixes a real bug. Without it, SWC's default client-page tree-shaker replaces getStaticProps/getServerSideProps with __N_SSG/__N_SSP and strips the functions from vinext's server module graph. The new styled-jsx-streaming.tsx fixture (with getServerSideProps) plus the data-export tests confirm the functions survive. The inline comment correctly explains why this SWC option belongs here.
• ESM runtime conversion matches the pinned runtime. Verified all three replacement regexes in convertStyledJsxRuntimeToEsm match styled-jsx@5.1.6/dist/index/index.js (client-only require, var React = require(...), and the exports.* tail). The fail-loud throw when any replacement is a no-op is a sound guard against a future layout drift.
• getRuntimeDistPath() resolution is correct. require.resolve("styled-jsx") → .../styled-jsx/index.js (which itself is module.exports = require('./dist/index')), so dirname(...) + dist/index/index.js lands on the real runtime.
• Fallback runtime is a coherent no-op. When the runtime can't be resolved, styled-jsx → STYLED_JSX_FALLBACK_RUNTIME (empty registry) and styled-jsx/style → null (default resolution). createStyleRegistry().styles() returns [], so the generated entry's static import never crashes and simply emits no styles. Consistent across both ids.
• Double-collect + flush is not duplicative. Eager collectPagesStyleRegistryHtml reads shell styles (available after renderToReadableStream resolves the shell synchronously) and flushes → head; the bodyAllReady-gated continuation collects only post-flush Suspense styles → before </body>.
• ISR tee ordering holds; no backpressure deadlock. bodyAllReady is captured before tee(). writePagesIsrCache drains its branch via readStreamAsText (driving the render → resolving allReady → resolving finalShellSuffix), and buildPagesCompositeStream independently drains the response branch and awaits the same finalShellSuffix. Both consumers drive rendering; nothing waits on an undrained branch.
• renderErrorPage refactor is behavior-preserving. The removed non-Document branch is fully subsumed by streamPageToResponse's default-shell path (dev-server.ts:513-529), and preserveExistingContentType: statusCode === 404 (dev-server.ts:558) reproduces the old conditional Content-Type. Error pages now additionally get styled-jsx. Prod pages-server-entry.ts isn't an error-page path, so no parity gap introduced.
• ISR style refresh is complete. removeStyledJsxStyles strips all __jsx- styles from both head and pre-</body> positions on every regeneration (including when the fresh render emits none), and re-inserts the freshly collected run into <head>. The isr-second-render-state.tsx dynamic z-index: ${timestamp} fixture proves styles actually refresh rather than going stale.

Non-blocking observations

1. Dev/prod load styled-jsx with different optionality assumptions. Prod (pages-server-entry.ts:212) statically imports styled-jsx (would hard-fail if unresolvable, though the plugin's fallback runtime prevents that), while dev (dev-server.ts:365) wraps the import in try/catch as optional. In practice both are safe because styled-jsx is a hard dependency of next and the plugin supplies a fallback. A one-line comment on the prod static import noting why it's safe to import unconditionally would stop a future reader from "fixing" the asymmetry by making prod optional (which would silently drop styles).

2. STYLED_JSX_STYLE_PATTERN lookahead assumes id is the first attribute. <style\b(?=[^>]*\bid=["']__jsx-) relies on styled-jsx emitting id before any >, which mapRulesToStyle does for the pinned runtime. Fine as-is; just coupled to attribute ordering alongside the convertStyledJsxRuntimeToEsm runtime-shape coupling (which at least fails loud).

3. ISR regen relocates late styles from before-</body> to <head>. First render splits shell styles (head) from late Suspense styles (before </body>); regeneration uses non-streaming renderToStringAsync and folds the combined run into <head>. Both placements are valid and covered by the custom-document ISR test — noting it's an intentional, not accidental, difference.

Nothing here blocks merge. Items 1-3 are optional clarifying comments for future maintainers.

[ask-bonk]

Review posted to PR #2894. Summary of findings:

Review Summary

Thorough review of the full diff, dev/prod parity across all four request-handling paths, and verification of the styled-jsx runtime shape against pinned styled-jsx@5.1.6. All relevant suites pass locally (14 styled-jsx, 137 pages-data/response, 5 pages-router streaming/ISR/static-export) and vp check is clean. No blocking issues found.

Key findings

• CodeQL alert is a false positive. The flagged freshStyledJsxHTML/styledJsxHTML come from React-rendering registry.styles() (developer-authored CSS via styled-jsx's own dangerouslySetInnerHTML). No user/request data flows into the content, and injection targets are trusted document templates. Matches Next.js behavior exactly — safe to dismiss.
• disableNextSsg: true fixes a real bug where SWC was silently stripping getStaticProps/getServerSideProps from vinext's server module graph. Well-covered by new tests/fixtures.
• ESM runtime conversion regexes all match the pinned runtime, with a sound fail-loud guard against future drift.
• ISR tee ordering holds with no backpressure deadlock — both cache-write and composite-stream branches independently drive the render and await the same finalShellSuffix.
• renderErrorPage refactor is behavior-preserving — the removed non-Document branch is subsumed by streamPageToResponse's default-shell path.

Non-blocking observations (optional clarifying comments)

1. Dev/prod encode different optionality assumptions for the styled-jsx import (both safe via the fallback runtime); a one-line comment on the prod static import would help.
2. STYLED_JSX_STYLE_PATTERN is coupled to id-first attribute ordering.
3. ISR regen intentionally relocates late styles from before-</body> to <head>.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(pages): preserve styled-jsx during streaming SSR

Independent pass over the full diff, with source-level verification against the pinned styled-jsx@5.1.6 runtime and dev/prod parity across renderPagesPageResponse, streamPageToResponse, _renderIsrPassToStringAsync, and the ISR regeneration path. Ran the relevant suites locally — all green (14 styled-jsx, 137 pages-data/response) — and vp check is clean on all six changed source files. No blocking issues found.

Verified against the actual runtime

• JSXStyle registers styles synchronously during SSR render (styled-jsx/dist/index/index.js:472-474: if (typeof window === "undefined") { registry.add(props); return null; }). This is the load-bearing fact for the whole design: shell styles are all present when renderToReadableStream resolves the shell, so the eager collectPagesStyleRegistryHtml → <head> collection is complete, and Suspense/late styles register only as their boundaries render, so gating the second collection on bodyAllReady captures exactly those. No overlap.
• flush() clears _indices/_instancesCounts (:373-379), so the eager collect-then-flush cannot double-emit into the late collection. Confirmed.
• registry.styles() always returns an array (mapRulesToStyle uses .map(), :288-303), so both the prod entry's styles.length > 0 guard and collectPagesStyleRegistryHtml's Array.isArray(styles) && styles.length === 0 check are safe.
• STYLED_JSX_STYLE_PATTERN id prefix is correct: computeId returns "jsx-" + baseId (:260) and mapRulesToStyle prepends "__" (:294), yielding id="__jsx-…", which the lookahead id=["']__jsx- matches.
• convertStyledJsxRuntimeToEsm matches the pinned runtime. The file tail is exactly exports.StyleRegistry = …; exports.createStyleRegistry = …; exports.style = JSXStyle; exports.useStyleRegistry = useStyleRegistry;\n, so all three replacements hit; the fail-loud throw on any no-op is a sound guard against future layout drift.

Verified correct

• ISR tee() has no backpressure deadlock. bodyAllReady is captured before tee(); finalShellSuffix awaits lateStyledJsxHTML → bodyAllReady. Both consumers drive the render independently — writePagesIsrCache drains the cache branch via readStreamAsText and buildPagesCompositeStream drains the response branch — and both await the same finalShellSuffix. Neither waits on an undrained branch. The bot path buffers compositeStream (which already awaits finalShellSuffix), so late styles land in the buffered/ETag'd HTML too.
• disableNextSsg: true fixes a real bug. Without it SWC's client-page tree-shaker rewrites getStaticProps/getServerSideProps to __N_SSG/__N_SSP and strips the functions from vinext's server module graph. The new styled-jsx-static-props/styled-jsx-streaming fixtures and the not.toContain("__N_SSP"/"__N_SSG") transform assertions lock this in.
• renderErrorPage refactor is behavior-preserving. The removed non-Document branch is subsumed by streamPageToResponse's else default-shell path (dev-server.ts:513-529), and preserveExistingContentType: statusCode === 404 reproduces the old conditional Content-Type. Error pages additionally gain styled-jsx.
• ISR head refresh is complete. removeStyledJsxStyles is global/unanchored so it strips __jsx- styles from both <head> and the pre-</body> region on every regen (including when the fresh render emits none), then re-inserts the fresh run into <head>. The document-isr-styled and isr-second-render-state (z-index: ${timestamp}) fixtures prove the styles actually advance and the stale ones are gone (not.toMatch(z-index: ${firstTimestamp})).
• Fallback runtime is a coherent no-op. No next → STYLED_JSX_FALLBACK_RUNTIME (empty registry, styles() → []) and styled-jsx/style → null; the generated entry's static import never crashes and emits no styles.

Non-blocking observations (agree with prior review)

1. Dev/prod encode different optionality for the styled-jsx import. Prod (pages-server-entry.ts) statically imports styled-jsx; dev (dev-server.ts:365) wraps it in try/catch. Both are safe because the plugin always resolves the id (real runtime or fallback), but a one-line comment on the prod static import stating why it is unconditionally safe would stop a future reader from "fixing" the asymmetry into an optional import (which would silently drop styles).
2. STYLED_JSX_STYLE_PATTERN and convertStyledJsxRuntimeToEsm are both coupled to the pinned runtime's shape (id-first attribute ordering / export tail). The ESM converter fails loud on drift; the style pattern does not. Acceptable given styled-jsx is pinned via next.

Nothing here blocks merge.

[ask-bonk]

Review posted to PR #2894.

Summary

I performed an independent review of PR #2894 (fix(pages): preserve styled-jsx during streaming SSR), verifying the implementation against the actual pinned styled-jsx@5.1.6 runtime source rather than just reading the diff. No blocking issues found.

Key verifications (against runtime source)

• JSXStyle registers styles synchronously during SSR render (index.js:472-474) — this is the load-bearing fact making the eager-collect (shell → <head>) + late-collect (Suspense → before </body>) split correct with no overlap.
• flush() clears the indices (:373-379), so eager + late collection cannot double-emit.
• registry.styles() always returns an array, so all length guards in both entries are safe.
• STYLED_JSX_STYLE_PATTERN's __jsx- id prefix and the convertStyledJsxRuntimeToEsm export-tail regex both match the pinned runtime exactly; the fail-loud throw guards against drift.

Verified correct

• ISR tee() has no backpressure deadlock (both branches independently drive the render; bot buffer path also awaits finalShellSuffix).
• disableNextSsg: true fixes a real bug (SWC was stripping getStaticProps/getServerSideProps).
• renderErrorPage refactor is behavior-preserving (removed branch subsumed by streamPageToResponse's default-shell path) and adds styled-jsx to error pages.
• ISR head refresh removes stale head+late styles and re-inserts fresh ones.
• Fallback runtime is a coherent no-op when next is absent.

Non-blocking (optional)

1. Dev/prod encode different optionality assumptions for the styled-jsx import (both safe) — a one-line comment on the prod static import would help future maintainers.
2. Regex coupling to the pinned runtime shape (acceptable; ESM converter fails loud).

Tests pass locally (14 styled-jsx, 137 pages-data/response) and vp check is clean on all six changed source files.

github run

[james-elicx]

Independent exact-head review of 7e1fd46eefdcd3c4eac98ff6a560f1e21a6258dd: NO FINDINGS.

I reviewed the full 13-file diff independently, including:

• request-local styled-jsx registry lifetime across shell readiness, Suspense completion, body streaming, and ISR tee/cache consumption;
• shell/late-style collection, nonce propagation, flush behavior, and cached-head regeneration/removal semantics;
• Pages dev, production, error-page, GSSP/GSP/ISR, custom _document, and relocated static-export paths;
• preservation of Pages data exports through the SWC styled-jsx transform and the downstream client-only server-export stripping boundary;
• the portable ESM runtime conversion, styled-jsx/style facade, relative-root resolution, missing-runtime fallback, and wrapper-shape failure behavior;
• the latest native load filter, including the corrected NUL-prefixed virtual IDs and its non-virtual fast-path exclusion.

Fresh validation on this immutable head:

• focused Vitest: 535/535 passed across styled-jsx, Pages data/response, dev/prod integration, ISR refresh, and actual relocated static-export output;
• exact targeted upstream suite: test/e2e/streaming-ssr/index.test.ts passed 5/5, retries 0/0;
• the required package builds completed successfully as part of the targeted harness;
• CI is fully green, local/remote head SHAs match, merge state is clean, git diff --check passed, and the worktree remained clean.

I found no actionable streaming/lifecycle gap, virtual-runtime or NUL-filter regression, dev/production/static-export divergence, or missing high-value test at this head.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(pages): preserve styled-jsx CSS ordering in SSR

Independent review of the full 13-file diff at head, with focus on dev/prod parity, the removed streaming marker, and the styled-jsx collection lifecycle. Ran the relevant suites locally — all green (14 styled-jsx, 139 pages-data/response, 5 pages-router streaming/ISR) — and vp check is clean on all six changed source files. No blocking issues found. A few non-blocking observations below, one of which corrects the record on prior reviews.

Verified correct

• convertStyledJsxRuntimeToEsm matches the installed runtime. Confirmed against styled-jsx@5.1.6/dist/index/index.js: require('client-only');, var React = require('react');, and the four-line exports.* tail all match the anchored regexes, and React__default/_interopDefaultLegacy still work after swapping to import * as React. The fail-loud throw on any no-op replacement is a sound guard against future layout drift.
• getRuntimeDistPath() resolution is correct. require.resolve("styled-jsx") → .../styled-jsx/index.js; dirname(...) + dist/index/index.js lands on the real runtime.
• disableNextSsg: true is valid and fixes a real bug. Confirmed the option exists in the installed next@16.2.7 SWC options, and the new transform test asserts getServerSideProps/getStaticProps survive without __N_SSG/__N_SSP. Previously these were silently stripped from vinext's server module graph.
• ISR head refresh is index-safe. refreshCachedHeadTags slices cachedHtml at indices computed from a length-preserving toLowerCase()/blank-out, so offsets stay valid; removeStyledJsxStyles strips __jsx- styles from both <head> and pre-</body> on every regen (including empty fresh renders).
• renderErrorPage refactor is behavior-preserving. The removed non-Document inline-HTML branch is subsumed by streamPageToResponse's else default-shell path (dev-server.ts:498-515), and preserveExistingContentType: statusCode === 404 reproduces the old conditional Content-Type. Error pages additionally gain styled-jsx.
• Fallback runtime is a coherent no-op when next/styled-jsx can't be resolved (styles() → [], styled-jsx/style → default resolution), so the prod entry's static import { StyleRegistry, createStyleRegistry } from "styled-jsx" never crashes.

Non-blocking observations

1. The prior reviews describe a "double-collect + flush (shell→<head>, then late→before </body>)" that does not exist in this final version. Both the dev path (dev-server.ts:407-417) and the prod path (pages-page-response.ts:610-623) do a single collect + flush after allReady, injecting all styled-jsx rules (shell and Suspense) before </body>. There is no eager <head> collection. This is fine functionally (styled-jsx styles are page-local and go after React's body stylesheet resources), but the "no double-emit" reasoning in the earlier ask-bonk/reviewer comments is verifying code that isn't here. Worth aligning the PR narrative so future readers aren't misled.

2. The __vinextStreamedHtmlResponse marker is now dead for HTML responses, but its infrastructure and tests remain. renderPagesPageResponse no longer sets the marker (it returns a plain buffered new Response(bufferedFullHtml, ...) and deletes Content-Length upstream at pages-page-response.ts:685). Consequently:

   • isVinextStreamedHtmlResponse(...) at prod-server.ts:2318 and the shouldStripStreamLength branch at worker-utils.ts:87 are now unreachable for HTML — the buffered path (arrayBuffer → recomputed framing) handles it correctly, so this is safe.
   • The marker-propagation in pages-request-pipeline.ts:866-870 and unified-request-context.ts:324-326 now only ever copies undefined.
   • deploy.test.ts:1595 ("...only for tagged streamed Pages HTML") and the equivalent in features.test.ts:4742 still set the marker synthetically to exercise mergeHeaders, so they pass but no longer reflect a real runtime code path.

   None of this is incorrect, but it's vestigial. Consider either removing the marker machinery (and its now-synthetic tests) or leaving a comment explaining it is retained only for the merge-header unit tests, so a future reader doesn't assume HTML responses still carry it.

3. Dev buffers the full document but still emits Transfer-Encoding: chunked (dev-server.ts:543, then a single res.write(prefix) + res.end(bufferedBody + finalSuffix)). Since the body is now fully known, this could set Content-Length instead (as prod does via sendCompressed). Valid HTTP either way; a minor dev-only efficiency/parity nit.

4. Dev/prod encode different optionality for the styled-jsx import. Prod (pages-server-entry.ts:212) statically imports it; dev (dev-server.ts:352) wraps it in try/catch. Both are safe because the plugin always resolves the id (real runtime or fallback), but a one-line comment on the prod static import stating why it is unconditionally safe would stop a future reader from "fixing" the asymmetry into an optional import (which would silently drop styles). (Also raised in prior reviews.)

5. removeStyledJsxStyles alternation includes <plaintext\b[^>]*>[\s\S]*$, which consumes to end-of-string once matched. React/Next never emit <plaintext>, so this is only a theoretical hazard for hand-authored cached HTML, but it means a stray <plaintext> before a styled-jsx block would swallow the rest of the document during regen. Extremely low risk given the pinned pipeline; flagging for completeness.

Nothing here blocks merge. Items 1-2 are the most useful to address so the codebase and its tests stay honest about what the streaming marker now means.

[ask-bonk]

Review posted to PR #2894.

Summary

I did an independent review of PR #2894 (fix(pages): preserve styled-jsx CSS ordering in SSR), focusing on dev/prod parity, the removed streaming marker, and the styled-jsx collection lifecycle. Ran the relevant suites locally (14 styled-jsx, 139 pages-data/response, 5 pages-router streaming/ISR — all green) and vp check is clean. No blocking issues.

Verified correct

• convertStyledJsxRuntimeToEsm regexes match the installed styled-jsx@5.1.6 runtime exactly; the fail-loud guard is sound.
• disableNextSsg: true is a real option in next@16.2.7 and fixes silent stripping of getServerSideProps/getStaticProps.
• ISR head refresh is index-safe (length-preserving lowercasing) and strips styled-jsx from both head and pre-</body>.
• The renderErrorPage refactor is behavior-preserving (removed inline branch subsumed by streamPageToResponse's default-shell path).
• The no-op fallback runtime keeps the prod entry's static styled-jsx import from ever crashing.

Non-blocking findings I raised (not covered by prior reviews)

1. The prior reviews describe a "double-collect (shell→head, late→body)" split that does not exist in the final code — both dev and prod do a single collect after allReady, all styles injected before </body>. Worth aligning the PR narrative.
2. The __vinextStreamedHtmlResponse marker is now dead for HTML — renderPagesPageResponse never sets it, so shouldStripStreamLength in worker-utils.ts and the branch in prod-server.ts:2318 are unreachable, and deploy.test.ts/features.test.ts only exercise it synthetically. Safe, but vestigial; suggest removing or commenting.
3. Dev buffers the whole document but still emits Transfer-Encoding: chunked (minor parity/efficiency nit vs prod's Content-Length).
4. Prod's static styled-jsx import vs dev's try/catch — a one-line comment on the prod import would prevent a future "fix" that silently drops styles.
5. removeStyledJsxStyles's <plaintext> alternation consumes to end-of-string (theoretical only).

Items 1 and 2 are the most valuable to address so the code and its tests stay honest about the streaming marker's current (non-)role.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(pages): preserve styled-jsx CSS ordering in SSR

Independent review at head c423475, verifying the styled-jsx collection lifecycle, ISR tee ordering, the removed streaming marker, and dev/prod parity against the pinned styled-jsx@5.1.6 runtime source. Ran the relevant suites locally — all green (14 styled-jsx, 72 pages-page-response, 465 pages-data/router) — including the new allReady-buffering and content-length integration tests. No blocking issues found.

Verified correct

• ESM runtime conversion matches the pinned runtime. Confirmed all three replacements in convertStyledJsxRuntimeToEsm (require('client-only'), var React = require('react'), and the four-line exports.* tail) hit against styled-jsx@5.1.6/dist/index/index.js. The fail-loud throw on any no-op is a sound guard against future layout drift. getRuntimeDistPath() resolves the runtime correctly via require.resolve + dist/index/index.js.
• Per-request registry isolation is safe. createStyleRegistry() produces a fresh StyleSheetRegistry per request (_indices/_instancesCounts are instance state). The only module-level mutable state (var cache = {} in computeId/computeSelector) is deterministic id/selector memoization, not per-request data, so concurrent requests cannot leak styles.
• ISR tee() ordering has no deadlock and no stale suffix. bodyAllReady is awaited before tee(), and finalShellSuffix is an eagerly-resolved Promise.resolve(injectPagesLateStyles(...)), so await options.shellSuffix in both the cache-write branch (writePagesIsrCache) and the composite branch never blocks on undrained data. Both tee branches are drained (readStreamAsText on each), so late styles land identically in the cached HTML and the served HTML.
• disableNextSsg: true is a valid option (confirmed present in the installed next@16.2.7 SWC options.d.ts:27) and fixes a real bug — without it SWC's client-page tree-shaker rewrites getStaticProps/getServerSideProps to __N_SSG/__N_SSP and strips them from vinext's server module graph. The new styled-jsx-streaming.tsx (gSSP) + styled-jsx-static-props.tsx fixtures and the transform assertions lock this in.
• The __vinextStreamedHtmlResponse marker is now genuinely dead and correctly removed everywhere. renderPagesPageResponse returns a plain buffered new Response(bufferedFullHtml, ...) with Content-Length deleted so the runtime recomputes framing. The marker read sites in prod-server.ts, worker-utils.ts (shouldStripStreamLength), and the copy sites in pages-request-pipeline.ts / unified-request-context.ts are all removed, and the now-synthetic deploy.test.ts/features.test.ts cases were deleted rather than left dangling. No stale source references remain (only dist/ build output, which is regenerated).
• renderErrorPage refactor is behavior-preserving. The removed non-Document inline-HTML branch is subsumed by streamPageToResponse's default-shell path, and preserveExistingContentType: statusCode === 404 reproduces the old conditional Content-Type. Error pages additionally gain styled-jsx. The error path re-throws on allReady rejection before any headers are written, so no partial/corrupt response is emitted.
• ISR head refresh is complete and index-safe. refreshCachedHeadTags strips all __jsx- styles from both <head> and pre-</body> on every regen (including empty fresh renders) via a length-preserving lowercase/blank-out before computing </head>/</body> offsets, then re-inserts the fresh run — consistent with the initial cache fill, which also injects late styles before </body>. The isr-second-render-state.tsx (z-index: ${timestamp}) fixture proves styles actually advance.

Non-blocking observations

1. Dev/prod encode different optionality for the styled-jsx import. Prod (pages-server-entry.ts:212) statically imports styled-jsx; dev (dev-server.ts:352) wraps it in try/catch. Both are safe because the plugin always resolves the id (real runtime or the STYLED_JSX_FALLBACK_RUNTIME no-op), but a one-line comment on the prod static import stating why it is unconditionally safe would stop a future reader from "fixing" the asymmetry into an optional import (which would silently drop styles). (Raised in prior reviews; still applies.)

2. Dev buffers the full document but still sends Transfer-Encoding: chunked (dev-server.ts:543, then res.write(prefix) + res.end(bufferedBody + finalSuffix)). Since the body is fully known, dev could set Content-Length like prod does. Valid HTTP either way — a minor dev-only parity/efficiency nit.

3. STYLED_JSX_STYLE_OPEN_PATTERN / the removeStyledJsxStyles <plaintext> alternation are coupled to the pinned runtime's output shape (id-first attribute ordering; <plaintext> consuming to end-of-string). React/Next never emit <plaintext> and styled-jsx always emits id first, so both are theoretical only — flagging for completeness alongside the (fail-loud) ESM-converter coupling.

Nothing here blocks merge. All three items are optional clarifying comments for future maintainers.

[ask-bonk]

Review posted to PR #2894.

Summary

I did an independent review of PR #2894 (fix(pages): preserve styled-jsx CSS ordering in SSR) at head c423475, verifying against the pinned styled-jsx@5.1.6 runtime source rather than just the diff. Ran the relevant suites locally (14 styled-jsx, 72 pages-page-response, 465 pages-data/router — all green) and confirmed the ESM conversion regexes match the installed runtime. No blocking issues found.

Key verifications

• ESM runtime conversion — all three regexes match styled-jsx@5.1.6; fail-loud guard is sound.
• Per-request registry isolation — each request gets a fresh registry; the only module-level state (computeId/computeSelector cache) is deterministic memoization, so no cross-request style leaks.
• ISR tee() ordering — allReady awaited before tee; finalShellSuffix is an eagerly-resolved promise, so both cache-write and composite branches drain without deadlock and get identical late styles.
• disableNextSsg: true — confirmed valid in next@16.2.7 and fixes real silent stripping of getStaticProps/getServerSideProps.
• Dead streaming marker — genuinely removed everywhere in source (only dist/ build output retains it); synthetic tests deleted rather than left dangling.
• renderErrorPage refactor — behavior-preserving; re-throws on allReady rejection before headers are written.
• ISR head refresh — index-safe (length-preserving lowercasing) and consistent with the initial cache fill.

Non-blocking observations

1. Prod static styled-jsx import vs dev try/catch — a one-line "why safe" comment would prevent a future "fix" that drops styles.
2. Dev buffers the full document but still emits Transfer-Encoding: chunked (minor parity/efficiency nit vs prod's Content-Length).
3. Regex coupling to the pinned runtime shape (id-first ordering, <plaintext> alternation) — theoretical only.

All items are optional clarifying comments; nothing blocks merge.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

UnknownError: ProviderInitError

github run

[ask-bonk]

@james-elicx Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(pages): preserve styled-jsx CSS ordering in SSR

Independent review at head, focused on correctness of the styled-jsx collection lifecycle, ISR head/style refresh, the removed __vinextStreamedHtmlResponse marker, and dev/prod parity. Verified the disableNextSsg: true change against vinext's own client-export stripping, and ran the relevant suites locally — all green (14 styled-jsx, 141 pages-page-response/data, 386 pages-router) — with vp check clean on all five changed source files. No blocking issues found.

Verified correct

• disableNextSsg: true is safe and does not leak server code to the client. My first concern was that disabling SWC's SSG tree-shaker on the shared transform would leave getServerSideProps/getStaticProps bodies (and any server secrets they close over) in the client bundle. Confirmed this is fine: vinext does not rely on SWC's __N_SSG/__N_SSP mechanism — it has its own dedicated stripServerExports plugin (plugins/strip-server-exports.ts, wired at index.ts:5939) that removes server data functions from the client graph. The SWC tree-shaker was actively harmful here because it also ran over vinext's server module graph and made the data functions vanish at runtime. The new styled-jsx-streaming.tsx (gSSP) fixture + transform assertions lock this in. Real bug, correct fix.
• allReady rejection is handled before any cache write or header emission. In renderPagesPageResponse, await bodyAllReady (line 624) throws before the ISR tee()/schedulePagesIsrCacheWrite (line 730) and before any response is constructed, so a failed render never persists partial/corrupt HTML to the ISR cache and never emits headers. The registry is flushed on the error path to release memory. Same shape in dev's streamPageToResponse.
• Content-Length handling is self-consistent after removing the streaming marker. renderPagesPageResponse deletes any stale Content-Length (line 699) before building new Response(bufferedFullHtml, { headers }); constructing a Response from a string body recomputes correct framing. The now-removed shouldStripStreamLength/isVinextStreamedHtmlResponse machinery in worker-utils.ts, prod-server.ts, pages-request-pipeline.ts, and unified-request-context.ts was only needed for the old streaming path — its removal is complete and the synthetic marker tests in deploy.test.ts/features.test.ts were deleted rather than left dangling, while the "untagged" content-length coverage remains.
• ISR style refresh is placement-consistent with the initial cache fill. On first fill, injectPagesLateStyles puts all styled-jsx rules before </body> (never in <head>), so on regen removeStyledJsxStyles correctly targets that region and refreshCachedHeadTags re-injects the fresh run before </body> — the next/head <head> refresh is orthogonal. The isr-second-render-state.tsx (z-index: ${timestamp}) fixture proves styles actually advance.
• STYLED_JSX_STYLE_OPEN_PATTERN anchoring is correct — it tests ^<style ...id="__jsx- against each full matched element (match[0]), and styled-jsx's mapRulesToStyle emits id first with the __jsx- prefix.
• Dev/prod both resolve styled-jsx through the same plugin. Dev imports via the Vite runner (importModule(runner, "styled-jsx")), prod statically imports; both hit the plugin's resolveId, and the fallback runtime is a coherent no-op (styles() → []) so neither path crashes when next/styled-jsx is unresolvable.

Non-blocking observations

1. Dev buffers the full document but still sends Transfer-Encoding: chunked (dev-server.ts:576, then res.write(prefix) + res.end(bufferedBody + finalSuffix)). Since the body is now fully known, dev could set Content-Length like the prod/Worker path. Valid HTTP either way — a minor dev-only parity/efficiency nit.

2. Dev/prod encode different optionality for the styled-jsx import. Prod (pages-server-entry.ts:212) statically imports it; dev (dev-server.ts:352) wraps the import in try/catch. Both are safe because the plugin always resolves the id (real runtime or fallback), but a one-line comment on the prod static import explaining why it is unconditionally safe would stop a future reader from "fixing" the asymmetry into an optional import (which would silently drop styles). (Raised in prior reviews; still applies.)

3. removeStyledJsxStyles's <plaintext\b[^>]*>[\s\S]*$ alternation consumes to end-of-string. If a <plaintext> ever appeared in cached HTML before a styled-jsx <style>, that alternation would swallow the style so it wouldn't be stripped/refreshed on regen. React/Next never emit <plaintext>, so this is theoretical only — flagging for completeness alongside the (fail-loud) convertStyledJsxRuntimeToEsm runtime-shape coupling.

Nothing here blocks merge. All three items are optional clarifying comments for future maintainers.

[ask-bonk]

Review posted to PR #2894. Summary of findings:

Review Summary

I did an independent review of PR #2894 (fix(pages): preserve styled-jsx CSS ordering in SSR), verifying the styled-jsx collection lifecycle, ISR refresh, the removed streaming marker, and dev/prod parity. Ran the relevant suites locally (14 styled-jsx, 141 pages-response/data, 386 pages-router — all green) and vp check is clean. No blocking issues found.

Key verifications (including one my earlier suspicion cleared)

• disableNextSsg: true is safe — my initial concern was that disabling SWC's SSG tree-shaker on the shared transform might leak getServerSideProps/getStaticProps bodies (and secrets) into the client bundle. Confirmed it doesn't: vinext has its own stripServerExports plugin that handles client stripping, so the SWC mechanism was only doing harm (also stripping the functions from the server graph). Real bug, correct fix.
• allReady rejection is handled before any cache write or headers — no partial HTML is ever cached and no headers are emitted on a failed render; the registry is flushed on error.
• Content-Length handling is self-consistent after removing the __vinextStreamedHtmlResponse marker — the buffered new Response(string) recomputes framing, and the marker removal is complete across all five sites with synthetic tests deleted (not dangling).
• ISR style placement is consistent between first fill and regeneration (all styled-jsx before </body>).

Non-blocking observations

1. Dev buffers the full document but still emits Transfer-Encoding: chunked (minor dev-only parity/efficiency nit vs prod's Content-Length).
2. Prod static styled-jsx import vs dev try/catch — a one-line "why safe" comment would prevent a future "fix" that drops styles.
3. removeStyledJsxStyles's <plaintext> alternation consumes to end-of-string (theoretical only; React/Next never emit it).

All items are optional clarifying comments; nothing blocks merge.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

UnknownError: ProviderInitError

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(pages): preserve styled-jsx CSS ordering in SSR

Independent review at branch head a76fb59. I verified the styled-jsx collection lifecycle, the array-root render path, disableNextSsg safety against client leakage, the ISR head/style refresh, and dev/prod parity against the pinned styled-jsx@5.1.6 runtime source. Ran the relevant suites locally — all green (14 styled-jsx, 74 pages-page-response, 467 pages-data/router) — and tsc --noEmit is clean on the vinext package. No blocking issues found.

Verified correct

• registry.styles() returns an array, and passing it as the render root is safe. mapRulesToStyle (styled-jsx/dist/index/index.js:288) returns cssRules.map(...), so collectPagesStyleRegistryHtml hands an array straight to renderToReadableStream(styles) (no Fragment wrap, unlike the entry's ISR path). I confirmed empirically that react-dom/server.edge's renderToReadableStream accepts an array root and renders it without error (<style>…</style><style>…</style>), so the unwrapped array is not a bug.
• disableNextSsg: true does not leak server code to the client. The styled-jsx transform runs across all environments, but client stripping is owned by the separate vinext:strip-server-exports plugin (index.ts:5919, gated on this.environment?.name === "client" + pages-dir), which fires on any page whose code contains getServerSideProps/getStaticProps. SWC's __N_SSG/__N_SSP tree-shaker was actively harmful here because it also ran over the server graph and made the data functions vanish at runtime. Real bug, correct fix; the new styled-jsx-streaming.tsx (gSSP) + styled-jsx-static-props.tsx fixtures lock it in.
• allReady rejection is handled before any headers or cache write. await bodyAllReady throws (flushing the registry) before the ISR tee()/schedulePagesIsrCacheWrite and before any Response is constructed, so a failed render never persists partial HTML or emits headers. Same shape in dev's streamPageToResponse.
• ISR tee() has no undrained branch. bodyAllReady is awaited before tee(); finalShellSuffix is an eagerly-resolved Promise.resolve(...). bufferedFullHtml drains branch 0, writePagesIsrCache/schedulePagesIsrCacheWrite drains branch 1, and both await the same already-resolved suffix. No deadlock.
• The __vinextStreamedHtmlResponse marker is fully removed. No source or test references remain (prod-server.ts, worker-utils.ts, pages-request-pipeline.ts, unified-request-context.ts all cleaned; synthetic deploy.test.ts/features.test.ts cases deleted). renderPagesPageResponse now deletes stale Content-Length and returns a buffered new Response(string), which recomputes framing correctly.
• ISR head/style refresh is placement-consistent and index-safe. removeStyledJsxStyles strips __jsx- styles from both <head> and pre-</body> on every regen (including empty fresh renders); refreshCachedHeadTags recomputes </head> on the already-stripped string via a length-preserving lowercase/blank-out, then re-injects the fresh run before </body>, matching the initial cache fill's injectPagesLateStyles placement. The isr-second-render-state.tsx (z-index: ${timestamp}) fixture proves styles advance.
• renderErrorPage refactor is behavior-preserving. The removed non-Document inline-HTML branch is subsumed by streamPageToResponse's default-shell path; the new getHeadHTML conditionally seeds charset/viewport only when Document is absent, and preserveExistingContentType: statusCode === 404 reproduces the old conditional Content-Type. Error pages additionally gain styled-jsx.

Non-blocking observations

1. injectPagesLateStyles/collectPagesStyleRegistryHtml are byte-for-byte duplicated as injectDevPagesLateStyles/collectDevPagesStyleRegistryHtml in dev-server.ts. AGENTS.md asks that response-shaping/streaming logic live in shared server/* helpers so dev and prod stay in lockstep. These four are pure functions with no dev/prod-specific behavior — a future edit to the </body> injection or the empty-styles guard has to be mirrored by hand across both files or they silently drift. Consider hoisting them into a shared module (e.g. server/pages-styled-jsx.ts) that both pages-page-response.ts and dev-server.ts import.

2. Dev buffers the whole document but still sends Transfer-Encoding: chunked (dev-server.ts:576, then res.write(prefix) + res.end(bufferedBody + finalSuffix)). Since the body is fully known, dev could set Content-Length like the prod/Worker path. Valid HTTP either way — a minor dev-only parity/efficiency nit.

3. Dev/prod encode different optionality for the styled-jsx import. Prod (pages-server-entry.ts:212) statically imports it; dev (dev-server.ts:352) wraps it in try/catch. Both are safe because the plugin always resolves the id (real runtime or the STYLED_JSX_FALLBACK_RUNTIME no-op), but a one-line comment on the prod static import stating why it is unconditionally safe would stop a future reader from "fixing" the asymmetry into an optional import (which would silently drop styles). (Raised in prior reviews; still applies.)

4. removeStyledJsxStyles's <plaintext\b[^>]*>[\s\S]*$ alternation consumes to end-of-string. If a <plaintext> ever preceded a styled-jsx <style> in cached HTML, that style would be swallowed and neither stripped nor refreshed on regen. React/Next never emit <plaintext>, so this is theoretical only — flagging for completeness alongside the (fail-loud) convertStyledJsxRuntimeToEsm runtime-shape coupling.

Nothing here blocks merge. Item 1 (the shared-helper dedup) is the most worth addressing per the repo's own entry/server layering guidance.

[ask-bonk]

Review posted to PR #2894.

Review Summary

I did an independent review of PR #2894 (fix(pages): preserve styled-jsx CSS ordering in SSR) at branch head a76fb59, verifying against the pinned styled-jsx@5.1.6 runtime source rather than just reading the diff. Ran the relevant suites locally (14 styled-jsx, 74 pages-page-response, 467 pages-data/router — all green) and confirmed tsc --noEmit is clean. No blocking issues found.

Key verifications

• Array-root render is safe — registry.styles() returns an array (.map()), passed unwrapped to renderToReadableStream; I confirmed empirically React accepts an array root and renders it correctly, so the missing Fragment wrap (vs. the entry's ISR path) is not a bug.
• disableNextSsg: true does not leak server code to the client — client stripping is owned by the separate vinext:strip-server-exports plugin (environment-gated), so the SWC tree-shaker was only doing harm to the server graph. Real bug, correct fix.
• allReady rejection handled before headers/cache write, ISR tee() has no undrained branch/deadlock, and the __vinextStreamedHtmlResponse marker is fully removed from source and tests.
• ISR head/style refresh is placement-consistent and index-safe; renderErrorPage refactor is behavior-preserving.

Non-blocking observations

1. Byte-identical duplication of injectPagesLateStyles/collectPagesStyleRegistryHtml between pages-page-response.ts and dev-server.ts — AGENTS.md asks these to live in shared server/* helpers so dev/prod can't drift. Most worth addressing.
2. Dev buffers the full document but still emits Transfer-Encoding: chunked (minor parity nit vs prod's Content-Length).
3. Prod static styled-jsx import vs dev try/catch — a one-line "why safe" comment would help.
4. removeStyledJsxStyles's <plaintext> alternation consumes to end-of-string (theoretical only).

Nothing blocks merge.

github run