fix: prevent rapid double-tap from pushing duplicate history entries#442
Merged
Junman140 merged 1 commit intoJun 1, 2026
Merged
Conversation
The useTransition guard alone was insufficient because isPending is an async state update — a second tap could arrive before React re-renders. A ref-based lock (set synchronously on click, cleared only when pathname actually changes via useEffect) ensures no second navigation can start while one is in-flight.
Contributor
📝 WalkthroughWalkthroughMobileNav switches from Next.js ChangesMobile Navigation Race Condition Prevention
Possibly related PRs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/mobile-nav.tsx`:
- Around line 33-35: The lock (navigatingTo.current) is only cleared inside the
pathname useEffect, so if a push is interrupted and the URL doesn't change it
stays non-null; modify the navigation handler(s) that set navigatingTo.current
(e.g., the tap/click handler that calls router.push or navigate) to always clear
navigatingTo.current in a finally block after the navigation promise settles,
and add a short fallback timeout (e.g., 2–5s) to reset navigatingTo.current to
null if the navigation neither resolves nor changes pathname; keep the existing
useEffect([pathname]) clear as well for the normal case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Closed
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
2 participants
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.
Closes #328
Problem
Rapidly tapping two nav tabs in succession can push two routes onto the history stack before the first one completes, corrupting the back-button history on mobile.
Root Cause
The previous
useTransitionguard was insufficient —isPendingis an async React state update. A second tap could arrive before React re-renders withisPending=true, and there was no synchronous lock preventing the secondrouter.pushfrom executing.Fix (
components/mobile-nav.tsx)<Link>with<button>+handleNav()handler for full navigation controlnavigatingTo) is set on click and only cleared whenpathnameactually changes (viauseEffect), preventing any second navigation while one is in-flightuseTransitionprovides theisPendingflag to visually disable buttons during navigationpathname === hrefguard skips re-pushing the current pageCloses #328
Summary by CodeRabbit