[PR #876] [CLOSED] fix(app-router): keep isPending true across router.back() and router.forward() #913

New issue

Closed

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

BreizhHardware commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/876
Author: @NathanDrake2406
Created: 4/23/2026
Status: ❌ Closed

Base: main ← Head: fix/router-back-forward-ispending

📝 Commits (10+)

2f6c475 test(app-router): add failing E2E for router.back/forward isPending continuity
e33b851 fix(app-router): keep isPending true across router.back() and router.forward()
f644540 fix(types): declare window.navigation shape used by traversal shim
3db6157 test(app-router): wait for router readiness in E2E
751a541 fix(app-router): gate traversal-pending arm on router readiness
7bfc4c9 fix(app-router): guard traversal pending hook
5881aa3 fix(app-router): heal applyAction race via dual-write
5f46d3d fix(app-router): assign traversal arm hook from mount effect
ef879b2 docs(app-router): clarify traversal degraded mode and transition wrapping
3099335 test(e2e): symmetrize hydration probe and annotate forward-test setup

📊 Changes

10 files changed (+673 additions, -18 deletions)

View changed files

📝 packages/vinext/src/global.d.ts (+17 -0)
📝 packages/vinext/src/server/app-browser-entry.ts (+71 -13)
📝 packages/vinext/src/shims/navigation.ts (+157 -2)
➕ tests/e2e/app-router/nextjs-compat/router-back-forward-pending.spec.ts (+276 -0)
📝 tests/e2e/helpers.ts (+2 -3)
➕ tests/fixtures/app-basic/app/nextjs-compat/router-back-forward-pending/destination/back-client.tsx (+58 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/router-back-forward-pending/destination/page.tsx (+19 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/router-back-forward-pending/destination/step2/page.tsx (+19 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/router-back-forward-pending/nav-client.tsx (+35 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/router-back-forward-pending/page.tsx (+19 -0)

📄 Description

What this changes

router.back() and router.forward() now keep useTransition().isPending true from the synchronous call site until the RSC traversal commits. Matches the behavior shipped for router.push / replace in #868 and for RSC-level redirects in #870.

Why

router.back() and router.forward() call window.history.back/forward() synchronously. The browser queues popstate as a new task, so by the time it fires the user's React.startTransition callback has already exited. No setState is tied to the transition, useTransition().isPending never latches, and any loading UI flashes to its idle state before the new route renders.

Wrapping the popstate handler itself in startTransition does not help, because that creates a transition unrelated to the one the user opened in their component.

Approach

Arm a deferred PendingBrowserRouterState (a Promise published into the router's useState slot) inside the user's transition, then adopt it on the subsequent popstate's navigateRsc.

Three small production changes:

shims/navigation.ts - add runProgrammaticTraversal(direction). It opens the user's startTransition, calls window.__VINEXT_ARM_TRAVERSAL_PENDING__?.(), then history.back/forward(). Gate the arm on window.navigation.canGoBack / canGoForward so a no-op traversal does not leave an unsettled pending.
server/app-browser-entry.ts - register __VINEXT_ARM_TRAVERSAL_PENDING__ at hydration time; it invokes beginPendingBrowserRouterState(), which auto-settles any stale pending, publishes a fresh promise via the setBrowserRouterState setter, and records it as activePendingBrowserRouterState.
navigateRsc adoption branch - when programmaticTransition is false and an active pending exists, adopt it so the commit resolves the promise the user's transition is suspended on. User-initiated popstates (keyboard, gesture, address bar) find no active pending and fall through unchanged.

No registry, no id map, no safety timer. The Navigation API's synchronous availability check handles the no-op case; on older browsers without the Navigation API, arming is skipped and isPending preservation degrades to current Next.js behavior (no hang).

Non-goals: Pages Router is unaffected; the arm hook is defined only for App Router pages.

Validation

New E2E: tests/e2e/app-router/nextjs-compat/router-back-forward-pending.spec.ts, four specs using the MutationObserver idle-flash pattern from #870:
- router.back() keeps isPending true until page A commits (reproduces the current bug pre-fix with log ["b:idle","b:pending","b:idle","__removed__"])
- router.forward() keeps isPending true until page B commits
- rapid router.back(); router.back(); stays pending through the second hop
- router.back() then router.forward() across separate clicks round-trips cleanly
All 66 nextjs-compat E2E tests pass, including the existing push/redirect pending specs from #868 and #870.
vp check clean (format, lint, type-aware).

Risks / follow-ups

Browsers without the Navigation API: isPending preservation is skipped for traversals. Same behavior as Next.js today on those browsers, no hang.
A user-initiated popstate interleaved with a programmatic traversal in flight will adopt the active pending. End state is always a real committed tree; only the "wrong URL under the programmatic caller's pending" edge case differs from perfect classification. Stamping history.state with a programmatic marker would close this; deliberately out of scope.

_{🔄 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/876 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 4/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/router-back-forward-ispending` --- ### 📝 Commits (10+) - [`2f6c475`](https://github.com/cloudflare/vinext/commit/2f6c475380aac633771f82dafaeb6515428e88fe) test(app-router): add failing E2E for router.back/forward isPending continuity - [`e33b851`](https://github.com/cloudflare/vinext/commit/e33b851dfd9f6558e5d4e8243f49d3732442fd42) fix(app-router): keep isPending true across router.back() and router.forward() - [`f644540`](https://github.com/cloudflare/vinext/commit/f6445408992f94eda8d18037ae411c94d674105c) fix(types): declare window.navigation shape used by traversal shim - [`3db6157`](https://github.com/cloudflare/vinext/commit/3db6157ec0a16bd688d70e957c55d1ad95552c81) test(app-router): wait for router readiness in E2E - [`751a541`](https://github.com/cloudflare/vinext/commit/751a5411224c6d34d3cc0e46d452d714814c3f82) fix(app-router): gate traversal-pending arm on router readiness - [`7bfc4c9`](https://github.com/cloudflare/vinext/commit/7bfc4c918b03ad442dbee70f36dcaab120eea5b2) fix(app-router): guard traversal pending hook - [`5881aa3`](https://github.com/cloudflare/vinext/commit/5881aa3c8ac4272e1f7b1d94e7e4fce62a888de7) fix(app-router): heal applyAction race via dual-write - [`5f46d3d`](https://github.com/cloudflare/vinext/commit/5f46d3da24fcf56d5eccdd5d3d548e6f7b46d89e) fix(app-router): assign traversal arm hook from mount effect - [`ef879b2`](https://github.com/cloudflare/vinext/commit/ef879b2dad5ecbba2ecbbcfff0a1c9acf5abb05f) docs(app-router): clarify traversal degraded mode and transition wrapping - [`3099335`](https://github.com/cloudflare/vinext/commit/30993358c302b101bb1b5b239054b71f5efe1fd6) test(e2e): symmetrize hydration probe and annotate forward-test setup ### 📊 Changes **10 files changed** (+673 additions, -18 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/global.d.ts` (+17 -0) 📝 `packages/vinext/src/server/app-browser-entry.ts` (+71 -13) 📝 `packages/vinext/src/shims/navigation.ts` (+157 -2) ➕ `tests/e2e/app-router/nextjs-compat/router-back-forward-pending.spec.ts` (+276 -0) 📝 `tests/e2e/helpers.ts` (+2 -3) ➕ `tests/fixtures/app-basic/app/nextjs-compat/router-back-forward-pending/destination/back-client.tsx` (+58 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/router-back-forward-pending/destination/page.tsx` (+19 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/router-back-forward-pending/destination/step2/page.tsx` (+19 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/router-back-forward-pending/nav-client.tsx` (+35 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/router-back-forward-pending/page.tsx` (+19 -0) </details> ### 📄 Description ## What this changes `router.back()` and `router.forward()` now keep `useTransition().isPending` true from the synchronous call site until the RSC traversal commits. Matches the behavior shipped for `router.push` / `replace` in #868 and for RSC-level redirects in #870. ## Why `router.back()` and `router.forward()` call `window.history.back/forward()` synchronously. The browser queues `popstate` as a new task, so by the time it fires the user's `React.startTransition` callback has already exited. No setState is tied to the transition, `useTransition().isPending` never latches, and any loading UI flashes to its idle state before the new route renders. Wrapping the popstate handler itself in `startTransition` does not help, because that creates a transition unrelated to the one the user opened in their component. ## Approach Arm a deferred `PendingBrowserRouterState` (a Promise published into the router's `useState` slot) inside the user's transition, then adopt it on the subsequent popstate's `navigateRsc`. Three small production changes: 1. **`shims/navigation.ts`** - add `runProgrammaticTraversal(direction)`. It opens the user's `startTransition`, calls `window.__VINEXT_ARM_TRAVERSAL_PENDING__?.()`, then `history.back/forward()`. Gate the arm on `window.navigation.canGoBack` / `canGoForward` so a no-op traversal does not leave an unsettled pending. 2. **`server/app-browser-entry.ts`** - register `__VINEXT_ARM_TRAVERSAL_PENDING__` at hydration time; it invokes `beginPendingBrowserRouterState()`, which auto-settles any stale pending, publishes a fresh promise via the `setBrowserRouterState` setter, and records it as `activePendingBrowserRouterState`. 3. **`navigateRsc` adoption branch** - when `programmaticTransition` is false and an active pending exists, adopt it so the commit resolves the promise the user's transition is suspended on. User-initiated popstates (keyboard, gesture, address bar) find no active pending and fall through unchanged. No registry, no id map, no safety timer. The Navigation API's synchronous availability check handles the no-op case; on older browsers without the Navigation API, arming is skipped and `isPending` preservation degrades to current Next.js behavior (no hang). Non-goals: Pages Router is unaffected; the arm hook is defined only for App Router pages. ## Validation - New E2E: `tests/e2e/app-router/nextjs-compat/router-back-forward-pending.spec.ts`, four specs using the MutationObserver idle-flash pattern from #870: - `router.back()` keeps `isPending` true until page A commits (reproduces the current bug pre-fix with log `["b:idle","b:pending","b:idle","__removed__"]`) - `router.forward()` keeps `isPending` true until page B commits - rapid `router.back(); router.back();` stays pending through the second hop - `router.back()` then `router.forward()` across separate clicks round-trips cleanly - All 66 nextjs-compat E2E tests pass, including the existing push/redirect pending specs from #868 and #870. - `vp check` clean (format, lint, type-aware). ## Risks / follow-ups - Browsers without the Navigation API: `isPending` preservation is skipped for traversals. Same behavior as Next.js today on those browsers, no hang. - A user-initiated popstate interleaved with a programmatic traversal in flight will adopt the active pending. End state is always a real committed tree; only the "wrong URL under the programmatic caller's pending" edge case differs from perfect classification. Stamping `history.state` with a programmatic marker would close this; deliberately out of scope. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>