fix(pages): align GSSP client transitions

[james-elicx]

Summary

• preserve Pages router state when a client-transition GSSP route returns notFound
• align GSSP readiness metadata and data-route behavior
• reject non-serializable GSSP props in production like Next.js

Validation

• 45 focused unit tests
• 7 focused Pages production E2E tests
• scoped vp check and final cumulative review: clean

Next.js parity

Targets four actionable assertions from test/e2e/getserversideprops/test/index.test.ts in deploy-suite run 27858251903. Assertions already owned by PR #2102 were excluded. No raw Next.js E2E was run locally.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

commit: e7f1a13

[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 e7f1a13 against base cf9d601 using alternating same-runner rounds. Next.js was unchanged and skipped.

1 improved · 0 regressed · 5 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	132.8 KB	132.9 KB	⚫ +0.1%
Client entry size (gzip)	vinext	118.7 KB	118.7 KB	⚫ -0.0%
Dev server cold start	vinext	2.43 s	2.39 s	🟢 -1.7%
Production build time	vinext	2.89 s	2.88 s	⚫ -0.3%
RSC entry closure size (gzip)	vinext	92.2 KB	92.2 KB	⚫ 0.0%
Server bundle size (gzip)	vinext	166.9 KB	166.9 KB	⚫ -0.0%

View detailed results and traces

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

[ask-bonk]

/bigbonk review

Reviewed the GSSP client-transition parity changes against the Next.js reference (pages-handler.ts, render.tsx, shared/lib/router/router.ts). Most of the PR is solid and well-matched to Next.js, but there is one blocking issue: a shared 404-body helper was overloaded, which both fails an existing test and introduces a Next.js parity divergence for fallback: false data-route misses.

Blocking — fallback: false data misses must stay {} (hard-navigate), not {"notFound":true}

The PR repurposes the shared buildPagesDataNotFoundResponse to emit {"notFound":true} for all three callers, but only the gSSP/gSP notFound: true cases should carry that marker. The third caller is the getStaticPaths fallback: false miss:

packages/vinext/src/server/pages-page-data.ts:636-640

if (fallback === false && !isValidPath) {
  // ...returns buildPagesNotFoundResult(options) -> now '{"notFound":true}'
  return buildPagesNotFoundResult(options);
}

Consequences:

1. Existing test now fails (CI will be red). tests/pages-router.test.ts:2112 ("returns JSON 404 when getStaticPaths fallback:false rejects the path") still asserts {}:

   AssertionError: expected { notFound: true } to deeply equal {}
   ❯ tests/pages-router.test.ts:2122:32
   

   The PR did not update this test, and the test's own comment documents the intended contract: "a JSON 404 for data requests ... so the client router can hard-navigate instead of failing to parse HTML as JSON."

2. Diverges from Next.js. In Next.js, a fallback: false non-prerendered dynamic route throws NoFallbackError (pages-handler.ts:198-204) and returns a plain 404 without a {notFound:true} body — the client's fetchNextData then takes the Failed to load static props → server-side-transition (hard navigation) path. The {"notFound":true} envelope is reserved for the genuine notFound: true data case (pages-handler.ts:601-619). With this PR, the new client logic in shims/router.ts:1710-1738 will now soft-render /404 for a fallback: false miss instead of hard-navigating, which is a behavioral change vs Next.js (skips the server-side transition).

Same bug in dev (parity): packages/vinext/src/server/dev-server.ts:761 makes the identical change in the fallback === false && !isValidPath branch — and the adjacent comment (lines 750-751) still says "hard-navigate instead of trying to parse HTML as JSON," now contradicting the emitted body.

Suggested fix: keep the fallback: false miss on the {} (deploy-skew/hard-navigate) shape and only emit {"notFound":true} for the gSSP/gSP notFound: true exits. e.g. split the helper (a notFound-marker variant vs the empty-object variant), route pages-page-data.ts:636-640 + dev-server.ts:761 to the empty-object variant, and route the two real notFound: true exits (pages-page-data.ts:742, :939; dev-server.ts:909, :1345) to the marker variant. Then tests/pages-router.test.ts:2122 stays green without modification.

Confirmed correct

• gSSP/gSP notFound: true → {"notFound":true} matches Next.js pages-handler.ts:619 exactly, and the client detection in shims/router.ts:1713-1714 mirrors shared/lib/router/router.ts:542 (tryToParseAsJSON(text)?.notFound). The soft-404 + nextDataRouteState route-state preservation (pathname/route/query/asPath pointed at the requested route) is the right Next.js behavior.
• Removing the GSSP isSerializableProps check from production runtime is correct. Next.js gates that check on (process.env.__NEXT_DEV_SERVER || isBuildTimeSSG) (render.tsx:1198-1200), so prod GSSP must not validate — the dev check in dev-server.ts:931-937 is correctly retained, preserving dev/prod parity with Next.js. (Note: the PR body says "reject non-serializable GSSP props in production like Next.js," which is the opposite of what the diff does — the wording should be corrected to avoid confusion, but the code is right.)
• Readiness serialization (pages-readiness.ts) emitting undefined for false markers correctly omits them from __NEXT_DATA__ (matching Next.js) and is safe — all consumers use strict === true checks (shims/router.ts:992,1064,1307-1316).

Nitpick

• tests/fixtures/pages-basic/pages/gssp-non-json.tsx: returning a Date exercises the non-serializable path nicely, but only the SSR text is asserted. After hydration, time.toString() runs on a JSON-deserialized string, not a Date — fine for this test, just be aware the value differs client-side (which is the documented Next.js footgun this is demonstrating).

Bottom line: fix the fallback: false regression (it fails CI and diverges from Next.js); the rest is good.

[ask-bonk]

Posted the review on PR #2240. Summary of what I found:

Blocking issue

The PR overloads the shared buildPagesDataNotFoundResponse helper to emit {"notFound":true} for all three callers, but only the gSSP/gSP notFound: true cases should carry that marker. The getStaticPaths fallback: false miss (pages-page-data.ts:636-640 and dev-server.ts:761) gets incorrectly caught up in this, causing:

1. CI failure — existing test tests/pages-router.test.ts:2122 ("returns JSON 404 when getStaticPaths fallback:false rejects the path") asserts {} and now fails with expected { notFound: true } to deeply equal {}. The PR didn't update it (and shouldn't — the test documents the correct Next.js contract).

