[PR #868] [MERGED] fix(app-router): keep isPending true across programmatic router.push #908

New issue

Closed

opened 2026-05-06 13:10:46 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:10:46 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/868
Author: @NathanDrake2406
Created: 4/22/2026
Status: ✅ Merged
Merged: 4/22/2026
Merged by: @james-elicx

Base: main ← Head: fix/programmatic-router-push-pending-transition

📝 Commits (2)

5a4b984 fix(app-router): track pending router state for programmatic navigations
12ee56d address review feedback on programmatic router pending state

📊 Changes

7 files changed (+290 additions, -37 deletions)

View changed files

📝 packages/vinext/src/global.d.ts (+1 -0)
📝 packages/vinext/src/server/app-browser-entry.ts (+155 -31)
📝 packages/vinext/src/shims/navigation.ts (+21 -5)
➕ tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts (+45 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/router-push-pending/page.tsx (+26 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/router-push-pending/pending-client.tsx (+28 -0)
📝 tests/form.test.ts (+14 -1)

📄 Description

Closes #639.

What this changes

router.push() / router.replace() / router.refresh() now publish a deferred router-state promise into the browser's useState slot the moment the programmatic call is made, instead of dispatching the new tree only at commit. BrowserRoot unwraps the slot with use(), so any render during the RSC fetch suspends with transition priority and the previous UI stays visible. useTransition().isPending latches true for the entire navigation window and drops exactly once, at commit.

router.push / replace / refresh also wrap their synchronous entry in React.startTransition, mirroring Next.js's packages/next/src/client/components/app-router-instance.ts.

Why

Before this change, the browser router state was a useReducer that only dispatched at commit time. React had nothing in-flight to wait on, so wrapping a programmatic navigation in useTransition did not latch isPending, and Suspense boundaries inside the destination tree visibly committed their fallbacks during the RSC fetch. Issue #639 describes the user-visible symptom: double-flashing Suspense fallbacks during client navigation triggered by router.push. A keyed list remount in a reporter's movies-ranking app is one concrete reproduction — the provider chip flashed empty because isPending was false while the new server component was still fetching.

Link clicks already worked because they go through a different code path that keeps old UI visible; the gap was specifically programmatic navigations.

Approach

Browser router state: useReducer → useState<AppRouterState | Promise<AppRouterState>> in server/app-browser-entry.ts. BrowserRoot unwraps the slot via use().
Introduce a per-navigation PendingBrowserRouterState (deferred promise + resolver + settled flag). Programmatic navigations call beginPendingBrowserRouterState() synchronously inside React.startTransition, which publishes the promise into the state slot. On successful dispatch, dispatchBrowserTree resolves it to the reducer output. On supersession, error, or an internal RSC redirect, it's settled with the current committed state so the slot recovers.
navigateClientSide gains a programmaticTransition arg, threaded through __VINEXT_RSC_NAVIGATE__'s public signature so only router.push / replace / refresh opt into the pending-promise machinery. Link clicks still go through the direct-setter path.
_appRouter.push / replace / refresh wrap their synchronous entry in React.startTransition, matching Next.js.

Scope boundaries (non-goals):

No change to <Link> navigation. Link clicks already preserved the previous UI via the existing transition path.
No change to server actions. commitSameUrlNavigatePayload passes null for pending state — server actions don't open a user-visible transition.
The internal RSC-redirect path (app-browser-entry.ts around the pathname-mismatch branch) still recurses through __VINEXT_RSC_NAVIGATE__. The outer pending is settled before the recursion, so isPending briefly flashes false across the redirect hop. A full Next.js-style inline redirect loop is filed as a follow-up.

Validation

Added tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts + fixture under tests/fixtures/app-basic/app/nextjs-compat/router-push-pending/: same-route search-param router.push inside useTransition must keep isPending true through the SSR delay and drop to false once the server filter commits.
vp exec playwright test tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts -c tests/e2e/app-router/nextjs-compat/playwright.nextjs-compat.config.ts — passes.
vp exec playwright test tests/e2e/app-router/nextjs-compat/search-params-key.spec.ts -c tests/e2e/app-router/nextjs-compat/playwright.nextjs-compat.config.ts — passes (no regression in the existing search-param navigation spec).
vp check — clean (format, lint, type).
Manually verified against a user-reported reproduction (keyed provider-chip list in movies-ranking) that the mid-navigation flash no longer occurs.

Risks / follow-ups

The internal RSC-redirect recursion at navigateRsc remains. A follow-up issue converts it into an inline loop inside navigateRsc so a single deferred promise can span all redirect hops, the way Next.js's action queue does. Until then, isPending across a same-call internal redirect is not perfectly continuous.
PendingBrowserRouterState settlement is diffused across multiple bailout sites in navigateRsc. Each is idempotent via the settled flag, but a future early return will need to remember to settle. A try / finally wrapper around the body is a reasonable follow-up cleanup.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/868 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 4/22/2026 **Status:** ✅ Merged **Merged:** 4/22/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `fix/programmatic-router-push-pending-transition` --- ### 📝 Commits (2) - [`5a4b984`](https://github.com/cloudflare/vinext/commit/5a4b984ff6c0438e698083179a903eabad1a22e7) fix(app-router): track pending router state for programmatic navigations - [`12ee56d`](https://github.com/cloudflare/vinext/commit/12ee56d238fad2064651b9f73c4b4f6a878cb749) address review feedback on programmatic router pending state ### 📊 Changes **7 files changed** (+290 additions, -37 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/global.d.ts` (+1 -0) 📝 `packages/vinext/src/server/app-browser-entry.ts` (+155 -31) 📝 `packages/vinext/src/shims/navigation.ts` (+21 -5) ➕ `tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts` (+45 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/router-push-pending/page.tsx` (+26 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/router-push-pending/pending-client.tsx` (+28 -0) 📝 `tests/form.test.ts` (+14 -1) </details> ### 📄 Description Closes #639. ## What this changes `router.push()` / `router.replace()` / `router.refresh()` now publish a deferred router-state promise into the browser's `useState` slot the moment the programmatic call is made, instead of dispatching the new tree only at commit. `BrowserRoot` unwraps the slot with `use()`, so any render during the RSC fetch suspends with transition priority and the previous UI stays visible. `useTransition().isPending` latches true for the entire navigation window and drops exactly once, at commit. `router.push` / `replace` / `refresh` also wrap their synchronous entry in `React.startTransition`, mirroring Next.js's `packages/next/src/client/components/app-router-instance.ts`. ## Why Before this change, the browser router state was a `useReducer` that only dispatched at commit time. React had nothing in-flight to wait on, so wrapping a programmatic navigation in `useTransition` did not latch `isPending`, and Suspense boundaries inside the destination tree visibly committed their fallbacks during the RSC fetch. Issue #639 describes the user-visible symptom: double-flashing Suspense fallbacks during client navigation triggered by `router.push`. A keyed list remount in a reporter's `movies-ranking` app is one concrete reproduction — the provider chip flashed empty because `isPending` was false while the new server component was still fetching. Link clicks already worked because they go through a different code path that keeps old UI visible; the gap was specifically programmatic navigations. ## Approach - Browser router state: `useReducer` → `useState<AppRouterState | Promise<AppRouterState>>` in `server/app-browser-entry.ts`. `BrowserRoot` unwraps the slot via `use()`. - Introduce a per-navigation `PendingBrowserRouterState` (deferred promise + resolver + settled flag). Programmatic navigations call `beginPendingBrowserRouterState()` synchronously inside `React.startTransition`, which publishes the promise into the state slot. On successful dispatch, `dispatchBrowserTree` resolves it to the reducer output. On supersession, error, or an internal RSC redirect, it's settled with the current committed state so the slot recovers. - `navigateClientSide` gains a `programmaticTransition` arg, threaded through `__VINEXT_RSC_NAVIGATE__`'s public signature so only `router.push` / `replace` / `refresh` opt into the pending-promise machinery. Link clicks still go through the direct-setter path. - `_appRouter.push` / `replace` / `refresh` wrap their synchronous entry in `React.startTransition`, matching Next.js. Scope boundaries (non-goals): - No change to `<Link>` navigation. Link clicks already preserved the previous UI via the existing transition path. - No change to server actions. `commitSameUrlNavigatePayload` passes `null` for pending state — server actions don't open a user-visible transition. - The internal RSC-redirect path (`app-browser-entry.ts` around the pathname-mismatch branch) still recurses through `__VINEXT_RSC_NAVIGATE__`. The outer pending is settled before the recursion, so `isPending` briefly flashes false across the redirect hop. A full Next.js-style inline redirect loop is filed as a follow-up. ## Validation - Added `tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts` + fixture under `tests/fixtures/app-basic/app/nextjs-compat/router-push-pending/`: same-route search-param `router.push` inside `useTransition` must keep `isPending` true through the SSR delay and drop to false once the server filter commits. - `vp exec playwright test tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts -c tests/e2e/app-router/nextjs-compat/playwright.nextjs-compat.config.ts` — passes. - `vp exec playwright test tests/e2e/app-router/nextjs-compat/search-params-key.spec.ts -c tests/e2e/app-router/nextjs-compat/playwright.nextjs-compat.config.ts` — passes (no regression in the existing search-param navigation spec). - `vp check` — clean (format, lint, type). - Manually verified against a user-reported reproduction (keyed provider-chip list in `movies-ranking`) that the mid-navigation flash no longer occurs. ## Risks / follow-ups - The internal RSC-redirect recursion at `navigateRsc` remains. A follow-up issue converts it into an inline loop inside `navigateRsc` so a single deferred promise can span all redirect hops, the way Next.js's action queue does. Until then, `isPending` across a same-call internal redirect is not perfectly continuous. - `PendingBrowserRouterState` settlement is diffused across multiple bailout sites in `navigateRsc`. Each is idempotent via the `settled` flag, but a future early return will need to remember to settle. A `try / finally` wrapper around the body is a reasonable follow-up cleanup. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>