From 7bdd3ecdeeaa10b5a0f42b1fbbcc394eb53a960f Mon Sep 17 00:00:00 2001 From: loveheaven Date: Fri, 29 May 2026 16:25:38 +0800 Subject: [PATCH] perf(sidebar): virtualize BooknoteView and memoize derivations (#4352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../components/sidebar/BooknoteItem.tsx | 36 ++- .../components/sidebar/BooknoteView.tsx | 253 ++++++++++++++---- 2 files changed, 229 insertions(+), 60 deletions(-) diff --git a/apps/readest-app/src/app/reader/components/sidebar/BooknoteItem.tsx b/apps/readest-app/src/app/reader/components/sidebar/BooknoteItem.tsx index 2d9874ac..df07481c 100644 --- a/apps/readest-app/src/app/reader/components/sidebar/BooknoteItem.tsx +++ b/apps/readest-app/src/app/reader/components/sidebar/BooknoteItem.tsx @@ -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 = ({ 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 = ({ bookKey, item, isNearest, o
  • = ({ bookKey, item, isNearest, o
    )}
    @@ -238,9 +253,7 @@ const BooknoteItem: React.FC = ({ bookKey, item, isNearest, o {item.page ? _('p {{page}}' + ' · ', { page: item.page }) : ''} - - {dayjs(item.createdAt).fromNow()} - + {createdAtLabel}
    = ({ 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); diff --git a/apps/readest-app/src/app/reader/components/sidebar/BooknoteView.tsx b/apps/readest-app/src/app/reader/components/sidebar/BooknoteView.tsx index 1e217a61..afb54b21 100644 --- a/apps/readest-app/src/app/reader/components/sidebar/BooknoteView.tsx +++ b/apps/readest-app/src/app/reader/components/sidebar/BooknoteView.tsx @@ -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(() => { + 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(() => { + 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(null); + const osRootRef = useRef(null); + const virtuosoRef = useRef(null); + const [scroller, setScroller] = useState(null); + const [containerHeight, setContainerHeight] = useState(400); + const lastScrolledCfiRef = useRef(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 ( +
    +

    + {row.group.label} +

    +
    + ); + } + return ( +
      + +
    + ); + }, + [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 ( -
    - {sortedGroups.length === 0 ? ( -
    +
    + {isEmpty ? ( +

    {type === 'annotation' ? _('No annotation yet') : _('No bookmark yet')}

    ) : ( -
      - {sortedGroups.map((group) => ( -
    • -

      {group.label}

      -
        - {group.booknotes.map((item, index) => ( - - ))} -
      -
      • - ))} -
    +
    + flatItems[index]?.key ?? index} + itemContent={renderItem} + overscan={500} + /> +
    )}
    );