[GH-ISSUE #869] App Router: convert internal RSC-redirect recursion into an inline loop so a single pending transition can span all hops #190

New issue

Closed

opened 2026-05-06 12:38:00 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 12:38:00 +02:00

Owner

Originally created by @NathanDrake2406 on GitHub (Apr 22, 2026).
Original GitHub issue: https://github.com/cloudflare/vinext/issues/869

Follow-up to #868.

Context

#868 introduced a per-navigation PendingBrowserRouterState (deferred promise + resolver) so programmatic router.push / replace / refresh can keep useTransition().isPending true for the full RSC-fetch window. The deferred promise is published into the browser's useState slot synchronously at navigation start, unwrapped via use() in BrowserRoot, and resolved at commit.

One edge case was left as a known limitation: the internal RSC-redirect path in packages/vinext/src/server/app-browser-entry.ts. When the RSC response's pathname differs from the requested pathname, navigateRsc recursively calls window.__VINEXT_RSC_NAVIGATE__(destinationPath, redirectDepth + 1, ..., programmaticTransition=false). The recursive call bumps activeNavigationId, which marks the outer call as superseded by its own redirect. The outer pending is settled with the current committed state just before the recursion, so useTransition().isPending flashes false mid-redirect instead of staying true for the full user-visible navigation.

Why this is awkward to fix in place

The recursive call is vinext's public navigation entry point doing double duty as the redirect follower. It:

increments activeNavigationId, which is what supersession checks use to bail out stale navigations — so the outer call cannot keep running past the recursion
creates a fresh request context, scroll/history effect stack, and pendingRouterState = null in the recursive frame
cannot take the outer frame's pendingRouterState because programmaticTransition=false short-circuits beginPendingBrowserRouterState, and threading it through window.__VINEXT_RSC_NAVIGATE__ would pollute a public API surface

The outer frame's pending is therefore orphaned unless we settle it before handing control off, which is what #868 does as a bandage.

What Next.js does

Next.js's packages/next/src/client/components/app-router-instance.ts handles navigations through a queue-based reducer:

dispatchAction creates one deferred promise per action and attaches its resolver to an ActionQueueNode.
setState(deferredPromise) publishes the promise into React's state slot once, inside startTransition.
The reducer runs to completion — including any internal redirects — and handleResult calls action.resolve(finalState) exactly once at the end.
Supersession is marking pending.discarded = true, not recursive self-invocation.

Because the entry point is never re-entered for a same-action redirect, there is nothing to orphan. A single deferred promise spans every redirect hop in that navigation.

Proposed direction

Convert navigateRsc so the redirect path becomes an inline loop instead of a recursive __VINEXT_RSC_NAVIGATE__ call:

Extract the fetch / stale-check / dispatch block into a loop keyed by (url, redirectDepth).
On an RSC response with a differing pathname, update local url / rscUrl / history state in place, bump redirectDepth, and continue.
Keep one navId and one pendingRouterState for the entire loop. Resolve the pending promise only at the final commit.
Leave window.__VINEXT_RSC_NAVIGATE__'s public signature unchanged — redirects become an implementation detail of navigateRsc, not a recursion through the public API.

Related work that needs to move with the redirect loop:

supersession checks (navId !== activeNavigationId) currently fire between awaits per-frame; in the loop version they still fire per-hop, but a single hop's bailout must settle the shared pending exactly once
redirectDepth > 10 check becomes a loop counter with the same threshold
replaceHistoryStateWithoutNotify in the current redirect block runs before the recursion — in the loop version it runs at the top of each iteration, before the next fetch
scroll and commit effects keep running once at the end, not once per redirect hop (arguably a latent improvement anyway)

Additional cleanup that naturally falls out

PendingBrowserRouterState settlement is currently spread across ~7 bailout sites inside navigateRsc. Each is idempotent via the settled flag, but a future early return will need to remember to settle. Wrapping the loop body in try { ... } finally { settlePendingBrowserRouterState(pendingRouterState); } makes ownership explicit and collapses the scattered calls to one.

Validation plan

Extend tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts to cover a programmatic push whose RSC response redirects (e.g. a route that returns a redirect() in its server component). Assert isPending stays true continuously until the final destination commits.
Existing search-param / intercepted-route / parallel-route E2E suites must continue to pass unchanged.

References

#639 (the original Suspense-fallback flash bug, closed by #868)
#868 (this PR, where the redirect limitation is documented inline at the recursion site)
.nextjs-ref/packages/next/src/client/components/app-router-instance.ts — Next.js's queue + reducer pattern

Originally created by @NathanDrake2406 on GitHub (Apr 22, 2026). Original GitHub issue: https://github.com/cloudflare/vinext/issues/869 Follow-up to #868. ## Context #868 introduced a per-navigation `PendingBrowserRouterState` (deferred promise + resolver) so programmatic `router.push` / `replace` / `refresh` can keep `useTransition().isPending` true for the full RSC-fetch window. The deferred promise is published into the browser's `useState` slot synchronously at navigation start, unwrapped via `use()` in `BrowserRoot`, and resolved at commit. One edge case was left as a known limitation: the internal RSC-redirect path in `packages/vinext/src/server/app-browser-entry.ts`. When the RSC response's pathname differs from the requested pathname, `navigateRsc` recursively calls `window.__VINEXT_RSC_NAVIGATE__(destinationPath, redirectDepth + 1, ..., programmaticTransition=false)`. The recursive call bumps `activeNavigationId`, which marks the outer call as superseded by its own redirect. The outer pending is settled with the current committed state just before the recursion, so `useTransition().isPending` flashes false mid-redirect instead of staying true for the full user-visible navigation. ## Why this is awkward to fix in place The recursive call is vinext's public navigation entry point doing double duty as the redirect follower. It: - increments `activeNavigationId`, which is what supersession checks use to bail out stale navigations — so the outer call cannot keep running past the recursion - creates a fresh request context, scroll/history effect stack, and `pendingRouterState = null` in the recursive frame - cannot take the outer frame's `pendingRouterState` because `programmaticTransition=false` short-circuits `beginPendingBrowserRouterState`, and threading it through `window.__VINEXT_RSC_NAVIGATE__` would pollute a public API surface The outer frame's pending is therefore orphaned unless we settle it before handing control off, which is what #868 does as a bandage. ## What Next.js does Next.js's `packages/next/src/client/components/app-router-instance.ts` handles navigations through a queue-based reducer: - `dispatchAction` creates **one** deferred promise per action and attaches its resolver to an `ActionQueueNode`. - `setState(deferredPromise)` publishes the promise into React's state slot once, inside `startTransition`. - The reducer runs to completion — including any internal redirects — and `handleResult` calls `action.resolve(finalState)` exactly once at the end. - Supersession is marking `pending.discarded = true`, not recursive self-invocation. Because the entry point is never re-entered for a same-action redirect, there is nothing to orphan. A single deferred promise spans every redirect hop in that navigation. ## Proposed direction Convert `navigateRsc` so the redirect path becomes an **inline loop** instead of a recursive `__VINEXT_RSC_NAVIGATE__` call: 1. Extract the fetch / stale-check / dispatch block into a loop keyed by `(url, redirectDepth)`. 2. On an RSC response with a differing pathname, update local `url` / `rscUrl` / history state in place, bump `redirectDepth`, and `continue`. 3. Keep one `navId` and one `pendingRouterState` for the entire loop. Resolve the pending promise only at the final commit. 4. Leave `window.__VINEXT_RSC_NAVIGATE__`'s public signature unchanged — redirects become an implementation detail of `navigateRsc`, not a recursion through the public API. Related work that needs to move with the redirect loop: - supersession checks (`navId !== activeNavigationId`) currently fire between awaits per-frame; in the loop version they still fire per-hop, but a single hop's bailout must settle the shared pending exactly once - `redirectDepth > 10` check becomes a loop counter with the same threshold - `replaceHistoryStateWithoutNotify` in the current redirect block runs before the recursion — in the loop version it runs at the top of each iteration, before the next fetch - scroll and commit effects keep running once at the end, not once per redirect hop (arguably a latent improvement anyway) ## Additional cleanup that naturally falls out `PendingBrowserRouterState` settlement is currently spread across ~7 bailout sites inside `navigateRsc`. Each is idempotent via the `settled` flag, but a future early return will need to remember to settle. Wrapping the loop body in `try { ... } finally { settlePendingBrowserRouterState(pendingRouterState); }` makes ownership explicit and collapses the scattered calls to one. ## Validation plan - Extend `tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts` to cover a programmatic push whose RSC response redirects (e.g. a route that returns a `redirect()` in its server component). Assert `isPending` stays true continuously until the final destination commits. - Existing search-param / intercepted-route / parallel-route E2E suites must continue to pass unchanged. ## References - #639 (the original Suspense-fallback flash bug, closed by #868) - #868 (this PR, where the redirect limitation is documented inline at the recursion site) - `.nextjs-ref/packages/next/src/client/components/app-router-instance.ts` — Next.js's queue + reducer pattern