fix: Novel Screen Scroll Lag, Source Pagination Stalls, and Skeleton Overlap (#1971)

* fix

* Update pnpm-lock.yaml

* fix tests
This commit is contained in:
Patrick Loser
2026-08-16 04:04:17 +02:00
committed by GitHub
parent 990cd4f2eb
commit af2a224781
13 changed files with 569 additions and 167 deletions
+2 -1
View File
@@ -48,6 +48,7 @@
"@react-navigation/bottom-tabs": "^7.18.11",
"@react-navigation/native": "^7.3.11",
"@react-navigation/native-stack": "^7.18.3",
"@rozenite/react-navigation-plugin": "^2.0.0",
"cheerio": "1.0.0-rc.12",
"color": "^5.0.3",
"dayjs": "^1.11.21",
@@ -73,7 +74,7 @@
"lodash-es": "^4.18.1",
"protobufjs": "^8.7.1",
"react": "19.2.3",
"react-native": "0.86.0",
"react-native": "^0.86.0",
"react-native-device-info": "^15.0.2",
"react-native-draggable-flatlist": "^4.0.3",
"react-native-drawer-layout": "^4.2.8",
+304 -101
View File
File diff suppressed because it is too large Load Diff
-8
View File
@@ -54,14 +54,6 @@ export const extendNovelList = (
}
}
extension.push({
id: undefined,
cover: '',
name: '',
path: '__loading-row',
completeRow: 2,
});
return [...data, ...extension];
};
@@ -59,7 +59,6 @@ describe('NovelList helpers', () => {
'/two',
'__loading-filler-0',
'__loading-filler-1',
'__loading-row',
]);
});
});
@@ -326,6 +326,10 @@ describe('novelStore.chapterActions', () => {
);
const request = harness.actions.loadUpToBatch(1);
// The store action defers the batched read to the idle callback (a
// macrotask in the test environment); let that tick run so the mocked
// load starts before the generation is invalidated.
await new Promise(resolve => setTimeout(resolve, 0));
harness.chapterRequestCoordinator.invalidate();
resolveLoad();
await request;
@@ -1,5 +1,6 @@
import { ChapterFilterKey, ChapterOrderKey } from '@database/constants';
import { ChapterInfo } from '@database/types';
import { runWhenIdle } from '@utils/runWhenIdle';
import { createBootstrapService } from '../store-helper/bootstrapService';
import {
bookmarkChaptersAction,
@@ -154,6 +155,11 @@ export const createNovelStoreChapterActions = ({
continue;
}
// Same deferral as getNextChapterBatch: the batched reads are
// synchronous 1000-row materialisations that must not run mid-scroll.
await new Promise<void>(resolve => {
runWhenIdle(() => resolve(), 1000);
});
await bootstrapService.loadUpToBatch({
targetBatch: nextTarget,
novel: state.novel,
@@ -192,6 +198,13 @@ export const createNovelStoreChapterActions = ({
promise: Promise.resolve(),
};
request.promise = (async () => {
// The batched read materialises up to 1000 chapter rows on the JS
// thread synchronously; running it inside the scroll callback stalls
// the list at every batch boundary on novels with >1000 chapters
// (measured frame drops). Defer it until the thread is idle.
await new Promise<void>(resolve => {
runWhenIdle(() => resolve(), 1000);
});
const result = await bootstrapService.getNextChapterBatch({
novel: state.novel,
pages: state.pages,
+8 -2
View File
@@ -1,4 +1,4 @@
import { useEffect } from 'react';
import { useEffect, useRef } from 'react';
import { DefaultTheme, NavigationContainer } from '@react-navigation/native';
import { createNativeStackNavigator } from '@react-navigation/native-stack';
@@ -38,6 +38,7 @@ import { backgroundTasks } from '@services/backgroundTasks';
import ReaderStack from './ReaderStack';
import { LibraryContextProvider } from '@components/Context/LibraryContext';
import { UpdateContextProvider } from '@components/Context/UpdateContext';
import { useReactNavigationDevTools } from '@rozenite/react-navigation-plugin';
const Stack = createNativeStackNavigator<RootStackParamList>();
const MainNavigator = () => {
@@ -67,12 +68,17 @@ const MainNavigator = () => {
}
}, [isOnboarded, refreshPlugins, updateLibraryOnLaunch]);
const navigationRef = useRef(null);
// Enable React Navigation DevTools in development
useReactNavigationDevTools({ ref: navigationRef });
if (!isOnboarded) {
return <OnboardingScreen />;
}
return (
<NavigationContainer<RootStackParamList>
ref={navigationRef}
theme={{
colors: {
...DefaultTheme.colors,
@@ -85,6 +85,50 @@ const BrowseSourceScreen = ({ route, navigation }: BrowseSourceScreenProps) => {
const { bottom, right } = useSafeAreaInsets();
const filterSheetRef = useRef<BottomSheetModalMethods | null>(null);
const lastFetchDistanceRef = useRef(Infinity);
const lastContentHeightRef = useRef(0);
const lastOffsetYRef = useRef(-1);
const handleScroll = (event: {
nativeEvent: {
contentOffset: { y: number };
contentSize: { height: number };
layoutMeasurement: { height: number };
};
}) => {
const { contentOffset, contentSize, layoutMeasurement } = event.nativeEvent;
const distanceFromEnd =
contentSize.height - (contentOffset.y + layoutMeasurement.height);
const offsetMoved = contentOffset.y !== lastOffsetYRef.current;
lastOffsetYRef.current = contentOffset.y;
// A page appended (the end moved away). Re-arm the gate so the next user
// scroll toward the end fetches again. Content-size-only events (offset
// unchanged) never fetch on their own — that is what turned the old
// onEndReached flow into a loop on sources that never return an empty
// page (measured: htmlparser2 hot on every frame).
if (contentSize.height !== lastContentHeightRef.current) {
lastContentHeightRef.current = contentSize.height;
if (offsetMoved) {
lastFetchDistanceRef.current = Infinity;
}
}
if (!offsetMoved) {
return;
}
if (
distanceFromEnd < layoutMeasurement.height * 1.5 &&
distanceFromEnd < lastFetchDistanceRef.current
) {
lastFetchDistanceRef.current = distanceFromEnd;
if (searchText) {
if (hasNextSearchPage) {
searchNextPage();
}
} else if (hasNextPage) {
fetchNextPage();
}
}
};
return (
<SafeAreaView>
<SearchbarV2
@@ -121,6 +165,12 @@ const BrowseSourceScreen = ({ route, navigation }: BrowseSourceScreenProps) => {
<NovelList
data={novelList}
inSource
ListFooterComponent={
(hasNextPage && !searchText) ||
(hasNextSearchPage && Boolean(searchText)) ? (
<SourceScreenSkeletonLoading theme={theme} completeRow={2} />
) : null
}
renderItem={({ item }) => {
const inLibrary = novelInLibrary(pluginId, item.path);
@@ -148,16 +198,8 @@ const BrowseSourceScreen = ({ route, navigation }: BrowseSourceScreenProps) => {
/>
);
}}
onEndReached={() => {
if (searchText) {
if (hasNextSearchPage) {
searchNextPage();
}
} else if (hasNextPage) {
fetchNextPage();
}
}}
onEndReachedThreshold={1.5}
onScroll={handleScroll}
scrollEventThrottle={32}
/>
)}
{!showLatestNovels && filterValues && !searchText ? (
+119 -31
View File
@@ -23,44 +23,84 @@ export const useBrowseSource = (
const isScreenMounted = useRef(true);
// Serialize page fetches: onEndReached bursts (threshold 1.5 with a short
// list) can bump currentPage several times before the first request settles.
// Without a queue, that spawns concurrent out-of-order page fetches — each
// append re-renders the whole grid and remounts the "loading more" shimmer
// cells, which reads as stutter while scrolling the source list.
const fetchQueueRef = useRef<Promise<void>>(Promise.resolve());
// Bumped whenever the list is reset to page 1 (refetch, filters); queued
// fetches from an older generation are discarded so they cannot append
// stale pages onto a fresh list.
const generationRef = useRef(0);
// Highest page whose items are in the list, and highest page requested.
// `fetchNextPage` always asks for `lastLoaded + 1`, so a failed/empty
// fetch is retried on the next approach instead of being skipped past.
const lastLoadedPageRef = useRef(0);
const requestedPageRef = useRef(1);
const fetchNovels = useCallback(
async (page: number, filters?: FilterToValues<Filters>) => {
if (isScreenMounted.current === true) {
(page: number, filters?: FilterToValues<Filters>) => {
const run = async () => {
if (!isScreenMounted.current) {
return;
}
const generation = generationRef.current;
try {
const plugin = getPlugin(pluginId);
if (!plugin) {
throw new Error(`Unknown plugin: ${pluginId}`);
}
await plugin
.popularNovels(page, {
showLatestNovels,
filters,
})
.then(res => {
setNovels(prevState =>
page === 1 ? res : [...prevState, ...res],
);
if (!res.length) {
setHasNextPage(false);
}
})
.catch(e => {
setError(e.message);
setHasNextPage(false);
});
const res = await plugin.popularNovels(page, {
showLatestNovels,
filters,
});
if (generation !== generationRef.current) {
return;
}
setNovels(prevState => (page === 1 ? res : [...prevState, ...res]));
lastLoadedPageRef.current = Math.max(lastLoadedPageRef.current, page);
if (!res.length) {
setHasNextPage(false);
}
setFilterValues(plugin.filters);
} catch (err: unknown) {
setError(`${err}`);
if (generation !== generationRef.current) {
return;
}
setError(err instanceof Error ? err.message : `${err}`);
// A failed page fetch must not permanently end the list: sources
// rate-limit and transient network errors are common, and once
// `hasNextPage` flips false nothing ever retries (the screen stays
// mounted behind the reader, so the dead state persists even after
// navigating away and back). Un-request the failed page so the next
// approach retries it; only a successful empty response marks the end.
requestedPageRef.current = Math.min(
requestedPageRef.current,
page - 1,
);
} finally {
setIsLoading(false);
if (generation === generationRef.current) {
setIsLoading(false);
}
}
}
};
fetchQueueRef.current = fetchQueueRef.current.then(run, run);
return fetchQueueRef.current;
},
[pluginId, showLatestNovels],
);
const fetchNextPage = () => {
if (hasNextPage) setCurrentPage(prevState => prevState + 1);
if (!hasNextPage) {
return;
}
const next = lastLoadedPageRef.current + 1;
if (next <= requestedPageRef.current) {
return;
}
requestedPageRef.current = next;
void fetchNovels(next, selectedFilters);
};
/**
@@ -73,15 +113,17 @@ export const useBrowseSource = (
}, []);
useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
fetchNovels(currentPage, selectedFilters);
}, [fetchNovels, currentPage, selectedFilters]);
const refetchNovels = () => {
generationRef.current += 1;
setError('');
setIsLoading(true);
setNovels([]);
setCurrentPage(1);
lastLoadedPageRef.current = 0;
requestedPageRef.current = 1;
fetchNovels(1, selectedFilters);
};
@@ -91,8 +133,11 @@ export const useBrowseSource = (
);
const setFilters = (filters?: FilterToValues<Filters>) => {
generationRef.current += 1;
setIsLoading(true);
setCurrentPage(1);
lastLoadedPageRef.current = 0;
requestedPageRef.current = 1;
fetchNovels(1, filters);
setSelectedFilters(filters);
};
@@ -119,48 +164,89 @@ export const useSearchSource = (pluginId: string) => {
const [searchText, setSearchText] = useState('');
const searchSource = (searchTerm: string) => {
searchGenerationRef.current += 1;
setSearchResults([]);
setHasNextSearchPage(true);
setCurrentPage(1);
lastLoadedPageRef.current = 0;
requestedPageRef.current = 1;
setSearchText(searchTerm);
setIsSearching(true);
};
const isScreenMounted = useRef(true);
// Serialize page fetches — see useBrowseSource.fetchNovels for why.
const fetchQueueRef = useRef<Promise<void>>(Promise.resolve());
// Bumped whenever a new search starts; queued fetches from a previous
// search are discarded so they cannot append stale results onto a new one.
const searchGenerationRef = useRef(0);
// Same last-loaded/requested page tracking as useBrowseSource so failed
// pages are retried instead of skipped.
const lastLoadedPageRef = useRef(0);
const requestedPageRef = useRef(1);
const fetchNovels = useCallback(
async (localSearchText: string, page: number) => {
if (isScreenMounted.current === true) {
(localSearchText: string, page: number) => {
const run = async () => {
if (!isScreenMounted.current) {
return;
}
const generation = searchGenerationRef.current;
try {
const plugin = getPlugin(pluginId);
if (!plugin) {
throw new Error(`Unknown plugin: ${pluginId}`);
}
const res = await plugin.searchNovels(localSearchText, page);
if (generation !== searchGenerationRef.current) {
return;
}
setSearchResults(prevState =>
page === 1 ? res : [...prevState, ...res],
);
lastLoadedPageRef.current = Math.max(lastLoadedPageRef.current, page);
if (!res.length) {
setHasNextSearchPage(false);
}
} catch (err: unknown) {
if (generation !== searchGenerationRef.current) {
return;
}
setSearchError(`${err}`);
setHasNextSearchPage(false);
// See fetchNovels: a transient failure must not permanently end
// pagination; un-request the failed page so the next approach
// retries it.
requestedPageRef.current = Math.min(
requestedPageRef.current,
page - 1,
);
} finally {
setIsSearching(false);
if (generation === searchGenerationRef.current) {
setIsSearching(false);
}
}
}
};
fetchQueueRef.current = fetchQueueRef.current.then(run, run);
return fetchQueueRef.current;
},
[pluginId],
);
const searchNextPage = () => {
if (hasNextSearchPage) setCurrentPage(prevState => prevState + 1);
if (!hasNextSearchPage) {
return;
}
const next = lastLoadedPageRef.current + 1;
if (next <= requestedPageRef.current) {
return;
}
requestedPageRef.current = next;
void fetchNovels(searchText, next);
};
useEffect(() => {
if (searchText) {
// eslint-disable-next-line react-hooks/set-state-in-effect
fetchNovels(searchText, currentPage);
}
}, [currentPage, fetchNovels, searchText]);
@@ -170,6 +256,8 @@ export const useSearchSource = (pluginId: string) => {
setSearchResults([]);
setCurrentPage(1);
setHasNextSearchPage(true);
lastLoadedPageRef.current = 0;
requestedPageRef.current = 1;
}, []);
return {
@@ -77,7 +77,11 @@ const SourceScreenSkeletonLoading: React.FC<Props> = ({
return renderLoadingNovel(completeRow);
}
if (displayMode === DisplayModes.List) {
// completeRow === 2: a single full-width trailing row (rendered as the
// list footer), not a screen-filling grid.
if (completeRow === 2) {
items = [1];
} else if (displayMode === DisplayModes.List) {
items = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
} else {
for (let i = 1; i * pictureHeight < window.height - 100; i++) {
+43 -4
View File
@@ -1,4 +1,10 @@
import React, { createContext, useContext, useEffect, useMemo } from 'react';
import React, {
createContext,
useContext,
useEffect,
useMemo,
useRef,
} from 'react';
import { RouteProp } from '@react-navigation/native';
import { useStore } from 'zustand';
import { ReaderStackParamList } from '@navigators/types';
@@ -30,11 +36,24 @@ const NovelStoreContext = createContext<NovelStoreApi | null>(null);
const NovelLayoutContext = createContext<NovelLayout | null>(null);
export function NovelContextProvider({ children, route }: Props) {
const params = (route.params ?? {}) as {
novel?: { path: string; pluginId: string };
path?: string;
pluginId?: string;
id?: number;
};
const initialNovel =
'id' in route.params ? (route.params as NovelInfo) : undefined;
params.id !== undefined ? (params as unknown as NovelInfo) : undefined;
const { path, pluginId } =
'novel' in route.params ? route.params.novel : route.params;
// The ReaderStack's route can carry Chapter-shaped params ({ novel,
// chapter }) or a stale first-mount params object (ReaderStack.tsx falls
// back to `useRef`'d params while a transition leaves `route.params`
// momentarily nullish). Guard both the key and the value: a malformed
// params object must degrade to the path-based bootstrap instead of
// throwing while reading `.path` off `undefined` (measured: "Cannot read
// property 'path' of undefined" when opening a novel from the library
// after a plugin open).
const { path = '', pluginId = '' } = params.novel ?? params;
const { switchNovelToLibrary } = useLibraryContext();
const { defaultChapterSort } = useAppSettings();
@@ -58,6 +77,26 @@ export function NovelContextProvider({ children, route }: Props) {
}
}, [novelStore]);
const gcHandleRef = useRef<{ cancel: () => void } | null>(null);
useEffect(() => {
// A new novel screen mounted; drop any reclaim still pending from the
// previous one so the collection cannot pause this screen's bootstrap.
gcHandleRef.current?.cancel();
gcHandleRef.current = null;
return () => {
// The store (full chapter list) and the fetch/parse garbage from an
// uncached bootstrap are released here. Hermes' concurrent GC only runs
// on allocation pressure, so without a nudge the dead objects sit in the
// heap and the heap cap ratchets up with every novel opened — the GC
// cost of scrolling then grows until the app is restarted (measured:
// +5 MB of collectible garbage per uncached open; forced GC dropped the
// cap 54.5 MB -> 50.3 MB). Reclaim after the pop transition finishes
// (~300 ms) so the collection cannot hitch the animation.
const timer = setTimeout(() => globalThis.gc?.(), 600);
gcHandleRef.current = { cancel: () => clearTimeout(timer) };
};
}, []);
const { bottom, top } = useSafeAreaInsets();
const layoutValue = useMemo(
@@ -5,7 +5,13 @@ import { useCallback, useMemo, useRef, useState } from 'react';
import { ChapterInfo, NovelInfo } from '@database/types';
import { useAppSettings, useDownload, useTheme } from '@hooks/persisted';
import { getString } from '@i18n/translations';
import { RefreshControl, useWindowDimensions, View } from 'react-native';
import {
type NativeScrollEvent,
type NativeSyntheticEvent,
RefreshControl,
useWindowDimensions,
View,
} from 'react-native';
import {
runOnJS,
SharedValue,
@@ -19,8 +25,7 @@ import PageNavigationBottomSheet from './PageNavigationBottomSheet';
import * as Haptics from 'expo-haptics';
import { ChapterListSkeleton } from '@components/Skeleton/Skeleton';
import { BottomSheetModalMethods } from '@gorhom/bottom-sheet/lib/typescript/types';
import { LegendListRef } from '@legendapp/list/react-native';
import { AnimatedLegendList } from '@legendapp/list/reanimated';
import { LegendList, LegendListRef } from '@legendapp/list/react-native';
import PagePaginationControl from './PagePaginationControl';
import { useNovelActions, useNovelValue } from '../NovelContext';
import { UseBooleanReturnType } from '@hooks/index';
@@ -166,7 +171,12 @@ const NovelScreenList = ({
[headerOpacity, lastRead, screenHeight, useFabForContinueReading],
);
const listSharedValues = useMemo(() => ({ scrollOffset }), [scrollOffset]);
const scrollHandler = useCallback(
(event: NativeSyntheticEvent<NativeScrollEvent>) => {
scrollOffset.value = event.nativeEvent.contentOffset.y;
},
[scrollOffset],
);
// --- Stable callbacks ---
@@ -411,7 +421,7 @@ const NovelScreenList = ({
return (
<>
<AnimatedLegendList
<LegendList
ref={listRef}
estimatedItemSize={64}
data={chapters}
@@ -424,7 +434,7 @@ const NovelScreenList = ({
refreshControl={refreshControlElement}
onEndReached={getNextChapterBatch}
onEndReachedThreshold={6}
sharedValues={listSharedValues}
onScroll={scrollHandler}
//drawDistance={1000}
ListHeaderComponent={listHeader}
/>
@@ -45,12 +45,12 @@ jest.mock('react-native-safe-area-context', () => ({
useSafeAreaInsets: () => ({ top: 0, bottom: 0 }),
}));
jest.mock('@legendapp/list/reanimated', () => {
jest.mock('@legendapp/list/react-native', () => {
const React = require('react');
const { View } = require('react-native');
return {
AnimatedLegendList: ({ data, renderItem, ListHeaderComponent }: any) =>
LegendList: ({ data, renderItem, ListHeaderComponent }: any) =>
React.createElement(
View,
null,
@@ -63,6 +63,7 @@ jest.mock('@legendapp/list/reanimated', () => {
),
),
),
LegendListRef: {},
};
});