fix(pages-router): cancel deduped GSSP data waits

[james-elicx]

vinext parity: concurrent Pages gSSP data navigation dedup and cancellation

Date: 2026-06-15
Branch: codex/wave2-pages-gssp-dedup
Base: origin/main at a3d2f921 (fix(router): honor hybrid pages route priority (#1997))

Scope

• Pages Router getServerSideProps client data navigation only.
• Excludes cacheComponents, PPR, and resume behavior.
• Adds in-flight /_next/data request identity, waiter-aware shared cancellation, and stale-response protection.

Upstream Next.js references

• Test: .nextjs-ref/test/e2e/getserversideprops/test/index.test.ts
  • should not trigger an error when a data request is cancelled due to another navigation
  • should dedupe server data requests
• Implementation: .nextjs-ref/packages/next/src/shared/lib/router/router.ts
  • fetchNextData() resolves dataHref against window.location.href and keys inflightCache by the resolved URL.
  • Router cancellation uses the active cancellation callback and emits a cancelled error without committing stale route data.

Implementation

• packages/vinext/src/shims/internal/pages-data-fetch-dedup.ts
  • Keys in-flight requests by resolved data URL plus x-deployment-id.
  • URL identity naturally keeps locale path, basePath, and query-string variants distinct.
  • Stores a shared AbortController, promise, settlement flag, and active waiter count per key.
  • Each caller releases exactly one waiter on response, rejection, or cancellation.
  • Cancelling one caller keeps the fetch alive while another waiter remains.
  • Cancelling the final waiter immediately evicts and aborts the shared fetch.
  • Both cancellation and settlement use entry identity checks, so an old finalizer cannot evict a replacement request.
• packages/vinext/src/shims/router.ts
  • Threads each navigation's abort signal into shared Pages data fetches and middleware data probes.
  • Defers superseded-navigation abort by one microtask so synchronous identical pushes can join the shared request before the prior waiter releases.
  • Navigation IDs still supersede stale work immediately; existing events, history writes, and stale-response checks remain intact.

Coverage

• Helper tests:
  • identical concurrent URLs share one fetch;
  • locale/basePath/query/deployment variants remain distinct;
  • one cancelled caller does not abort the shared fetch;
  • all callers cancelling aborts the underlying fetch and evicts immediately;
  • a later identical caller starts and successfully consumes a replacement request;
  • the aborted old request settling after replacement does not evict that replacement;
  • settled/rejected requests evict and retry.
• Deferred router test:
  • repeated identical Router.push() calls issue one fetch;
  • stale navigation emits one { cancelled: true } route error;
  • only the latest navigation completes, loads, renders, and commits __NEXT_DATA__;
  • both pushes keep their existing history/event return behavior.
• Production browser fixture:
  • four identical pushes produce one /_next/data/...gssp-dedup-slow.json?key=same request and server hit 1;
  • different query keys produce distinct data requests;
  • a superseded slow request lands on the newer route with no console/page errors.

Validation

Passed:

• vp test run tests/pages-data-fetch-dedup.test.ts tests/pages-data-prefetch.test.ts tests/shims.test.ts -t "Pages Router _next/data client navigation|dedupedPagesDataFetch|prefetchPagesData"
  • 21 passed, 1089 skipped by the targeted name filter.
• PLAYWRIGHT_PROJECT=pages-router-prod npx playwright test tests/e2e/pages-router-prod/gssp-data-dedup.spec.ts --project=pages-router-prod
  • 1 passed; the project command rebuilt vinext, built the Pages fixture, and ran it with vinext start.
• vp check <changed files>
  • formatting, lint, and type checks clean.
• git diff --check
  • clean.

Review notes

• Review finding addressed: final-waiter cancellation now aborts the underlying fetch and permits immediate replacement.
• Self-review specifically checked waiter underflow, cancel/settle races, stale finalizer eviction, and synchronous identical-push ordering.
• No cacheComponents/PPR/resume files or behavior touched.
• No server-side request cache introduced; dedup is browser-realm and in-flight only.
• No push or PR created. Commit remains local for clean review.

[pkg-pr-new]

Open in StackBlitz

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

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

commit: 5568e6d

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

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #2038 (cancel deduped GSSP data waits)

I reviewed the waiter-aware dedup/cancellation logic, the router signal threading, the deferred-abort change, and the tests. The core design is sound: waiter accounting is guarded against double-release, final-waiter eviction + abort works, the settle/abort race is handled via entry-identity checks, and I could not reproduce any unhandled-promise-rejection (every entry.promise always has a .then/.catch attached through cloneSharedResponse). Targeted tests pass locally (pages-data-fetch-dedup, pages-data-prefetch, the new shims nav test).

A few non-blocking observations:

1. Comment/code mismatch on the deferred abort (router.ts:1902-1907)

// different destinations still abort the abandoned request in this turn.
const previousAbortController = routerRuntimeState.activeAbortController;
if (previousAbortController) queueMicrotask(() => previousAbortController.abort());

The abort is always deferred by one microtask, including for different destinations — the comment says different destinations abort "in this turn," which is not what the code does. The behavior is still correct (the old waiter releases one microtask later and the underlying fetch is then aborted/evicted because no other waiter shares its key), but the comment overstates the timing distinction. Suggest rewording to match: supersession via navigationId is synchronous; the AbortSignal is always deferred by a microtask regardless of destination.

2. Abort cancellation correctness in resolveMiddlewareDataEffect relies on assertStillCurrent() (router.ts:1426-1441)

Now that signal is threaded into the middleware data probe, an aborted probe rejects with AbortError, which the catch { return null; } swallows into "no middleware effect." The navigation then proceeds, and only assertStillCurrent() at router.ts:1944 saves it (because in production an abort only ever fires alongside a navigationId bump). This is correct today but fragile — it couples the safety of the swallowed abort to the signal↔navigationId pairing in navigateClient. Consider re-throwing AbortError from this helper (like the gSSP path at router.ts:1532-1535 does) so cancellation is explicit rather than implicit. Not a blocker since assertStillCurrent() currently backstops it.

3. Key resolution against window.location.href (pages-data-fetch-dedup.ts:47-57)

getInflightKey resolves dataHref against window.location.href. For the normal root-relative /_next/data/... hrefs this is stable, but if a prior navigation has already replaceState'd the URL between a prefetch and the real navigation, the resolved key could differ for the same logical destination and miss the dedup. Edge case only; current fixtures don't exercise it. Worth a comment noting the assumption that dataHref is origin-stable.

Verification done

• vp test run tests/pages-data-fetch-dedup.test.ts tests/pages-data-prefetch.test.ts — 8 passed
• vp test run tests/shims.test.ts -t "Pages Router _next/data client navigation" — 13 passed
• Confirmed gssp-redirect-target.tsx (normal-text / "a normal page") exists for the push-cancelled E2E assertion
• Confirmed consoleErrors fixture auto-fails on console errors (the void consoleErrors; pattern matches repo convention), so the "stays silent" check is active

Overall this looks correct and well-tested. The notes above are clarity/robustness improvements, not correctness blockers.

[ask-bonk]

Review posted to PR #2038.

Summary

I reviewed PR #2038 (fix(pages-router): cancel deduped GSSP data waits). The core implementation is correct and well-tested. I verified:

• Waiter accounting is guarded against double-release (released flag).
• Final-waiter eviction + abort works, and the settle/abort race is handled via entry-identity checks (inflight.get(key) === entry).
• No unhandled promise rejections — I reproduced the all-callers-cancel and clear scenarios in a standalone Node script; entry.promise always has a .then attached through cloneSharedResponse.
• Relevant tests pass locally (dedup unit tests, prefetch test, new shims nav test).

I posted three non-blocking observations:

1. Comment/code mismatch in router.ts:1902-1907 — the abort is always deferred by a microtask, contradicting the comment claiming different destinations abort "in this turn."
2. Fragile abort handling in resolveMiddlewareDataEffect (router.ts:1426-1441) — the now-threaded signal causes AbortError to be swallowed into null; correctness relies on the downstream assertStillCurrent() backstop rather than explicit re-throw.
3. Key resolution against window.location.href (pages-data-fetch-dedup.ts:47-57) — an edge case where a replaceState between prefetch and navigation could miss dedup; worth a comment about the origin-stable assumption.

None are correctness blockers — they're clarity/robustness improvements.

github run