2. Next.js parity divergence — In Next.js, fallback: false misses throw NoFallbackError → plain 404 → client hard-navigates. The {"notFound":true} envelope is reserved for genuine notFound: true exits (pages-handler.ts:601-619). This PR makes fallback: false misses soft-render /404 instead.

Suggested fix: split the helper so fallback: false misses keep {} and only the real gSSP/gSP notFound: true exits emit {"notFound":true}.

Confirmed correct

• gSSP/gSP notFound: true → {"notFound":true} exactly matches pages-handler.ts:619, and the client detection mirrors Next.js router.ts:542.
• Removing the GSSP isSerializableProps check from production is correct — Next.js gates it on dev/build-time only (render.tsx:1198-1200). The PR body's "reject ... in production" wording is inverted, but the code is right.
• Readiness undefined-omission is safe (all consumers use === true).

I cross-checked everything against a fresh Next.js canary clone and ran the affected test files to confirm the regression.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review

Re-reviewed at e7f1a13 (two commits — 452b507, e7f1a13 — landed after the prior review at ae0a321). The prior blocking issue is fully resolved. No new issues found. This is ready to merge.

Prior blocker — resolved

The earlier review flagged that the shared buildPagesDataNotFoundResponse helper was overloaded to emit {"notFound":true} for all three callers, which (1) failed tests/pages-router.test.ts:2122 and (2) diverged from Next.js for fallback: false data misses. That is now fixed by splitting the helper:

• buildPagesDataFallbackMissResponse → {} (pages-page-data.ts:286), routed from the fallback: false && !isValidPath data exit (pages-page-data.ts:654)
• buildPagesDataNotFoundResponse → {"notFound":true} (pages-page-data.ts:269), routed only from the genuine gSSP/gSP notFound: true exits via buildPagesNotFoundResult (pages-page-data.ts:759, :956)

Dev parity confirmed: dev-server.ts:761 emits {} for the fallback: false miss; dev-server.ts:909 emits {"notFound":true} for gSSP notFound: true. The comment at dev-server.ts:750-753 was updated to match the emitted body (no longer contradictory).

Tests updated to match the split (tests/pages-page-data.test.ts):

• getStaticPaths excludes a path → {} (line 352)
• getStaticProps / getServerSideProps returns notFound → { notFound: true } (lines 373, 394)

