Race condition in isSameRoute classification causes janky transitions during rapid navigation

[Divkix]

Problem

During rapid successive navigations (e.g., user clicks A→B→C quickly), the isSameRoute check in app-browser-entry.ts compares against stale window.location.pathname because URL commits are deferred via useLayoutEffect.

Current Behavior (lines 619-622)

const isSameRoute =
  stripBasePath(url.pathname, __basePath) ===
  stripBasePath(window.location.pathname, __basePath);

Race condition window:

1. Navigation 1 starts (A→B):

   • renderNavigationPayload() called with isSameRoute=false (correctly: B ≠ A)
   • navigationCommitEffect queues URL update via pushHistoryStateWithoutNotify()
   • URL hasn't updated yet in window.location

2. Navigation 2 starts (B→C) before Navigation 1 commits:

   • isSameRoute compares url.pathname (C) with window.location.pathname (still A)
   • Result: C === A is false → classified as cross-route → synchronous updates
   • Should be: C === B comparison to determine if B→C is same-route

3. Navigation 1 eventually commits:

   • commitClientNavigationState() calls syncCommittedUrlStateFromLocation()
   • state.cachedPathname updates to B
   • But isSameRoute was already evaluated incorrectly

Impact

Scenario	Misclassification Result
Same-route (search param change) during cross-route transition	Uses sync updates instead of smooth startTransition
Cross-route during another pending navigation	May incorrectly use startTransition (rare but possible)

User-facing: Janky, non-smooth transitions when rapidly clicking between routes or changing filters during page loads.

Code Reference

This was acknowledged as a known limitation in PR #690 with this comment (lines 621-625):

// NB: During rapid navigations, window.location.pathname may not reflect
// the previous navigation's URL yet (URL commit is deferred). This could
// cause misclassification (synchronous instead of startTransition or vice
// versa), resulting in slightly less smooth transitions but correct behavior.

This issue tracks implementing a proper fix rather than accepting the workaround.

Root Cause

The ClientNavigationState tracks pending params but not pending pathname:

type ClientNavigationState = {
  // ...
  clientParams: Record<string, string | string[]>;
  pendingClientParams: Record<string, string | string[]> | null;  // ✅ Tracked
  cachedPathname: string;  // ❌ Only committed, no pending pathname
  // ...
}

Proposed Solution

Option A: Track Pending Pathname (Recommended)

Add pendingPathname to ClientNavigationState in navigation.ts:

type ClientNavigationState = {
  // ... existing fields
  pendingPathname: string | null;
  cachedPathname: string;
}

Flow changes:

1. Set pendingPathname immediately when __VINEXT_RSC_NAVIGATE__ starts
2. Update isSameRoute comparison:

   const currentPath = state?.pendingPathname ?? 
                       state?.cachedPathname ?? 
                       window.location.pathname;
   const isSameRoute = stripBasePath(url.pathname, __basePath) === 
                       stripBasePath(currentPath, __basePath);

3. Clear pendingPathname in commitClientNavigationState() after successful commit
4. Handle error cases: clear on navigation failure/supersession

Complexity: Requires restructuring navigation flow because isSameRoute is evaluated before stageClientParams() in both cached and non-cached paths.

Option B: Track In-Flight Navigation Destination

Add a destinationPathname field set immediately when navigation starts, before any async work. This separates "where we are going" from "params we are staging".

Edge Cases to Handle

• Overlapping navigations (A→B→C): When C starts, should compare against B (most recent pending), not A. The navId/activeNavigationId system already supersedes older navigations.

• Navigation superseded before commit: If B→C starts but is superseded by B→D, pending pathname should update to D. Cleanup must be robust.

• Error during navigation: If navigation fails, pendingPathname must be cleared in catch blocks. Current _snapshotPending pattern shows the complexity of cleanup.

• Back/forward (traverse) navigation: Different code path via popstate handler. Uses window.location.href directly, less affected.

Files Affected

• packages/vinext/src/shims/navigation.ts — add pendingPathname to state type, update commitClientNavigationState()
• packages/vinext/src/server/app-browser-entry.ts — update isSameRoute logic, set/clear pending pathname

Why This Matters Now

1. UX regression: Rapid navigation is a common user pattern. The current workaround produces visible jank.

2. Self-contained fix: ~30 lines changed, does not depend on major refactors like Layout Persistence  #726 (layout persistence architecture).

3. Code health: The comment acknowledges this as debt. Better to fix properly than let it linger.

Related

• PR fix: cross-route client navigation hangs in Firefox (#652) #690 (Firefox navigation hang fix) — introduced the workaround comment
• Review comment: fix: cross-route client navigation hangs in Firefox (#652) #690 (comment)
• Issue Layout Persistence  #726 — Layout persistence architecture (orthogonal to this race condition)

Testing Needed

• E2E test for rapid navigation sequence (click A→B→C without waiting for intermediate commits)
• Unit test for isSameRoute classification with simulated pending navigation
• Verify no regression in back/forward navigation
• Verify error handling clears pending state correctly