From 6cade0931015521a9d84492d2946a4fee1b7f05e Mon Sep 17 00:00:00 2001
From: kewton <newtons.boiled.clock@gmail.com>
Date: Tue, 14 Apr 2026 12:43:22 +0900
Subject: [PATCH] fix(sidebar): remove fallback timer that caused full-page
 reloads

Next.js App Router defers history.pushState() to a React effect, so
window.location.pathname does not update synchronously with router.push().
The 500ms fallback timer was firing before the URL updated and calling
window.location.href, causing a full hard reload on every sidebar click.

Remove the fallback timer and useEffect cleanup entirely; router.push()
alone is sufficient for App Router navigation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 src/components/layout/Sidebar.tsx             | 40 +++----------------
 tests/unit/components/layout/Sidebar.test.tsx | 28 +------------
 2 files changed, 7 insertions(+), 61 deletions(-)

diff --git a/src/components/layout/Sidebar.tsx b/src/components/layout/Sidebar.tsx
index 5ebc7e5e..97def4a9 100644
--- a/src/components/layout/Sidebar.tsx
+++ b/src/components/layout/Sidebar.tsx
@@ -156,45 +156,15 @@ export const Sidebar = memo(function Sidebar() {
     }));
   }, []);
 
-  // Ref to track the single active fallback timer.
-  // Using a ref (not state) so updates don't trigger re-renders.
-  const fallbackTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
-
-  useEffect(() => {
-    return () => {
-      if (fallbackTimerRef.current !== null) {
-        clearTimeout(fallbackTimerRef.current);
-        fallbackTimerRef.current = null;
-      }
-    };
-  }, []);
-
   // Handle branch selection.
-  // Fallback: if router.push silently fails (e.g., Next.js Router Cache corruption),
-  // navigate via window.location.href after a short delay.
-  //
-  // Bug fix: previously each click created a new timer whose cleanup was never
-  // invoked (useCallback return values are discarded by event handlers), and
-  // popstate does not fire on programmatic router.push calls. Accumulated timers
-  // compared stale target paths against the current pathname and triggered spurious
-  // full-page reloads. Using a single ref-tracked timer ensures only the latest
-  // click's fallback can fire.
+  // Note: no fallback timer — Next.js App Router defers history.pushState to a React
+  // effect, so window.location.pathname does not update synchronously with router.push().
+  // A fallback timer that checks window.location.pathname would fire before the URL
+  // updates and trigger a spurious full-page reload on every navigation.
   const handleBranchClick = useCallback((branchId: string) => {
     selectWorktree(branchId);
-    const targetPath = `/worktrees/${branchId}`;
-    router.push(targetPath);
+    router.push(`/worktrees/${branchId}`);
     closeMobileDrawer();
-
-    // Cancel any timer from a previous click before setting a new one.
-    if (fallbackTimerRef.current !== null) {
-      clearTimeout(fallbackTimerRef.current);
-    }
-    fallbackTimerRef.current = setTimeout(() => {
-      fallbackTimerRef.current = null;
-      if (window.location.pathname !== targetPath) {
-        window.location.href = targetPath;
-      }
-    }, 500);
   }, [selectWorktree, router, closeMobileDrawer]);
 
   // DnD sensors: require 8px move before activating (distinguishes click from drag)
diff --git a/tests/unit/components/layout/Sidebar.test.tsx b/tests/unit/components/layout/Sidebar.test.tsx
index 5275619d..689177bc 100644
--- a/tests/unit/components/layout/Sidebar.test.tsx
+++ b/tests/unit/components/layout/Sidebar.test.tsx
@@ -331,25 +331,8 @@ describe('Sidebar', () => {
       expect(screen.getAllByText('feature/test-1').length).toBeGreaterThanOrEqual(1);
     });
 
-    it('should clear pending fallback timer on unmount', async () => {
-      const hrefSetter = vi.fn();
-      const locationObj = {
-        ...originalLocation,
-        pathname: '/worktrees/feature-test-1',
-      };
-      Object.defineProperty(locationObj, 'href', {
-        get: () => 'http://localhost/worktrees/feature-test-1',
-        set: (value: string) => {
-          hrefSetter(value);
-        },
-        configurable: true,
-      });
-      Object.defineProperty(window, 'location', {
-        writable: true,
-        value: locationObj,
-      });
-
-      const { unmount } = render(
+    it('should call router.push directly without fallback timer', async () => {
+      render(
         <Wrapper>
           <Sidebar />
         </Wrapper>
@@ -359,18 +342,11 @@ describe('Sidebar', () => {
         expect(screen.getAllByText('feature/test-2').length).toBeGreaterThanOrEqual(1);
       });
 
-      vi.useFakeTimers();
-
       const branchItem = screen.getAllByText('feature/test-2')[0].closest('[data-testid="branch-list-item"]');
       expect(branchItem).not.toBeNull();
 
       fireEvent.click(branchItem!);
       expect(mockPush).toHaveBeenCalledWith('/worktrees/feature-test-2');
-
-      unmount();
-      vi.advanceTimersByTime(500);
-
-      expect(hrefSetter).not.toHaveBeenCalled();
     });
   });