I ran the affected suites locally — all green:

• tests/pages-router.test.ts, tests/pages-page-data.test.ts, tests/pages-readiness.test.ts → 361 passed (incl. the previously-failing pages-router.test.ts:2122)
• tests/shims.test.ts, tests/pages-readiness.test.ts → 1144 passed
• vp check on the four changed source files → clean (format, lint, types)

Confirmed correct (carried forward + re-verified)

• gSSP/gSP notFound: true → {"notFound":true} matches Next.js pages-handler.ts:619. Client detection in shims/router.ts:1713-1714 mirrors router.ts:542 (tryToParseAsJSON(text)?.notFound). The soft-404 + nextDataRouteState route-state preservation (shims/router.ts:1729-1736, merged into __NEXT_DATA__ at :2060) points pathname/route/query/asPath at the requested route — verified end-to-end by the new prod E2E (production.spec.ts: pathname: "/gssp-not-found/[slug]", slug: "first", correct asPath/nextDataPage).
• Production gSSP isSerializableProps check removed (pages-page-data.ts, formerly the result?.props !== undefined → isSerializableProps(..., "getServerSideProps", ...) block). The gSP check is correctly retained (pages-page-data.ts:968), and the dev gSSP check is retained (dev-server.ts:932). This matches Next.js gating the gSSP check on dev/build-time only, preserving dev/prod parity.
• Readiness serialization (pages-readiness.ts): emitting undefined for false markers omits them from __NEXT_DATA__ (matches Next.js). Type-safe — all five fields are boolean | undefined (shims/internal/utils.ts:16-25), and every consumer uses strict === true (shims/router.ts:992,1064,1307-1316). The appGip === true && nextData.gsp !== true guard at :1310 is unaffected (undefined !== true).
• Prod parity for the fallback split: the Pages production handler delegates to resolvePagesPageData (no separate fallback: false branch in prod-server.ts/worker-entry.ts), so it inherits the fix automatically. Only dev-server.ts has a parallel implementation, and it was updated.

Nitpicks (non-blocking)

• PR body wording is inverted. It says "reject non-serializable GSSP props in production like Next.js," but the diff removes the production gSSP check (which is the correct behavior). Suggest rewording to "skip non-serializable GSSP validation in production, matching Next.js (dev/build-time only)" to avoid confusing future readers.
• tests/fixtures/pages-basic/pages/gssp-non-json.tsx: returning a Date exercises the non-serializable path, but only SSR text is asserted. After hydration, time.toString() runs on a JSON-deserialized string, not a Date — fine for this test, just the documented Next.js footgun being demonstrated.

Bottom line: the prior blocker is resolved correctly, parity with Next.js holds, and all affected tests pass. Approve.

[ask-bonk]

Posted the review to PR #2240.

Summary

I re-reviewed PR #2240 at the latest commit e7f1a13. The blocking issue from the prior ask-bonk review is fully resolved — two new commits (452b507, e7f1a13) landed the fix. No new issues found; ready to merge.

What was fixed

The previously-overloaded buildPagesDataNotFoundResponse helper is now split into two:

• buildPagesDataFallbackMissResponse → {} for fallback: false misses (hard-navigate, matches Next.js NoFallbackError path)
• buildPagesDataNotFoundResponse → {"notFound":true} only for genuine gSSP/gSP notFound: true exits

Dev parity confirmed (dev-server.ts:761 emits {}, :909 emits {"notFound":true}, comment corrected). Tests updated to assert the correct shapes.

Verification I ran

• tests/pages-router.test.ts, tests/pages-page-data.test.ts, tests/pages-readiness.test.ts → 361 passed (incl. the previously-failing pages-router.test.ts:2122)
• tests/shims.test.ts + readiness → 1144 passed
• vp check on the 4 changed source files → clean

Confirmed correct

• Route-state preservation via nextDataRouteState (verified by new prod E2E)
• Production gSSP isSerializableProps removal (dev check retained — correct Next.js dev/prod parity)
• Readiness undefined-omission is type-safe; all consumers use === true
• Prod inherits the fallback fix via delegation to resolvePagesPageData

Nitpicks (non-blocking)

• PR body wording is inverted ("reject ... in production" — the code removes the prod check, which is correct)
• The Date fixture only asserts SSR text (documented Next.js footgun)

github run