fix(navigation): track pending pathname to fix isSameRoute race condition during rapid navigation

packages/vinext/src/server/app-browser-entry.ts

@@ -23,17 +23,20 @@ import { notifyAppRouterTransitionStart } from "../client/instrumentation-client
 import {
   __basePath,
   activateNavigationSnapshot,
+  clearPendingPathname,
   commitClientNavigationState,
   consumePrefetchResponse,
   createClientNavigationRenderSnapshot,
   getClientNavigationRenderContext,
+  getClientNavigationState,
   getPrefetchCache,
   getPrefetchedUrls,
   pushHistoryStateWithoutNotify,
   replaceClientParamsWithoutNotify,
   replaceHistoryStateWithoutNotify,
   restoreRscResponse,
   setClientParams,
+  setPendingPathname,
   snapshotRscResponse,
   setMountedSlotsHeader,
   setNavigationContext,
@@ -190,8 +193,10 @@ function drainPrePaintEffects(upToRenderId: number): void {
         // Winning navigation: run its actual pre-paint effect
         effect();
       } else {
-        // Superseded navigation: balance its activateNavigationSnapshot()
-        commitClientNavigationState();
+        // Superseded navigation: balance its activateNavigationSnapshot().
+        // Pass undefined navId intentionally so this cleanup cannot clear
+        // pendingPathname owned by the current active navigation.
+        commitClientNavigationState(undefined);
       }
     }
   }
