feat: two-phase navigation commit with visited response cache#643
Open
NathanDrake2406 wants to merge 10 commits intocloudflare:mainfrom
Open
feat: two-phase navigation commit with visited response cache#643NathanDrake2406 wants to merge 10 commits intocloudflare:mainfrom
NathanDrake2406 wants to merge 10 commits intocloudflare:mainfrom
Conversation
- Fix broken indentation and missing fallback in server action redirect handler
- Replace useEffect with useLayoutEffect in NavigationCommitSignal
- Eliminate `as` type assertions via createFromFetch<ReactNode> generics
- Replace querySelectorAll("*") + getComputedStyle with document.getAnimations()
- Cache getSearchParamsSnapshot fallback to prevent potential infinite re-renders
- Add module-level fallback for setClientParams in test/SSR environments
- Fix History.prototype reference crash in test environments
- Update global.d.ts prefetch cache type to match PrefetchCacheEntry
- Update prefetch-cache tests for async storePrefetchResponse and CachedRscResponse
Add Playwright tests for navigation flash regressions covering link-sync, param-sync, and query-sync scenarios with corresponding test fixture pages.
commit: |
Form component now delegates to navigateClientSide instead of calling pushState + navigate separately. Update test assertions to expect the new 4-arg __VINEXT_RSC_NAVIGATE__ signature and add missing window mock properties (pathname, search, hash, scrollX/Y).
NathanDrake2406
changed the title
Restore useEffect in NavigationCommitSignal, restore querySelectorAll-based animation suppression, and restore separate effect hooks.
…ts, and Firefox nav hang - Buffer full RSC response before createFromFetch to prevent flight parser microtask interleaving that causes partial tree commits on cold navigations - Await createFromFetch to fully resolve the tree before rendering - Add navigation snapshot activation counter so hooks only prefer the render snapshot context during active transitions, fixing pushState/ replaceState reactivity (shallow routing) - Restore immediate history.pushState for server action redirects and use fire-and-forget navigate to avoid useTransition/startTransition deadlock - Always call commitClientNavigationState after navigation commit - Don't block navigation on pending prefetch responses, matching Next.js segment cache behavior (fixes Firefox nav bar hang) - Always run animation suppression for all navigations, not just cold fetches Ref cloudflare#639
suppressFreshNavigationAnimations only caught animations with fill-mode: both/backwards (opacity-based check). Real-world CSS uses the default fill-mode (none), making the function a no-op. Next.js has zero animation suppression code — they avoid the problem via segment-level caching that only re-mounts changed segments. Remove the function, its composition helper, and E2E assertions that tested suppression behavior. The animation replay issue is tracked as a separate architectural concern (segment-level caching).
…ignal Consolidate the two layout/effect hooks into one useLayoutEffect. The requestAnimationFrame fires after paint regardless of where it's scheduled from, so useEffect was unnecessary.
React's startTransition hangs indefinitely in Firefox when replacing the entire component tree (cross-route navigation). The transition never commits, leaving the old page visible with no way to recover. Use startTransition only for same-route navigations (searchParam changes) where it keeps the old UI visible during loading. For cross-route navigations (different pathname), use synchronous state updates so the new page renders immediately. Also removes debug logging from the investigation.
- Increment nextNavigationRenderId for server action renders to avoid stale renderId collisions with navigation commits - Deactivate snapshot counter on navigation failure to prevent hooks from permanently returning stale values - Remove navigateClientSide wrapper (was 1:1 pass-through of navigateImpl) - Remove dead fallback URL update block in navigateClientSide that could never fire (createNavigationCommitEffect always pushes history) - Rename animation test to match what it actually asserts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
useLayoutEffectafter React commits the new tree, preventingusePathname()/useSearchParams()/useParams()from returning stale values during transitionsClientNavigationRenderSnapshot) 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 cyclesnapshotRscResponsebefore passing tocreateFromFetch, 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)ArrayBufferafter each navigation and cached (30s TTL, 50 entry cap). Back/forward navigations replay from cache instantly.router.refresh()bypasses the cache_navigationSnapshotActiveCount). After commit, hooks fall through touseSyncExternalStoreso userpushState/replaceStatecalls are immediately reflectedconsumePrefetchResponseonly returns settled responses synchronously, matching Next.js's segment cache behavior. Pending prefetches never block navigation (fixes Firefox nav bar hang)pushHistoryStateWithoutNotify/replaceHistoryStateWithoutNotifywrap the original (unpatched) history methods to update the URL without triggeringuseSyncExternalStoresubscribers prematurelyhistory.pushStatebefore fire-and-forget__VINEXT_RSC_NAVIGATE__with.catchcleanup, avoiding deadlock between the form's outeruseTransitionandrenderNavigationPayload's innerstartTransitionUnified navigation surface
navigateClientSide()is now the single entry point for all client-side navigation (Link, Form,router.push/replace)navigateClientSidewhich coordinates with__VINEXT_RSC_NAVIGATE__navigateImplNext.js parity notes
This brings vinext closer to Next.js for common navigation cases (forward nav, back/forward, prefetch reuse, shallow routing):
useLayoutEffectflow, vs Next.js's 2000+ line router reducer state machineArrayBufferis minimal compared to Next.js's multi-tier Flight cacheKnown 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 errorspushState/replaceStateupdates reflected inusePathname/useSearchParamsredirect()navigates correctly