perf(sidebar): virtualize BooknoteView and memoize derivations (#4352)

Switching the annotation/bookmark sidebar to a flat virtualized list eliminates the per-item layout reads that caused multi-second jank when toggling tabs on books with hundreds of notes. A. Virtualize the list with react-virtuoso - Flatten group headers + notes into a single FlatBooknoteRow array. - Embed an OverlayScrollbars instance inside the tab so scrollbar styling is preserved while Virtuoso owns the viewport (same nested pattern as TOCView). - Track the parent scroll-container's height with ResizeObserver to give Virtuoso a bounded viewport. - Replace the per-item useScrollToItem (which called getBoundingClientRect and closest() on every BooknoteItem on every progress tick — O(n) sync reflow on 1000+ items) with a single virtuosoRef.scrollToIndex driven by nearestCfi. B. Stabilize derivations with useMemo / useCallback - filteredNotes, sortedGroups, flatItems, nearestCfi all useMemo so an unrelated config change (e.g. viewSettings autosave) no longer triggers a full sort + group rebuild. - handleBrowseBookNotes is now useCallback so BooknoteItem's React.memo can hit on prop equality. C. Memoize BooknoteItem - Wrap the component in React.memo. With stable item / onClick references from the parent, re-renders triggered by sibling progress updates no longer cascade across every visible row. - Cache marked.parse(item.note) and dayjs(item.createdAt).fromNow() in useMemo. marked is the dominant per-render cost for note rows. - isCurrent moves to a useMemo over isCfiInLocation; the per-item scrollIntoView is removed since BooknoteView now drives scrolling. useScrollToItem is intentionally left intact — SearchResults still uses it and its smaller list does not exhibit the same jank.
2026-05-29 16:25:38 +08:00
parent a848c142c8
commit 7bdd3ecdee
2 changed files with 229 additions and 60 deletions
@@ -1,6 +1,6 @@
 import clsx from 'clsx';
 import dayjs from 'dayjs';
-import React, { useRef, useState } from 'react';
+import React, { useMemo, useRef, useState } from 'react';
 import { MdEdit, MdDelete } from 'react-icons/md';

 import { marked } from 'marked';
@@ -13,8 +13,8 @@ import { useBookDataStore } from '@/store/bookDataStore';
 import { useTranslation } from '@/hooks/useTranslation';
 import { useResponsiveSize } from '@/hooks/useResponsiveSize';
 import { eventDispatcher } from '@/utils/event';
+import { isCfiInLocation } from '@/utils/cfi';
 import { removeBookNoteOverlays } from '../../utils/annotatorUtil';
-import useScrollToItem from '../../hooks/useScrollToItem';
 import TextButton from '@/components/TextButton';
 import TextEditor, { TextEditorRef } from '@/components/TextEditor';

@@ -44,7 +44,22 @@ const BooknoteItem: React.FC<BooknoteItemProps> = ({ bookKey, item, isNearest, o
  const size18 = useResponsiveSize(18);

  const progress = getProgress(bookKey);
-  const { isCurrent, viewRef } = useScrollToItem(cfi, progress, isNearest);
+  // Active highlight: keep visual "current" state but don't scroll from the
+  // item itself anymore — the parent (virtualized) BooknoteView handles
+  // scrolling via virtuosoRef.scrollToIndex, avoiding N getBoundingClientRect
+  // calls when the list grows large.
+  const isCurrent = useMemo(
+    () => isCfiInLocation(cfi, progress?.location) || !!isNearest,
+    [cfi, progress?.location, isNearest],
+  );
+
+  // marked.parse is heavy when called on every list scroll re-render across
+  // hundreds of items. Cache by note text — note edits change item.note and
+  // bust the cache automatically.
+  const noteHtml = useMemo(() => (note ? marked.parse(note) : ''), [note]);
+
+  // dayjs().fromNow() reformats every render; cache per createdAt.
+  const createdAtLabel = useMemo(() => dayjs(item.createdAt).fromNow(), [item.createdAt]);

  const handleClickItem = (event: React.MouseEvent | React.KeyboardEvent) => {
    event.preventDefault();
@@ -137,7 +152,7 @@ const BooknoteItem: React.FC<BooknoteItemProps> = ({ bookKey, item, isNearest, o
    <li
      // eslint-disable-next-line jsx-a11y/no-noninteractive-element-to-interactive-role
      role='button'
-      ref={viewRef}
+      aria-current={isCurrent ? 'page' : undefined}
      className={clsx(
        'booknote-item border-base-300 content group relative my-2 cursor-pointer rounded-lg p-2',
        isCurrent
@@ -168,7 +183,7 @@ const BooknoteItem: React.FC<BooknoteItemProps> = ({ bookKey, item, isNearest, o
          <div
            className='content prose prose-sm font-size-sm'
            dir='auto'
-            dangerouslySetInnerHTML={{ __html: marked.parse(item.note) }}
+            dangerouslySetInnerHTML={{ __html: noteHtml }}
          ></div>
        )}
        <div className='flex items-start'>
@@ -238,9 +253,7 @@ const BooknoteItem: React.FC<BooknoteItemProps> = ({ bookKey, item, isNearest, o
            <span className='truncate text-sm text-gray-500 sm:text-xs'>
              {item.page ? _('p {{page}}' + ' · ', { page: item.page }) : ''}
            </span>
-            <span className='truncate text-sm text-gray-500 sm:text-xs'>
-              {dayjs(item.createdAt).fromNow()}
-            </span>
+            <span className='truncate text-sm text-gray-500 sm:text-xs'>{createdAtLabel}</span>
          </div>
          <div
            className={clsx('flex items-center justify-end gap-3', isEditable && 'w-full')}
@@ -270,4 +283,9 @@ const BooknoteItem: React.FC<BooknoteItemProps> = ({ bookKey, item, isNearest, o
  );
 };

-export default BooknoteItem;
+// Memoize: BooknoteView re-renders on every progress tick / config change.
+// Without React.memo each tick would re-render every visible note row even
+// though their props are unchanged. Default shallow compare is enough since
+// `item` and `onClick` are stable references from the parent's useMemo /
+// useCallback.
+export default React.memo(BooknoteItem);
@@ -1,15 +1,29 @@
-import React, { useMemo } from 'react';
+import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
+import { Virtuoso, VirtuosoHandle } from 'react-virtuoso';
+import { useOverlayScrollbars } from 'overlayscrollbars-react';
+import 'overlayscrollbars/overlayscrollbars.css';
 import * as CFI from 'foliate-js/epubcfi.js';
+
 import { useBookDataStore } from '@/store/bookDataStore';
 import { useReaderStore } from '@/store/readerStore';
 import { useSidebarStore } from '@/store/sidebarStore';
 import { findTocItemBS } from '@/services/nav';
 import { findNearestCfi } from '@/utils/cfi';
 import { TOCItem } from '@/libs/document';
-import { BooknoteGroup, BookNoteType } from '@/types/book';
+import { BookNote, BooknoteGroup, BookNoteType } from '@/types/book';
 import { useTranslation } from '@/hooks/useTranslation';
 import BooknoteItem from './BooknoteItem';

+type FlatBooknoteRow =
+  | { kind: 'group-header'; key: string; group: BooknoteGroup }
+  | {
+      kind: 'note';
+      key: string;
+      group: BooknoteGroup;
+      item: BookNote;
+      indexInGroup: number;
+    };
+
 const BooknoteView: React.FC<{
  type: BookNoteType;
  bookKey: string;
@@ -21,73 +35,210 @@ const BooknoteView: React.FC<{
  const { setActiveBooknoteType, setBooknoteResults } = useSidebarStore();
  const config = getConfig(bookKey)!;
  const progress = getProgress(bookKey);
+  const allNotes = config.booknotes ?? [];

-  const { booknotes: allNotes = [] } = config;
-  const booknotes = allNotes.filter((note) => note.type === type && !note.deletedAt);
+  // Filter active notes of this type. useMemo so referential stability flows
+  // through derived data and prevents needless recomputation when unrelated
+  // config fields change (e.g. viewSettings, lastUpdated).
+  const filteredNotes = useMemo(
+    () => allNotes.filter((note) => note.type === type && !note.deletedAt),
+    [allNotes, type],
+  );

-  const booknoteGroups: { [href: string]: BooknoteGroup } = {};
-  for (const booknote of booknotes) {
-    const tocItem = findTocItemBS(toc ?? [], booknote.cfi);
-    const href = tocItem?.href || '';
-    const label = tocItem?.label || '';
-    const id = tocItem?.id || 0;
-    if (!booknoteGroups[href]) {
-      booknoteGroups[href] = { id, href, label, booknotes: [] };
+  // Build groups + sort by toc id and intra-group cfi.
+  const sortedGroups = useMemo<BooknoteGroup[]>(() => {
+    const groups: { [href: string]: BooknoteGroup } = {};
+    for (const booknote of filteredNotes) {
+      const tocItem = findTocItemBS(toc ?? [], booknote.cfi);
+      const href = tocItem?.href || '';
+      const label = tocItem?.label || '';
+      const id = tocItem?.id || 0;
+      if (!groups[href]) {
+        groups[href] = { id, href, label, booknotes: [] };
+      }
+      groups[href].booknotes.push(booknote);
    }
-    booknoteGroups[href].booknotes.push(booknote);
-  }
-
-  Object.values(booknoteGroups).forEach((group) => {
-    group.booknotes.sort((a, b) => {
-      return CFI.compare(a.cfi, b.cfi);
+    Object.values(groups).forEach((g) => {
+      g.booknotes.sort((a, b) => CFI.compare(a.cfi, b.cfi));
    });
-  });
+    return Object.values(groups).sort((a, b) => a.id - b.id);
+  }, [filteredNotes, toc]);

-  const sortedGroups = Object.values(booknoteGroups).sort((a, b) => {
-    return a.id - b.id;
-  });
+  // Flatten group/item tree into a single virtualizable list.
+  const flatItems = useMemo<FlatBooknoteRow[]>(() => {
+    const rows: FlatBooknoteRow[] = [];
+    for (const group of sortedGroups) {
+      rows.push({ kind: 'group-header', key: `h-${group.href}`, group });
+      group.booknotes.forEach((item, indexInGroup) => {
+        rows.push({
+          kind: 'note',
+          key: `n-${group.href}-${indexInGroup}-${item.cfi}`,
+          group,
+          item,
+          indexInGroup,
+        });
+      });
+    }
+    return rows;
+  }, [sortedGroups]);

+  // Nearest cfi for "current" highlight; sortedGroups identity tracks content
+  // changes so stale cached cfis aren't kept after a delete/edit.
  const nearestCfi = useMemo(() => {
-    const allSorted = sortedGroups.flatMap((g) => g.booknotes.map((n) => n.cfi));
+    const allSorted: string[] = [];
+    for (const g of sortedGroups) {
+      for (const n of g.booknotes) allSorted.push(n.cfi);
+    }
    return findNearestCfi(allSorted, progress?.location);
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [progress?.location, sortedGroups.length]);
+  }, [progress?.location, sortedGroups]);

-  const handleBrowseBookNotes = () => {
-    if (booknotes.length === 0) return;
-
-    const sorted = [...booknotes].sort((a, b) => CFI.compare(a.cfi, b.cfi));
+  const handleBrowseBookNotes = useCallback(() => {
+    if (filteredNotes.length === 0) return;
+    const sorted = [...filteredNotes].sort((a, b) => CFI.compare(a.cfi, b.cfi));
    setActiveBooknoteType(bookKey, type);
    setBooknoteResults(bookKey, sorted);
-  };
+  }, [filteredNotes, bookKey, type, setActiveBooknoteType, setBooknoteResults]);
+
+  // ---- Virtualization wiring (mirrors TOCView pattern) ----
+  const containerRef = useRef<HTMLDivElement | null>(null);
+  const osRootRef = useRef<HTMLDivElement | null>(null);
+  const virtuosoRef = useRef<VirtuosoHandle | null>(null);
+  const [scroller, setScroller] = useState<HTMLElement | null>(null);
+  const [containerHeight, setContainerHeight] = useState(400);
+  const lastScrolledCfiRef = useRef<string | null>(null);
+
+  const [initialize, osInstance] = useOverlayScrollbars({
+    defer: true,
+    options: { scrollbars: { autoHide: 'scroll' } },
+    events: {
+      initialized(instance) {
+        const { viewport } = instance.elements();
+        viewport.style.overflowX = 'var(--os-viewport-overflow-x)';
+        viewport.style.overflowY = 'var(--os-viewport-overflow-y)';
+      },
+    },
+  });
+
+  useEffect(() => {
+    const root = osRootRef.current;
+    if (scroller && root) {
+      initialize({ target: root, elements: { viewport: scroller } });
+    }
+    return () => osInstance()?.destroy();
+  }, [scroller, initialize, osInstance]);
+
+  const handleScrollerRef = useCallback((el: HTMLElement | Window | null) => {
+    setScroller(el instanceof HTMLElement ? el : null);
+  }, []);
+
+  // Track the parent scroll container's available height so Virtuoso has a
+  // bounded viewport. Same pattern as TOCView.
+  useEffect(() => {
+    const updateHeight = () => {
+      if (!containerRef.current) return;
+      const rect = containerRef.current.getBoundingClientRect();
+      const parentContainer = containerRef.current.closest('.scroll-container');
+      if (parentContainer) {
+        const parentRect = parentContainer.getBoundingClientRect();
+        const availableHeight = parentRect.height - (rect.top - parentRect.top);
+        setContainerHeight(Math.max(400, availableHeight));
+      }
+    };
+    updateHeight();
+    window.addEventListener('resize', updateHeight);
+    let resizeObserver: ResizeObserver | null = null;
+    if (containerRef.current) {
+      const parentContainer = containerRef.current.closest('.scroll-container');
+      if (parentContainer) {
+        resizeObserver = new ResizeObserver(updateHeight);
+        resizeObserver.observe(parentContainer);
+      }
+    }
+    return () => {
+      window.removeEventListener('resize', updateHeight);
+      resizeObserver?.disconnect();
+    };
+  }, []);
+
+  // When the nearest cfi changes (progress moved or notes changed), scroll
+  // the virtualized list so the active note stays in view. We replace the
+  // per-item useScrollToItem (which forced 1000 layout reads) with a single
+  // virtuosoRef.scrollToIndex call.
+  useEffect(() => {
+    if (!nearestCfi || !flatItems.length) return;
+    if (nearestCfi === lastScrolledCfiRef.current) return;
+    const idx = flatItems.findIndex((row) => row.kind === 'note' && row.item.cfi === nearestCfi);
+    if (idx < 0) return;
+    const isEink = document.documentElement.getAttribute('data-eink') === 'true';
+    virtuosoRef.current?.scrollToIndex({
+      index: idx,
+      align: 'center',
+      behavior: isEink ? 'auto' : 'smooth',
+    });
+    lastScrolledCfiRef.current = nearestCfi;
+  }, [nearestCfi, flatItems]);
+
+  const renderItem = useCallback(
+    (index: number) => {
+      const row = flatItems[index];
+      if (!row) return null;
+      if (row.kind === 'group-header') {
+        return (
+          <div className='px-2 pt-2'>
+            <h3 className='content font-size-base line-clamp-1 px-2 font-normal'>
+              {row.group.label}
+            </h3>
+          </div>
+        );
+      }
+      return (
+        <ul className='px-2'>
+          <BooknoteItem
+            bookKey={bookKey}
+            item={row.item}
+            isNearest={row.item.cfi === nearestCfi}
+            onClick={handleBrowseBookNotes}
+          />
+        </ul>
+      );
+    },
+    [flatItems, bookKey, nearestCfi, handleBrowseBookNotes],
+  );
+
+  // Always mount the containerRef host so the height-measurement effect (and
+  // its ResizeObserver) can attach on first mount, even when starting from the
+  // empty state. Otherwise transitioning empty -> populated (e.g. after
+  // importing notes) would leave Virtuoso stuck at the initial 400px until a
+  // remount (tab switch) occurs.
+  const isEmpty = sortedGroups.length === 0;

  return (
-    <div className='rounded pt-2'>
-      {sortedGroups.length === 0 ? (
-        <div className='flex h-32 items-center justify-center text-gray-500'>
+    <div ref={containerRef} className='booknote-list rounded pt-2' role='tree'>
+      {isEmpty ? (
+        <div
+          className='flex items-center justify-center text-gray-500'
+          style={{ height: containerHeight }}
+        >
          <p className='font-size-sm text-center'>
            {type === 'annotation' ? _('No annotation yet') : _('No bookmark yet')}
          </p>
        </div>
      ) : (
-        <ul role='tree' className='px-2'>
-          {sortedGroups.map((group) => (
-            <li key={group.href} className='p-2'>
-              <h3 className='content font-size-base line-clamp-1 font-normal'>{group.label}</h3>
-              <ul>
-                {group.booknotes.map((item, index) => (
-                  <BooknoteItem
-                    key={`${index}-${item.cfi}`}
-                    bookKey={bookKey}
-                    item={item}
-                    isNearest={item.cfi === nearestCfi}
-                    onClick={handleBrowseBookNotes}
-                  />
-                ))}
-              </ul>
-            </li>
-          ))}
-        </ul>
+        <div
+          ref={osRootRef}
+          data-overlayscrollbars-initialize=''
+          style={{ height: containerHeight }}
+        >
+          <Virtuoso
+            ref={virtuosoRef}
+            scrollerRef={handleScrollerRef}
+            style={{ height: containerHeight }}
+            totalCount={flatItems.length}
+            computeItemKey={(index) => flatItems[index]?.key ?? index}
+            itemContent={renderItem}
+            overscan={500}
+          />
+        </div>
      )}
    </div>
  );