@@ -200,9 +205,16 @@ function drainPrePaintEffects(upToRenderId: number): void {
 function createNavigationCommitEffect(
   href: string,
   historyUpdateMode: HistoryUpdateMode | undefined,
+  navId: number,
   params: Record<string, string | string[]>,
 ): () => void {
   return () => {
+    // Only update URL if this is still the active navigation.
+    // A newer navigation would have incremented activeNavigationId.
+    if (navId !== activeNavigationId) {
+      return;
+    }
+
     const targetHref = new URL(href, window.location.origin).href;
     stageClientParams(params);
 
@@ -212,7 +224,7 @@ function createNavigationCommitEffect(
       pushHistoryStateWithoutNotify(null, "", href);
     }
 
-    commitClientNavigationState();
+    commitClientNavigationState(navId);
   };
 }
 
@@ -551,7 +563,7 @@ async function renderNavigationPayload(
     const resolve = pendingNavigationCommits.get(renderId);
     pendingNavigationCommits.delete(renderId);
     if (snapshotActivated) {
-      commitClientNavigationState();
+      commitClientNavigationState(navId);
     }
     resolve?.();
     throw error;
@@ -747,17 +759,24 @@ async function main(): Promise<void> {
     try {
       const url = new URL(href, window.location.origin);
       const rscUrl = toRscUrl(url.pathname + url.search);
-      // Use startTransition for same-route navigations (searchParam changes)
-      // so React keeps the old UI visible during the transition. For cross-route
-      // navigations (different pathname), use synchronous updates — React's
-      // startTransition hangs in Firefox when replacing the entire tree.
-      // 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.
-      const isSameRoute =
-        stripBasePath(url.pathname, __basePath) ===
+
+      // Get navigation state for comparison
+      const navState = getClientNavigationState();
+
+      // Compare against previous pending navigation, then committed, then window.location
+      // This ensures correct isSameRoute classification during rapid successive navigations
+      const currentPath =
+        navState?.pendingPathname ??
+        navState?.cachedPathname ??
         stripBasePath(window.location.pathname, __basePath);
+
+      const targetPath = stripBasePath(url.pathname, __basePath);
+      const isSameRoute = targetPath === currentPath;
+
+      // Set this navigation as the pending pathname, overwriting any previous.
+      // Pass navId so only this navigation (or a newer one) can clear it later.
+      setPendingPathname(url.pathname, navId);
+
       const elementsAtNavStart = getBrowserRouterState().elements;
       const mountedSlotsHeader = getMountedSlotIdsHeader(elementsAtNavStart);
       const cachedRoute = getVisitedResponse(rscUrl, mountedSlotsHeader, navigationKind);
@@ -789,7 +808,7 @@ async function main(): Promise<void> {
             cachedNavigationSnapshot,
             href,
             navId,
-            createNavigationCommitEffect(href, historyUpdateMode, cachedParams),
+            createNavigationCommitEffect(href, historyUpdateMode, navId, cachedParams),
             isSameRoute,
             toActionType(navigationKind),
           );
@@ -878,7 +897,7 @@ async function main(): Promise<void> {
           navigationSnapshot,
           href,
           navId,
-          createNavigationCommitEffect(href, historyUpdateMode, navParams),
+          createNavigationCommitEffect(href, historyUpdateMode, navId, navParams),
           isSameRoute,
           toActionType(navigationKind),
         );
@@ -905,7 +924,13 @@ async function main(): Promise<void> {
       // before re-throwing, so this guard correctly skips the double-decrement case.
       if (_snapshotPending) {
         _snapshotPending = false;
-        commitClientNavigationState();
+        commitClientNavigationState(navId);
+      }
+      // Clear pending pathname on error so subsequent navigations compare correctly.
+      // Only clear if this is still the active navigation — a newer navigation
+      // has already overwritten pendingPathname with its own target.
+      if (navId === activeNavigationId) {
+        clearPendingPathname(navId);
       }
       // Don't hard-navigate to a stale URL if this navigation was superseded by
       // a newer one — the newer navigation is already in flight and would be clobbered.

packages/vinext/src/shims/navigation.ts

@@ -500,6 +500,8 @@ type ClientNavigationState = {
   clientParamsJson: string;
   pendingClientParams: Record<string, string | string[]> | null;
   pendingClientParamsJson: string | null;
+  pendingPathname: string | null;
+  pendingPathnameNavId: number | null;
   originalPushState: typeof window.history.pushState;
   originalReplaceState: typeof window.history.replaceState;
   patchInstalled: boolean;
@@ -525,7 +527,7 @@ export function getMountedSlotsHeader(): string | null {
   return globalState[_MOUNTED_SLOTS_HEADER_KEY] ?? null;
 }
 
-function getClientNavigationState(): ClientNavigationState | null {
+export function getClientNavigationState(): ClientNavigationState | null {
   if (isServer) return null;
 
   const globalState = window as ClientNavigationGlobal;
@@ -538,6 +540,8 @@ function getClientNavigationState(): ClientNavigationState | null {
     clientParamsJson: "{}",
     pendingClientParams: null,
     pendingClientParamsJson: null,
+    pendingPathname: null,
+    pendingPathnameNavId: null,
     // NB: These capture the currently installed history methods, not guaranteed
     // native ones. If a third-party library (analytics, router) has already patched
     // history methods before this module loads, we intentionally preserve that
@@ -768,6 +772,34 @@ export function getClientParams(): Record<string, string | string[]> {
   return getClientNavigationState()?.clientParams ?? _fallbackClientParams;
 }
 
+/**
+ * Set the pending pathname for client-side navigation.
+ * Strips the base path before storing. Associates the pathname with the given navId
+ * so only that navigation (or a newer one) can clear it.
+ */
+export function setPendingPathname(pathname: string, navId: number): void {
+  const state = getClientNavigationState();
+  if (!state) return;
+  state.pendingPathname = stripBasePath(pathname, __basePath);
+  state.pendingPathnameNavId = navId;
+}
+
+/**
+ * Clear the pending pathname, but only if the given navId matches the one
+ * that set it, or if pendingPathnameNavId is null (no active owner).
+ * This prevents superseded navigations from clearing state belonging to newer navigations.
+ */
+export function clearPendingPathname(navId: number): void {
+  const state = getClientNavigationState();
+  if (!state) return;
+  // Only clear if this navId is the one that set the pendingPathname,
+  // or if pendingPathnameNavId is null (no owner)
+  if (state.pendingPathnameNavId === null || state.pendingPathnameNavId === navId) {
+    state.pendingPathname = null;
+    state.pendingPathnameNavId = null;
+  }
+}
+
 function getClientParamsSnapshot(): Record<string, string | string[]> {
   return getClientNavigationState()?.clientParams ?? _EMPTY_PARAMS;
 }
@@ -927,7 +959,14 @@ function withSuppressedUrlNotifications<T>(fn: () => T): T {
   }
 }
 
-export function commitClientNavigationState(): void {
+/**
+ * Commit pending client navigation state to committed snapshots.
+ *
+ * navId is optional: callers that don't own pendingPathname (for example,
+ * superseded pre-paint cleanup) may pass undefined to flush snapshot/params
+ * state without clearing pendingPathname owned by the active navigation.
+ */
+export function commitClientNavigationState(navId?: number): void {
   if (isServer) return;
   const state = getClientNavigationState();
   if (!state) return;
@@ -947,6 +986,18 @@ export function commitClientNavigationState(): void {
     state.pendingClientParams = null;
     state.pendingClientParamsJson = null;
   }
+  // Clear pending pathname when navigation commits, but only if:
+  // - The navId matches the one that set pendingPathname
+  // - No newer navigation has overwritten pendingPathname (pendingPathnameNavId === null or matches)
+  // - navId is undefined only for non-owning callers, which must not clear
+  //   pendingPathname for an active navigation.
+  const canClearPendingPathname =
+    state.pendingPathnameNavId === null ||
+    (navId !== undefined && state.pendingPathnameNavId === navId);
+  if (canClearPendingPathname) {
+    state.pendingPathname = null;
+    state.pendingPathnameNavId = null;
+  }
   const shouldNotify = urlChanged || state.hasPendingNavigationUpdate;
   state.hasPendingNavigationUpdate = false;
 

tests/e2e/app-router/navigation-flows.spec.ts

@@ -225,4 +225,48 @@ test.describe("App Router navigation flows", () => {
     await page.goto(`${BASE}/client-nav-test?q=test-param`);
     await expect(page.locator('[data-testid="client-search-q"]')).toHaveText("test-param");
   });
+
+  test("back/forward through rapid-nav pages maintains state", async ({ page }) => {
+    const errors: string[] = [];
+    page.on("console", (msg) => {
+      if (msg.type() === "error") {
+        errors.push(msg.text());
+      }
+    });
+
+    // Start at page A
+    await page.goto(`${BASE}/nav-rapid/page-a`);
+    await expect(page.locator("h1")).toHaveText("Page A");
+    await waitForAppRouterHydration(page);
+
+    // Navigate A -> B
+    await page.click('[data-testid="page-a-link-to-b"]');
+    await expect(page.locator("h1")).toHaveText("Page B");
+
+    // Navigate B -> C
+    await page.click('[data-testid="link-to-c"]');
+    await expect(page.locator("h1")).toHaveText("Page C");
+
+    // Back navigation
+    await page.goBack();
+    await page.goBack();
+
+    // Should be back at A
+    await expect(page.locator("h1")).toHaveText("Page A");
+    await expect(page).toHaveURL(`${BASE}/nav-rapid/page-a`);
+
+    // Forward navigation
+    await page.goForward();
+    await page.goForward();
+
+    // Should be back at C
+    await expect(page.locator("h1")).toHaveText("Page C");
+    await expect(page).toHaveURL(`${BASE}/nav-rapid/page-c`);
+
+    // Verify no navigation-related errors
+    const navigationErrors = errors.filter(
+      (e) => e.includes("navigation") || e.includes("vinext") || e.includes("router"),
+    );
+    expect(navigationErrors).toHaveLength(0);
+  });
 });

tests/e2e/app-router/rapid-navigation.spec.ts

@@ -0,0 +1,94 @@
+import { test, expect } from "@playwright/test";
+
+const BASE = "http://localhost:4174";
+
+/**
+ * Wait for the RSC browser entry to hydrate. The browser entry sets
+ * window.__VINEXT_RSC_ROOT__ after hydration completes.
+ */
+async function waitForHydration(page: import("@playwright/test").Page) {
+  await expect(async () => {
+    const ready = await page.evaluate(() => !!(window as any).__VINEXT_RSC_ROOT__);
+    expect(ready).toBe(true);
+  }).toPass({ timeout: 10_000 });
+}
+
+test.describe("rapid navigation", () => {
+  test("A→B→C rapid navigation completes smoothly", async ({ page }) => {
+    const errors: string[] = [];
+    page.on("console", (msg) => {
+      if (msg.type() === "error") {
+        errors.push(msg.text());
+      }
+    });
+
+    // Start at page A
+    await page.goto(`${BASE}/nav-rapid/page-a`);
+    await expect(page.locator("h1")).toHaveText("Page A");
+    await waitForHydration(page);
+
+    // Click B then immediately click C (before B fully commits)
+    // Use a single page.evaluate() to click both links atomically.
+    // This avoids React re-render issues and prevents "execution context destroyed" errors in CI.
+    await page.evaluate(() => {
+      const linkB = document.querySelector('[data-testid="page-a-link-to-b"]') as HTMLElement;
+      const linkC = document.querySelector('[data-testid="page-a-link-to-c"]') as HTMLElement;
+      if (linkB) linkB.click();
+      if (linkC) linkC.click();
+    });
+
+    // Use toHaveURL (polling) instead of waitForURL (navigation event) because
+    // rapid back-to-back client-side navigations abort the first navigation,
+    // causing waitForURL to fail with ERR_ABORTED.
+    await expect(page).toHaveURL(`${BASE}/nav-rapid/page-c`, { timeout: 10_000 });
+    await expect(page.locator("h1")).toHaveText("Page C");
+
+    const navigationErrors = errors.filter(
+      (e) => e.includes("navigation") || e.includes("vinext") || e.includes("router"),
+    );
+    expect(navigationErrors).toHaveLength(0);
+  });
+
+  test("same-route query change during cross-route navigation", async ({ page }) => {
+    const errors: string[] = [];
+    page.on("console", (msg) => {
+      if (msg.type() === "error") {
+        errors.push(msg.text());
+      }
+    });
+
+    // Start at page A
+    await page.goto(`${BASE}/nav-rapid/page-a`);
+    await expect(page.locator("h1")).toHaveText("Page A");
+    await waitForHydration(page);
+
+    // Navigate to B then immediately change query param (same-route nav)
+    // Use a single page.evaluate() to click both links atomically.
+    //
+    // Why isSameRoute === true for the second navigation:
+    // Both clicks run synchronously in the same microtask. The first click calls
+    // setPendingPathname('/nav-rapid/page-b', navId=1), storing the pending target.
+    // The second click then reads pendingPathname (which is '/nav-rapid/page-b'),
+    // compares it to its own target pathname (also '/nav-rapid/page-b'), and sees
+    // they match — so isSameRoute is true. This is correct behavior: the second
+    // navigation is a same-route query-param change, not a cross-route navigation.
+    await page.evaluate(() => {
+      const linkB = document.querySelector('[data-testid="page-a-link-to-b"]') as HTMLElement;
+      const linkFilter = document.querySelector(
+        '[data-testid="page-a-link-to-b-filter"]',
+      ) as HTMLElement;
+      if (linkB) linkB.click();
+      if (linkFilter) linkFilter.click();
+    });
+
+    // Should settle on B with query param
+    await expect(page.locator("h1")).toHaveText("Page B");
+    await expect(page).toHaveURL(`${BASE}/nav-rapid/page-b?filter=test`);
+
+    // Verify no navigation-related errors
+    const navigationErrors = errors.filter(
+      (e) => e.includes("navigation") || e.includes("vinext") || e.includes("router"),
+    );
+    expect(navigationErrors).toHaveLength(0);
+  });
+});

tests/fixtures/app-basic/app/nav-rapid/layout.tsx

@@ -0,0 +1,3 @@
+export default function NavRapidLayout({ children }: { children: React.ReactNode }) {
+  return <main data-testid="nav-rapid-container">{children}</main>;
+}