fix(library): restore breadcrumb 'All' navigation by bypassing next-view-transitions, closes #3782 (#3829)

The breadcrumb "All" button was broken on first click after entering a group because next-view-transitions@0.3.5's useTransitionRouter wraps router.replace() in startTransition + document.startViewTransition, and this combination is incompatible with Next.js 16.2 RSC navigation when only the search params change for the same pathname (e.g. /library?group=foo -> /library). The navigation silently never commits. Extract the library navigation logic into a useLibraryNavigation hook that uses plain useRouter from next/navigation. The data-nav-direction attribute is still set so existing directional CSS keeps working when view transitions fire via popstate. See https://github.com/shuding/next-view-transitions/issues/65 for the upstream incompatibility. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 22:04:37 +08:00
parent de11511c30
commit 7bf4822b27
3 changed files with 208 additions and 25 deletions
@@ -0,0 +1,147 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { renderHook, act } from '@testing-library/react';
+
+vi.mock('next/navigation', () => ({
+  useRouter: vi.fn(),
+}));
+
+import { useRouter, ReadonlyURLSearchParams } from 'next/navigation';
+import { useLibraryNavigation } from '@/app/library/hooks/useLibraryNavigation';
+
+function mockRouter() {
+  return {
+    push: vi.fn(),
+    replace: vi.fn(),
+    back: vi.fn(),
+    forward: vi.fn(),
+    refresh: vi.fn(),
+    prefetch: vi.fn(),
+  };
+}
+
+function makeSearchParams(query: string): ReadonlyURLSearchParams {
+  // URLSearchParams shares the read-only subset of methods we need.
+  return new URLSearchParams(query) as unknown as ReadonlyURLSearchParams;
+}
+
+describe('useLibraryNavigation', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    document.documentElement.removeAttribute('data-nav-direction');
+  });
+
+  it('navigates to /library (no query) when going back to root from a group', () => {
+    // Regression: clicking the breadcrumb "All" button from /library?group=foo
+    // must call router.replace('/library'). Previously this navigation was
+    // wrapped through next-view-transitions' useTransitionRouter which is
+    // incompatible with Next.js 16.2 RSC navigation when the pathname stays
+    // the same and all query params are removed, causing the click to do
+    // nothing on the first attempt. See readest/readest#3782.
+    const router = mockRouter();
+    vi.mocked(useRouter).mockReturnValue(router as never);
+
+    const { result } = renderHook(() => useLibraryNavigation(makeSearchParams('group=foo')));
+
+    act(() => {
+      result.current('');
+    });
+
+    expect(router.replace).toHaveBeenCalledTimes(1);
+    expect(router.replace).toHaveBeenCalledWith('/library', undefined);
+  });
+
+  it('preserves other search params when going back to root', () => {
+    const router = mockRouter();
+    vi.mocked(useRouter).mockReturnValue(router as never);
+
+    const { result } = renderHook(() =>
+      useLibraryNavigation(makeSearchParams('groupBy=author&group=cf6342e&sort=title')),
+    );
+
+    act(() => {
+      result.current('');
+    });
+
+    expect(router.replace).toHaveBeenCalledTimes(1);
+    const calledHref = router.replace.mock.calls[0]![0] as string;
+    expect(calledHref.startsWith('/library?')).toBe(true);
+    const calledParams = new URLSearchParams(calledHref.slice('/library?'.length));
+    expect(calledParams.get('group')).toBeNull();
+    expect(calledParams.get('groupBy')).toBe('author');
+    expect(calledParams.get('sort')).toBe('title');
+  });
+
+  it('navigates forward to a group by setting the group param', () => {
+    const router = mockRouter();
+    vi.mocked(useRouter).mockReturnValue(router as never);
+
+    const { result } = renderHook(() => useLibraryNavigation(makeSearchParams('')));
+
+    act(() => {
+      result.current('group-id-abc');
+    });
+
+    expect(router.replace).toHaveBeenCalledWith('/library?group=group-id-abc', undefined);
+  });
+
+  it('replaces the group param when navigating between sibling groups', () => {
+    const router = mockRouter();
+    vi.mocked(useRouter).mockReturnValue(router as never);
+
+    const { result } = renderHook(() => useLibraryNavigation(makeSearchParams('group=group-a')));
+
+    act(() => {
+      result.current('group-b');
+    });
+
+    expect(router.replace).toHaveBeenCalledWith('/library?group=group-b', undefined);
+  });
+
+  it('sets data-nav-direction to "back" when returning to root from a group', () => {
+    const router = mockRouter();
+    vi.mocked(useRouter).mockReturnValue(router as never);
+
+    const { result } = renderHook(() => useLibraryNavigation(makeSearchParams('group=foo')));
+
+    act(() => {
+      result.current('');
+    });
+
+    expect(document.documentElement.getAttribute('data-nav-direction')).toBe('back');
+  });
+
+  it('sets data-nav-direction to "forward" when entering a group', () => {
+    const router = mockRouter();
+    vi.mocked(useRouter).mockReturnValue(router as never);
+
+    const { result } = renderHook(() => useLibraryNavigation(makeSearchParams('')));
+
+    act(() => {
+      result.current('group-id');
+    });
+
+    expect(document.documentElement.getAttribute('data-nav-direction')).toBe('forward');
+  });
+
+  it('invokes onBeforeNavigate with the current group before navigating', () => {
+    const router = mockRouter();
+    vi.mocked(useRouter).mockReturnValue(router as never);
+    const onBeforeNavigate = vi.fn();
+
+    const { result } = renderHook(() =>
+      useLibraryNavigation(makeSearchParams('group=foo'), onBeforeNavigate),
+    );
+
+    act(() => {
+      result.current('');
+    });
+
+    expect(onBeforeNavigate).toHaveBeenCalledTimes(1);
+    expect(onBeforeNavigate).toHaveBeenCalledWith('foo');
+    // Order matters: callback should run before router.replace so callers can
+    // capture state (e.g. scroll position) of the leaving view.
+    expect(onBeforeNavigate.mock.invocationCallOrder[0]).toBeLessThan(
+      router.replace.mock.invocationCallOrder[0]!,
+    );
+  });
+});
@@ -0,0 +1,51 @@
+import { useCallback } from 'react';
+import { ReadonlyURLSearchParams, useRouter } from 'next/navigation';
+
+import { navigateToLibrary } from '@/utils/nav';
+
+/**
+ * Hook for navigating between library views (group/subgroup/root) while
+ * setting a `data-nav-direction` attribute used by the directional view
+ * transition CSS.
+ *
+ * NOTE: This hook intentionally uses the plain Next.js `useRouter` instead of
+ * `useAppRouter` (which wraps router calls in `next-view-transitions`'s
+ * `useTransitionRouter`). The wrapped router is incompatible with Next.js
+ * 16.2's RSC navigation when only the search params change for the same
+ * pathname (e.g. `/library?group=foo` -> `/library`), which previously caused
+ * the breadcrumb "All" button to do nothing on the first click after entering
+ * a group. See https://github.com/readest/readest/issues/3782 and
+ * https://github.com/shuding/next-view-transitions/issues/65.
+ */
+export function useLibraryNavigation(
+  searchParams: ReadonlyURLSearchParams | null,
+  onBeforeNavigate?: (currentGroup: string) => void,
+) {
+  const router = useRouter();
+
+  return useCallback(
+    (targetGroup: string) => {
+      const currentGroup = searchParams?.get('group') || '';
+
+      onBeforeNavigate?.(currentGroup);
+
+      // Detect and set navigation direction so the view transition CSS can
+      // animate forward (entering a group) vs back (returning to a parent or
+      // the root) using the appropriate slide direction.
+      const direction = currentGroup && !targetGroup ? 'back' : 'forward';
+      if (typeof document !== 'undefined') {
+        document.documentElement.setAttribute('data-nav-direction', direction);
+      }
+
+      const params = new URLSearchParams(searchParams?.toString());
+      if (targetGroup) {
+        params.set('group', targetGroup);
+      } else {
+        params.delete('group');
+      }
+
+      navigateToLibrary(router, `${params.toString()}`);
+    },
+    [searchParams, router, onBeforeNavigate],
+  );
+}
@@ -61,6 +61,7 @@ import { BackupWindow } from './components/BackupWindow';
 import { useDragDropImport } from './hooks/useDragDropImport';
 import { useTransferQueue } from '@/hooks/useTransferQueue';
 import { useAppRouter } from '@/hooks/useAppRouter';
