[PR #745] [MERGED] fix(navigation): track pending pathname to fix isSameRoute race condition during rapid navigation #813

New issue

Closed

opened 2026-05-06 13:10:14 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:10:14 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/745
Author: @Divkix
Created: 4/2/2026
Status: ✅ Merged
Merged: 4/13/2026
Merged by: @james-elicx

Base: main ← Head: fix/744-pending-pathname-race

📝 Commits (10+)

78eefbf fix(navigation): track pending pathname to fix isSameRoute race condition during rapid navigation
76101bb fix(navigation): address review feedback on pending pathname race condition fix
7056e6a resolve merge conflict
5c9e3d6 Resolve merge conflict: keep pending pathname race condition fix (#744)
391a244 Address PR review feedback for pending pathname race condition fix
61734a1 Fix unit tests - remove client-side mocks that fail in CI
9287f4b Fix formatting issues in tests/shims.test.ts
3cbab5a chore(tests): clarify smoke test comments for pending pathname helpers
b35fc90 fix: clear pending pathname when navigating to different route
4f694b6 fix: remove race condition in pending pathname handling

📊 Changes

8 files changed (+298 additions, -18 deletions)

View changed files

📝 packages/vinext/src/server/app-browser-entry.ts (+48 -16)
📝 packages/vinext/src/shims/navigation.ts (+53 -2)
📝 tests/e2e/app-router/navigation-flows.spec.ts (+44 -0)
➕ tests/e2e/app-router/rapid-navigation.spec.ts (+84 -0)
➕ tests/fixtures/app-basic/app/nav-rapid/layout.tsx (+3 -0)
➕ tests/fixtures/app-basic/app/nav-rapid/page-a/page.tsx (+26 -0)
➕ tests/fixtures/app-basic/app/nav-rapid/page-b/page.tsx (+22 -0)
➕ tests/fixtures/app-basic/app/nav-rapid/page-c/page.tsx (+18 -0)

📄 Description

Fixes #744

Problem

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

Solution

Add pendingPathname to ClientNavigationState to track the destination URL of in-flight navigations. This allows isSameRoute to compare against the most recent pending destination instead of stale window.location.pathname.

Changes

Add pendingPathname field to ClientNavigationState type in navigation.ts
Export setPendingPathname() and clearPendingPathname() helpers
Update isSameRoute logic in app-browser-entry.ts to compare against: pendingPathname → cachedPathname → window.location.pathname
Clear pendingPathname on navigation commit (commitClientNavigationState) and error
Add navId-based ownership model to prevent superseded navigations from clobbering active state
Add E2E tests for rapid navigation scenarios (A→B→C, same-route query change)
Add back/forward state consistency test in navigation-flows.spec.ts

Edge Cases Handled

A→B→C rapid navigation: C compares against B (pending) rather than A (stale window.location)
Navigation superseded: New navigation overwrites pending, old bails out via navId check
Navigation error: clearPendingPathname() resets state
Back/forward: Different code path (popstate), not affected by this change

Testing

E2E tests: 2 rapid navigation scenarios in rapid-navigation.spec.ts
Back/forward state consistency test in navigation-flows.spec.ts
All existing tests pass

_{🔄 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/745 **Author:** [@Divkix](https://github.com/Divkix) **Created:** 4/2/2026 **Status:** ✅ Merged **Merged:** 4/13/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `fix/744-pending-pathname-race` --- ### 📝 Commits (10+) - [`78eefbf`](https://github.com/cloudflare/vinext/commit/78eefbf29af3a180a1e048a938baab593f43f8e0) fix(navigation): track pending pathname to fix isSameRoute race condition during rapid navigation - [`76101bb`](https://github.com/cloudflare/vinext/commit/76101bbd0e84044edef3c6825272f0000d7d24f9) fix(navigation): address review feedback on pending pathname race condition fix - [`7056e6a`](https://github.com/cloudflare/vinext/commit/7056e6a0bd370ba127363049c5e6655e6ed0e4f0) resolve merge conflict - [`5c9e3d6`](https://github.com/cloudflare/vinext/commit/5c9e3d6dbb53d4f9a7fb1e91901aa4aa41f4157b) Resolve merge conflict: keep pending pathname race condition fix (#744) - [`391a244`](https://github.com/cloudflare/vinext/commit/391a244ee851f4037a96ba5ff3913ae5f9c0a645) Address PR review feedback for pending pathname race condition fix - [`61734a1`](https://github.com/cloudflare/vinext/commit/61734a1a8a3fbb88045cc640da402e91e117932e) Fix unit tests - remove client-side mocks that fail in CI - [`9287f4b`](https://github.com/cloudflare/vinext/commit/9287f4b342e17067bf1b721410e83ea082781986) Fix formatting issues in tests/shims.test.ts - [`3cbab5a`](https://github.com/cloudflare/vinext/commit/3cbab5a1ca1053d8a3b5cb2fd2edcb5ddc33d94b) chore(tests): clarify smoke test comments for pending pathname helpers - [`b35fc90`](https://github.com/cloudflare/vinext/commit/b35fc90e3702d967f874ea617bbbbfc9a0a8ee55) fix: clear pending pathname when navigating to different route - [`4f694b6`](https://github.com/cloudflare/vinext/commit/4f694b67f6756f915e9a28e008121c8d63737923) fix: remove race condition in pending pathname handling ### 📊 Changes **8 files changed** (+298 additions, -18 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/server/app-browser-entry.ts` (+48 -16) 📝 `packages/vinext/src/shims/navigation.ts` (+53 -2) 📝 `tests/e2e/app-router/navigation-flows.spec.ts` (+44 -0) ➕ `tests/e2e/app-router/rapid-navigation.spec.ts` (+84 -0) ➕ `tests/fixtures/app-basic/app/nav-rapid/layout.tsx` (+3 -0) ➕ `tests/fixtures/app-basic/app/nav-rapid/page-a/page.tsx` (+26 -0) ➕ `tests/fixtures/app-basic/app/nav-rapid/page-b/page.tsx` (+22 -0) ➕ `tests/fixtures/app-basic/app/nav-rapid/page-c/page.tsx` (+18 -0) </details> ### 📄 Description Fixes #744 ## Problem During rapid successive navigations (e.g., user clicks A→B→C quickly), the "isSameRoute" check in app-browser-entry.ts compared against stale window.location.pathname because URL commits are deferred via useLayoutEffect. ## Solution Add pendingPathname to ClientNavigationState to track the destination URL of in-flight navigations. This allows isSameRoute to compare against the most recent pending destination instead of stale window.location.pathname. ## Changes - Add pendingPathname field to ClientNavigationState type in navigation.ts - Export setPendingPathname() and clearPendingPathname() helpers - Update isSameRoute logic in app-browser-entry.ts to compare against: pendingPathname → cachedPathname → window.location.pathname - Clear pendingPathname on navigation commit (commitClientNavigationState) and error - Add navId-based ownership model to prevent superseded navigations from clobbering active state - Add E2E tests for rapid navigation scenarios (A→B→C, same-route query change) - Add back/forward state consistency test in navigation-flows.spec.ts ## Edge Cases Handled - A→B→C rapid navigation: C compares against B (pending) rather than A (stale window.location) - Navigation superseded: New navigation overwrites pending, old bails out via navId check - Navigation error: clearPendingPathname() resets state - Back/forward: Different code path (popstate), not affected by this change ## Testing - E2E tests: 2 rapid navigation scenarios in rapid-navigation.spec.ts - Back/forward state consistency test in navigation-flows.spec.ts - All existing tests pass --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>