[PR #643] [CLOSED] feat: two-phase navigation commit with visited response cache #740

Closed
opened 2026-05-06 13:09:53 +02:00 by BreizhHardware · 0 comments

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/643
Author: @NathanDrake2406
Created: 3/22/2026
Status: Closed

Base: mainHead: fix/app-router-navigation-cache-replay


📝 Commits (10+)

  • a83634d Fix app router navigation replay and refresh caching
  • 5729d0f fix: address review issues in navigation cache/replay
  • 3303a3f test: add navigation regression e2e tests and fixtures
  • 4f313fc fix: update form tests for navigateClientSide calling convention
  • 85df5b7 fix: revert behavioral changes to match Codex's original intent
  • c864945 fix: buffer RSC responses, fix shallow routing, server action redirects, and Firefox nav hang
  • 4b57ce2 refactor: remove animation suppression — it needs an architectural fix
  • 046087c refactor: replace useEffect with useLayoutEffect in NavigationCommitSignal
  • 072087e fix: skip startTransition for cross-route navigations (Firefox hang)
  • 3afabab fix: address code review findings from elegance pass

📊 Changes

19 files changed (+2040 additions, -304 deletions)

View changed files

📝 packages/vinext/src/global.d.ts (+12 -4)
📝 packages/vinext/src/server/app-browser-entry.ts (+453 -48)
📝 packages/vinext/src/shims/form.tsx (+5 -8)
📝 packages/vinext/src/shims/link.tsx (+19 -97)
📝 packages/vinext/src/shims/navigation.ts (+484 -123)
tests/e2e/app-router/navigation-regressions.spec.ts (+686 -0)
📝 tests/e2e/app-router/nextjs-compat/actions-revalidate.spec.ts (+36 -1)
tests/fixtures/app-basic/app/nav-flash/link-sync/FilterLinks.tsx (+36 -0)
tests/fixtures/app-basic/app/nav-flash/link-sync/page.tsx (+56 -0)
tests/fixtures/app-basic/app/nav-flash/list/page.tsx (+30 -0)
tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/FilterControls.tsx (+28 -0)
tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/page.tsx (+39 -0)
tests/fixtures/app-basic/app/nav-flash/provider/[id]/page.tsx (+15 -0)
tests/fixtures/app-basic/app/nav-flash/query-sync/FilterControls.tsx (+28 -0)
tests/fixtures/app-basic/app/nav-flash/query-sync/page.tsx (+56 -0)
📝 tests/fixtures/app-basic/app/nextjs-compat/action-revalidate/page.tsx (+4 -0)
📝 tests/fixtures/app-basic/app/page.tsx (+9 -0)
📝 tests/form.test.ts (+23 -14)
📝 tests/prefetch-cache.test.ts (+21 -9)

📄 Description

Ref #639

Summary

Redesigns the App Router client-side navigation system to eliminate URL/hook desync flashes and enable instant back/forward navigation.

Core architecture changes

  • Two-phase navigation commit - URL and history updates are deferred to useLayoutEffect after React commits the new tree, preventing usePathname()/useSearchParams()/useParams() from returning stale values during transitions
  • Navigation render context - A React context (ClientNavigationRenderSnapshot) provides the pending URL and params to hooks during the render phase, before history is committed. This means components see consistent navigation state throughout the entire render cycle
  • RSC response buffering - Cold navigation responses are fully buffered via snapshotRscResponse before passing to createFromFetch, ensuring the flight parser processes all rows in a single synchronous pass (prevents partial tree commits where list content updates before heading hooks catch up)
  • Visited response cache - RSC responses are snapshotted to ArrayBuffer after each navigation and cached (5 min TTL for forward nav, no expiry for back/forward, 50 entry cap). Back/forward navigations replay from cache instantly, matching Next.js BFCache behavior. router.refresh() bypasses the cache
  • Navigation snapshot activation counter - Hooks only prefer the render snapshot context during active transitions (tracked via ref-counted _navigationSnapshotActiveCount). After commit, hooks fall through to useSyncExternalStore so user pushState/replaceState calls are immediately reflected
  • Non-blocking prefetch consumption - consumePrefetchResponse only returns settled responses synchronously, matching Next.js's segment cache behavior. Pending prefetches never block navigation (fixes Firefox nav bar hang)
  • History notification suppression - pushHistoryStateWithoutNotify/replaceHistoryStateWithoutNotify wrap the original (unpatched) history methods to update the URL without triggering useSyncExternalStore subscribers prematurely
  • Server action redirect fix - Immediate history.pushState before fire-and-forget __VINEXT_RSC_NAVIGATE__ with .catch cleanup, avoiding deadlock between the form's outer useTransition and renderNavigationPayload's inner startTransition

Unified navigation surface

  • navigateClientSide() is now the single entry point for all client-side navigation (Link, Form, router.push/replace)
  • Link and Form no longer manipulate history directly - they delegate to navigateClientSide which coordinates with __VINEXT_RSC_NAVIGATE__
  • Hash-only changes, scroll restoration, and animation suppression are handled centrally in navigateClientSide

Next.js parity notes

This brings vinext closer to Next.js for common navigation cases (forward nav, back/forward, prefetch reuse, shallow routing):

  • Page-level caching vs Next.js's segment-level caching - vinext caches and replays the entire RSC response per URL. Simpler but coarser; a back/forward replays the whole page tree, not just changed segments
  • Cache TTLs match Next.js - Forward navigation uses 5 min TTL (matches STATIC_STALETIME_MS). Back/forward bypasses TTL entirely (matches BFCache behavior where entries are served regardless of age). Eviction is LRU-only (50 entries)
  • Two-phase commit is a straightforward stage -> render -> commit-in-useLayoutEffect flow, vs Next.js's 2000+ line router reducer state machine
  • Snapshot/restore via ArrayBuffer is minimal compared to Next.js's multi-tier Flight cache

Known limitation: CSS animation replay on navigation

vinext replaces the entire page tree on navigation, causing all CSS animations (fade-in, slide-in) to replay on freshly-mounted elements. Next.js avoids this architecturally via segment-level caching - only changed segments re-mount, so unchanged layouts/templates keep their DOM elements. This is a deeper architectural issue tracked separately from this PR.

Test plan

  • pnpm test tests/shims.test.ts tests/link.test.ts tests/app-router.test.ts - all pass (1074 tests)
  • pnpm run test:e2e - all pass (501 tests, 0 failures)
  • pnpm run check - zero lint, type, or formatting errors
  • Shallow routing: pushState/replaceState updates reflected in usePathname/useSearchParams
  • Server action redirect() navigates correctly
  • Cold Link navigation keeps URL state aligned with committed tree
  • Animation suppression on mount-time row animations

🔄 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/643 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 3/22/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/app-router-navigation-cache-replay` --- ### 📝 Commits (10+) - [`a83634d`](https://github.com/cloudflare/vinext/commit/a83634d0aa6144078679f5ef55fb6f46f55142ac) Fix app router navigation replay and refresh caching - [`5729d0f`](https://github.com/cloudflare/vinext/commit/5729d0fd1ae22182c88d5486c97d5a76ab14b1b0) fix: address review issues in navigation cache/replay - [`3303a3f`](https://github.com/cloudflare/vinext/commit/3303a3fa2a7621d43e7740f6c1baba09b8494121) test: add navigation regression e2e tests and fixtures - [`4f313fc`](https://github.com/cloudflare/vinext/commit/4f313fc1f73db95cc8a0baf8559be9aa0ebbcd98) fix: update form tests for navigateClientSide calling convention - [`85df5b7`](https://github.com/cloudflare/vinext/commit/85df5b789814fcb46f7fbd52f27e0ac6f4c3d0c6) fix: revert behavioral changes to match Codex's original intent - [`c864945`](https://github.com/cloudflare/vinext/commit/c86494514aa77ea51696a792b4792ea21e8af0c3) fix: buffer RSC responses, fix shallow routing, server action redirects, and Firefox nav hang - [`4b57ce2`](https://github.com/cloudflare/vinext/commit/4b57ce20c31b29c1df004ac1d6968723ef55c460) refactor: remove animation suppression — it needs an architectural fix - [`046087c`](https://github.com/cloudflare/vinext/commit/046087cbd5603416d54246b096a6e6b3f898ed74) refactor: replace useEffect with useLayoutEffect in NavigationCommitSignal - [`072087e`](https://github.com/cloudflare/vinext/commit/072087e9c984626e744138160f177d36e97ff3c6) fix: skip startTransition for cross-route navigations (Firefox hang) - [`3afabab`](https://github.com/cloudflare/vinext/commit/3afababb545dd56df4ec417af66753bd6d2381e4) fix: address code review findings from elegance pass ### 📊 Changes **19 files changed** (+2040 additions, -304 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/global.d.ts` (+12 -4) 📝 `packages/vinext/src/server/app-browser-entry.ts` (+453 -48) 📝 `packages/vinext/src/shims/form.tsx` (+5 -8) 📝 `packages/vinext/src/shims/link.tsx` (+19 -97) 📝 `packages/vinext/src/shims/navigation.ts` (+484 -123) ➕ `tests/e2e/app-router/navigation-regressions.spec.ts` (+686 -0) 📝 `tests/e2e/app-router/nextjs-compat/actions-revalidate.spec.ts` (+36 -1) ➕ `tests/fixtures/app-basic/app/nav-flash/link-sync/FilterLinks.tsx` (+36 -0) ➕ `tests/fixtures/app-basic/app/nav-flash/link-sync/page.tsx` (+56 -0) ➕ `tests/fixtures/app-basic/app/nav-flash/list/page.tsx` (+30 -0) ➕ `tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/FilterControls.tsx` (+28 -0) ➕ `tests/fixtures/app-basic/app/nav-flash/param-sync/[filter]/page.tsx` (+39 -0) ➕ `tests/fixtures/app-basic/app/nav-flash/provider/[id]/page.tsx` (+15 -0) ➕ `tests/fixtures/app-basic/app/nav-flash/query-sync/FilterControls.tsx` (+28 -0) ➕ `tests/fixtures/app-basic/app/nav-flash/query-sync/page.tsx` (+56 -0) 📝 `tests/fixtures/app-basic/app/nextjs-compat/action-revalidate/page.tsx` (+4 -0) 📝 `tests/fixtures/app-basic/app/page.tsx` (+9 -0) 📝 `tests/form.test.ts` (+23 -14) 📝 `tests/prefetch-cache.test.ts` (+21 -9) </details> ### 📄 Description Ref #639 ## Summary Redesigns the App Router client-side navigation system to eliminate URL/hook desync flashes and enable instant back/forward navigation. ### Core architecture changes - **Two-phase navigation commit** - URL and history updates are deferred to `useLayoutEffect` after React commits the new tree, preventing `usePathname()`/`useSearchParams()`/`useParams()` from returning stale values during transitions - **Navigation render context** - A React context (`ClientNavigationRenderSnapshot`) provides the pending URL and params to hooks during the render phase, before history is committed. This means components see consistent navigation state throughout the entire render cycle - **RSC response buffering** - Cold navigation responses are fully buffered via `snapshotRscResponse` before passing to `createFromFetch`, ensuring the flight parser processes all rows in a single synchronous pass (prevents partial tree commits where list content updates before heading hooks catch up) - **Visited response cache** - RSC responses are snapshotted to `ArrayBuffer` after each navigation and cached (5 min TTL for forward nav, no expiry for back/forward, 50 entry cap). Back/forward navigations replay from cache instantly, matching Next.js BFCache behavior. `router.refresh()` bypasses the cache - **Navigation snapshot activation counter** - Hooks only prefer the render snapshot context during active transitions (tracked via ref-counted `_navigationSnapshotActiveCount`). After commit, hooks fall through to `useSyncExternalStore` so user `pushState`/`replaceState` calls are immediately reflected - **Non-blocking prefetch consumption** - `consumePrefetchResponse` only returns settled responses synchronously, matching Next.js's segment cache behavior. Pending prefetches never block navigation (fixes Firefox nav bar hang) - **History notification suppression** - `pushHistoryStateWithoutNotify`/`replaceHistoryStateWithoutNotify` wrap the original (unpatched) history methods to update the URL without triggering `useSyncExternalStore` subscribers prematurely - **Server action redirect fix** - Immediate `history.pushState` before fire-and-forget `__VINEXT_RSC_NAVIGATE__` with `.catch` cleanup, avoiding deadlock between the form's outer `useTransition` and `renderNavigationPayload`'s inner `startTransition` ### Unified navigation surface - `navigateClientSide()` is now the single entry point for all client-side navigation (Link, Form, `router.push/replace`) - Link and Form no longer manipulate history directly - they delegate to `navigateClientSide` which coordinates with `__VINEXT_RSC_NAVIGATE__` - Hash-only changes, scroll restoration, and animation suppression are handled centrally in `navigateClientSide` ### Next.js parity notes This brings vinext closer to Next.js for common navigation cases (forward nav, back/forward, prefetch reuse, shallow routing): - **Page-level caching** vs Next.js's segment-level caching - vinext caches and replays the entire RSC response per URL. Simpler but coarser; a back/forward replays the whole page tree, not just changed segments - **Cache TTLs match Next.js** - Forward navigation uses 5 min TTL (matches `STATIC_STALETIME_MS`). Back/forward bypasses TTL entirely (matches BFCache behavior where entries are served regardless of age). Eviction is LRU-only (50 entries) - **Two-phase commit** is a straightforward stage -> render -> commit-in-`useLayoutEffect` flow, vs Next.js's 2000+ line router reducer state machine - **Snapshot/restore via `ArrayBuffer`** is minimal compared to Next.js's multi-tier Flight cache ### Known limitation: CSS animation replay on navigation vinext replaces the entire page tree on navigation, causing all CSS animations (fade-in, slide-in) to replay on freshly-mounted elements. Next.js avoids this architecturally via segment-level caching - only changed segments re-mount, so unchanged layouts/templates keep their DOM elements. This is a deeper architectural issue tracked separately from this PR. ## Test plan - [x] `pnpm test tests/shims.test.ts tests/link.test.ts tests/app-router.test.ts` - all pass (1074 tests) - [x] `pnpm run test:e2e` - all pass (501 tests, 0 failures) - [x] `pnpm run check` - zero lint, type, or formatting errors - [x] Shallow routing: `pushState`/`replaceState` updates reflected in `usePathname`/`useSearchParams` - [x] Server action `redirect()` navigates correctly - [x] Cold Link navigation keeps URL state aligned with committed tree - [x] Animation suppression on mount-time row animations --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
BreizhHardware 2026-05-06 13:09:53 +02:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/vinext#740
No description provided.