+import { useLibraryNavigation } from './hooks/useLibraryNavigation';
 import { Toast } from '@/components/Toast';
 import {
  createBookGroups,
@@ -151,31 +152,15 @@ const LibraryPageContent = ({ searchParams }: { searchParams: ReadonlyURLSearchP
    }
  }, []);

-  // Unified navigation function that handles scroll position and direction
-  const handleLibraryNavigation = useCallback(
-    (targetGroup: string) => {
-      const currentGroup = searchParams?.get('group') || '';
-
-      // Save current scroll position BEFORE navigation
-      saveScrollPosition(currentGroup);
-
-      // Detect and set navigation direction
-      const direction = currentGroup && !targetGroup ? 'back' : 'forward';
-      document.documentElement.setAttribute('data-nav-direction', direction);
-
-      // Build query params
-      const params = new URLSearchParams(searchParams?.toString());
-      if (targetGroup) {
-        params.set('group', targetGroup);
-      } else {
-        params.delete('group');
-      }
-
-      navigateToLibrary(router, `${params.toString()}`);
-    },
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [searchParams, router],
-  );
+  // Unified navigation function that handles scroll position and direction.
+  // Uses a plain Next.js router (via useLibraryNavigation) instead of the
+  // view-transition-wrapped useAppRouter, because next-view-transitions@0.3.5
+  // is incompatible with Next.js 16.2 RSC navigation when only search params
+  // change for the same pathname (e.g. /library?group=foo -> /library), which
+  // previously broke the breadcrumb "All" button on the first click after
+  // entering a group. See https://github.com/readest/readest/issues/3782 and
+  // https://github.com/shuding/next-view-transitions/issues/65.
+  const handleLibraryNavigation = useLibraryNavigation(searchParams, saveScrollPosition);

  useTheme({ systemUIVisible: true, appThemeColor: 'base-200' });
  useUICSS();