Fix pagination (#11448)

Co-authored-by: Eric Bailey <git@esb.lol>

oxlint-suppressions.json

@@ -1191,7 +1191,7 @@
       "count": 2
     },
     "typescript/no-floating-promises": {
-      "count": 2
+      "count": 1
     },
     "typescript/no-unsafe-member-access": {
       "count": 2
@@ -1226,7 +1226,7 @@
       "count": 3
     },
     "typescript/no-floating-promises": {
-      "count": 3
+      "count": 2
     },
     "typescript/no-unsafe-member-access": {
       "count": 3

src/lib/api/feed/custom.test.ts

@@ -0,0 +1,40 @@
+import {type Client} from '@atproto/lex'
+
+import {CustomFeedAPI} from './custom'
+
+jest.mock('#/state/preferences/languages', () => ({
+  getAppLanguageAsContentLanguage: () => '',
+  getContentLanguages: () => [],
+}))
+
+jest.mock('./utils', () => ({
+  createBskyTopicsHeader: () => ({}),
+  isBlueskyOwnedFeed: () => false,
+}))
+
+describe('CustomFeedAPI', () => {
+  it('preserves the cursor from an empty logged-out fallback page', async () => {
+    const originalFetch = global.fetch
+    const fetchMock: jest.MockedFunction<typeof fetch> = jest
+      .fn()
+      .mockResolvedValueOnce(Response.json({feed: []}))
+      .mockResolvedValueOnce(Response.json({feed: [], cursor: 'next'}))
+    global.fetch = fetchMock
+    const api = new CustomFeedAPI({
+      client: {did: undefined} as unknown as Client,
+      feedParams: {
+        feed: 'at://did:example:feed/app.bsky.feed.generator/test',
+      },
+    })
+
+    try {
+      await expect(api.fetch({cursor: undefined, limit: 10})).resolves.toEqual({
+        cursor: 'next',
+        feed: [],
+      })
+      expect(fetchMock).toHaveBeenCalledTimes(2)
+    } finally {
+      global.fetch = originalFetch
+    }
+  })
+})

src/lib/api/feed/custom.ts

@@ -96,7 +96,7 @@ export class CustomFeedAPI implements FeedAPI {
     const feed =
       data.feed.length > limit ? data.feed.slice(0, limit) : data.feed
     return {
-      cursor: feed.length ? data.cursor : undefined,
+      cursor: data.cursor,
       feed,
     }
   }
@@ -154,11 +154,7 @@ async function loggedOutFetch(
 
   // no data, try again with language headers removed
   data = await getFeedOrNull(params, '')
-  if (data?.feed?.length) {
-    return data
-  }
-
-  return null
+  return data
 }
 
 /**

src/lib/api/feed/likes.ts

@@ -48,10 +48,8 @@ export class LikesFeedAPI implements FeedAPI {
       cursor,
       limit,
     })
-    // HACKFIX: the API incorrectly returns a cursor when there are no items -sfn
-    const isEmptyPage = data.feed.length === 0
     return {
-      cursor: isEmptyPage ? undefined : data.cursor,
+      cursor: data.cursor,
       feed: data.feed,
     }
   }

src/lib/api/feed/merge.test.ts

@@ -0,0 +1,106 @@
+import {type Client} from '@atproto/lex'
+
+import {app} from '#/lexicons'
+import {MergeFeedAPI} from './merge'
+
+const post = {} as app.bsky.feed.defs.FeedViewPost
+
+describe('MergeFeedAPI', () => {
+  it('drains a terminal following queue without restarting the source', async () => {
+    const api = new MergeFeedAPI({
+      client: {} as Client,
+      feedParams: {},
+      feedTuners: [],
+    })
+    api.reset()
+    api.following.queue = [post, post, post]
+    api.following.hasMore = false
+
+    const first = await api.fetch({cursor: 'started', limit: 2})
+    const second = await api.fetch({cursor: first.cursor, limit: 2})
+
+    expect(first.feed).toHaveLength(2)
+    expect(first.cursor).toBeDefined()
+    expect(second.feed).toHaveLength(1)
+    expect(second.cursor).toBeUndefined()
+  })
+
+  it('stops when the following source repeats its cursor', async () => {
+    const call = jest
+      .fn()
+      .mockResolvedValueOnce({feed: [], cursor: 'a'})
+      .mockResolvedValueOnce({feed: [], cursor: 'a'})
+    const api = new MergeFeedAPI({
+      client: {call} as unknown as Client,
+      feedParams: {},
+      feedTuners: [],
+    })
+
+    const first = await api.fetch({cursor: undefined, limit: 1})
+    const second = await api.fetch({cursor: first.cursor, limit: 1})
+
+    expect(first.cursor).toBeDefined()
+    expect(second.cursor).toBeUndefined()
+    expect(call).toHaveBeenCalledTimes(2)
+  })
+
+  it('stops when the following source returns an empty cursor', async () => {
+    const call = jest.fn().mockResolvedValue({feed: [], cursor: ''})
+    const api = new MergeFeedAPI({
+      client: {call} as unknown as Client,
+      feedParams: {},
+      feedTuners: [],
+    })
+
+    const result = await api.fetch({cursor: undefined, limit: 1})
+
+    expect(result.cursor).toBeUndefined()
+    expect(call).toHaveBeenCalledTimes(1)
+  })
+
+  it('stops when the following source cycles to an earlier cursor', async () => {
+    const call = jest
+      .fn()
+      .mockResolvedValueOnce({feed: [], cursor: 'a'})
+      .mockResolvedValueOnce({feed: [], cursor: 'b'})
+      .mockResolvedValueOnce({feed: [], cursor: 'a'})
+    const api = new MergeFeedAPI({
+      client: {call} as unknown as Client,
+      feedParams: {},
+      feedTuners: [],
+    })
+
+    const first = await api.fetch({cursor: undefined, limit: 1})
+    const second = await api.fetch({cursor: first.cursor, limit: 1})
+    const third = await api.fetch({cursor: second.cursor, limit: 1})
+
+    expect(third.cursor).toBeUndefined()
+    expect(call).toHaveBeenCalledTimes(3)
+  })
+
+  it('drains a terminal custom-feed queue without restarting the source', async () => {
+    const call = jest.fn()
+    const api = new MergeFeedAPI({
+      client: {call} as unknown as Client,
+      feedParams: {
+        mergeFeedEnabled: true,
+        mergeFeedSources: [
+          'at://did:example:feed/app.bsky.feed.generator/test',
+        ],
+      },
+      feedTuners: [],
+    })
+    api.reset()
+    api.following.hasMore = false
+    api.customFeeds[0].queue = [post, post, post]
+    api.customFeeds[0].hasMore = false
+
+    const first = await api.fetch({cursor: 'started', limit: 2})
+    const second = await api.fetch({cursor: first.cursor, limit: 2})
+
+    expect(first.feed).toHaveLength(2)
+    expect(second.feed).toHaveLength(1)
+    expect(second.cursor).toBeUndefined()
+    expect(call).not.toHaveBeenCalled()
+  })
+})

src/lib/api/feed/merge.ts

@@ -108,8 +108,8 @@ export class MergeFeedAPI implements FeedAPI {
 
     const promises = []
 
-    // always keep following topped up
-    if (this.following.numReady < limit) {
+    // always keep following topped up while the source has another page
+    if (this.following.hasMore && this.following.numReady < limit) {
       await this.following.fetchNext(60)
     }
 
@@ -125,7 +125,7 @@ export class MergeFeedAPI implements FeedAPI {
       !this.following.hasMore && this.following.numReady < limit
     if (this.params.mergeFeedEnabled || outOfFollows) {
       for (const feed of feeds) {
-        if (feed.numReady < 5) {
+        if (feed.hasMore && feed.numReady < 5) {
           promises.push(feed.fetchNext(10))
         }
       }
@@ -145,8 +145,12 @@ export class MergeFeedAPI implements FeedAPI {
       }
     }
 
+    const hasMore =
+      this.following.hasMore ||
+      this.following.numReady > 0 ||
+      this.customFeeds.some(feed => feed.hasMore || feed.numReady > 0)
     return {
-      cursor: String(this.itemCursor),
+      cursor: hasMore ? String(this.itemCursor) : undefined,
       feed: posts,
     }
   }
@@ -155,8 +159,8 @@ export class MergeFeedAPI implements FeedAPI {
     const i = this.itemCursor++
     const candidateFeeds = this.customFeeds.filter(f => f.numReady > 0)
     const canSample = candidateFeeds.length > 0
-    const hasFollows = this.following.hasMore
     const hasFollowsReady = this.following.numReady > 0
+    const hasFollows = this.following.hasMore || hasFollowsReady
 
     // this condition establishes the frequency that custom feeds are woven into follows
     const shouldSample =
@@ -187,6 +191,7 @@ class MergeFeedSource {
   feedTuners: FeedTunerFn[]
   sourceInfo: ReasonFeedSource | undefined
   cursor: string | undefined = undefined
+  seenCursors = new Set<string>()
   queue: app.bsky.feed.defs.FeedViewPost[] = []
   hasMore = true
 
@@ -221,11 +226,16 @@ class MergeFeedSource {
     const page = await this._getFeed(this.cursor, n)
     if (page) {
       this.cursor = page.cursor
-      if (page.feed.length) {
-        this.queue = this.queue.concat(page.feed)
+      const cursor = this.cursor
+      if (cursor) {
+        this.hasMore = !this.seenCursors.has(cursor)
+        this.seenCursors.add(cursor)
       } else {
         this.hasMore = false
       }
+      if (page.feed.length) {
+        this.queue = this.queue.concat(page.feed)
+      }
     } else {
       this.hasMore = false
     }

src/state/queries/notifications/feed.ts

@@ -16,7 +16,7 @@
  * 3. Don't call this query's `refetch()` if you're trying to sync latest; call `checkUnread()` instead.
  */
 
-import {useCallback, useEffect, useMemo, useRef} from 'react'
+import {useCallback, useMemo, useRef} from 'react'
 import {AtUri} from '@atproto/syntax'
 import {moderatePost} from '@bsky/sdk/moderation'
 import {
@@ -37,6 +37,7 @@ import {
   didOrHandleUriMatches,
   embedViewRecordToPostView,
   getEmbeddedPost,
+  useAutoPagination,
 } from '../util'
 import {type FeedPage} from './types'
 import {useUnreadNotificationsApi} from './unread'
@@ -220,54 +221,9 @@ export function useNotificationFeedQuery(opts: {
     ),
   })
 
-  // The server may end up returning an empty page, a page with too few items,
-  // or a page with items that end up getting filtered out. When we fetch pages,
-  // we'll keep track of how many items we actually hope to see. If the server
-  // doesn't return enough items, we're going to continue asking for more items.
-  const lastItemCount = useRef(0)
-  const wantedItemCount = useRef(0)
-  const autoPaginationAttemptCount = useRef(0)
-  useEffect(() => {
-    const {data, isLoading, isRefetching, isFetchingNextPage, hasNextPage} =
-      query
-    // Count the items that we already have.
-    let itemCount = 0
-    for (const page of data?.pages || []) {
-      itemCount += page.items.length
-    }
-
-    // If items got truncated, reset the state we're tracking below.
-    if (itemCount !== lastItemCount.current) {
-      if (itemCount < lastItemCount.current) {
-        wantedItemCount.current = itemCount
-      }
-      lastItemCount.current = itemCount
-    }
-
-    // Now track how many items we really want, and fetch more if needed.
-    if (isLoading || isRefetching) {
-      // During the initial fetch, we want to get an entire page's worth of items.
-      wantedItemCount.current = PAGE_SIZE
-    } else if (isFetchingNextPage) {
-      if (itemCount > wantedItemCount.current) {
-        // We have more items than wantedItemCount, so wantedItemCount must be out of date.
-        // Some other code must have called fetchNextPage(), for example, from onEndReached.
-        // Adjust the wantedItemCount to reflect that we want one more full page of items.
-        wantedItemCount.current = itemCount + PAGE_SIZE
-      }
-    } else if (hasNextPage) {
-      // At this point we're not fetching anymore, so it's time to make a decision.
-      // If we didn't receive enough items from the server, paginate again until we do.
-      if (itemCount < wantedItemCount.current) {
-        autoPaginationAttemptCount.current++
-        if (autoPaginationAttemptCount.current < 50 /* failsafe */) {
-          query.fetchNextPage()
-        }
-      } else {
-        autoPaginationAttemptCount.current = 0
-      }
-    }
-  }, [query])
+  const itemCount =
+    query.data?.pages.reduce((count, page) => count + page.items.length, 0) ?? 0
+  useAutoPagination(query, itemCount, PAGE_SIZE)
 
   return query
 }

src/state/queries/post-feed.ts

@@ -1,4 +1,4 @@
-import {useCallback, useEffect, useMemo, useRef} from 'react'
+import {useCallback, useMemo, useRef} from 'react'
 import {AppState} from 'react-native'
 import {type Client} from '@atproto/lex'
 import {type AtIdentifierString, AtUri, type AtUriString} from '@atproto/syntax'
@@ -46,6 +46,7 @@ import {
   didOrHandleUriMatches,
   embedViewRecordToPostView,
   getEmbeddedPost,
+  useAutoPagination,
 } from './util'
 
 type ActorDid = string
@@ -368,56 +369,13 @@ export function usePostFeedQuery(
     ),
   })
 
-  // The server may end up returning an empty page, a page with too few items,
-  // or a page with items that end up getting filtered out. When we fetch pages,
-  // we'll keep track of how many items we actually hope to see. If the server
-  // doesn't return enough items, we're going to continue asking for more items.
-  const lastItemCount = useRef(0)
-  const wantedItemCount = useRef(0)
-  const autoPaginationAttemptCount = useRef(0)
-  useEffect(() => {
-    const {data, isLoading, isRefetching, isFetchingNextPage, hasNextPage} =
-      query
-    // Count the items that we already have.
-    let itemCount = 0
-    for (const page of data?.pages || []) {
-      for (const slice of page.slices) {
-        itemCount += slice.items.length
-      }
+  let itemCount = 0
+  for (const page of query.data?.pages || []) {
+    for (const slice of page.slices) {
+      itemCount += slice.items.length
     }
-
-    // If items got truncated, reset the state we're tracking below.
-    if (itemCount !== lastItemCount.current) {
-      if (itemCount < lastItemCount.current) {
-        wantedItemCount.current = itemCount
-      }
-      lastItemCount.current = itemCount
-    }
-
-    // Now track how many items we really want, and fetch more if needed.
-    if (isLoading || isRefetching) {
-      // During the initial fetch, we want to get an entire page's worth of items.
-      wantedItemCount.current = MIN_POSTS
-    } else if (isFetchingNextPage) {
-      if (itemCount > wantedItemCount.current) {
-        // We have more items than wantedItemCount, so wantedItemCount must be out of date.
-        // Some other code must have called fetchNextPage(), for example, from onEndReached.
-        // Adjust the wantedItemCount to reflect that we want one more full page of items.
-        wantedItemCount.current = itemCount + MIN_POSTS
-      }
-    } else if (hasNextPage) {
-      // At this point we're not fetching anymore, so it's time to make a decision.
-      // If we didn't receive enough items from the server, paginate again until we do.
-      if (itemCount < wantedItemCount.current) {
-        autoPaginationAttemptCount.current++
-        if (autoPaginationAttemptCount.current < 50 /* failsafe */) {
-          query.fetchNextPage()
-        }
-      } else {
-        autoPaginationAttemptCount.current = 0
-      }
-    }
-  }, [query])
+  }
+  useAutoPagination(query, itemCount, MIN_POSTS)
 
   return query
 }

src/state/queries/util.test.tsx

@@ -0,0 +1,100 @@
+import {renderHook} from '@testing-library/react-native'
+
+import {useAutoPagination} from './util'
+
+function query(overrides: Record<string, unknown> = {}) {
+  return {
+    data: {pageParams: [undefined]},
+    isLoading: false,
+    isRefetching: false,
+    isFetchingNextPage: false,
+    hasNextPage: true,
+    fetchNextPage: jest.fn().mockResolvedValue(undefined),
+    ...overrides,
+  }
+}
+
+describe('useAutoPagination', () => {
+  it('fetches another page when visible items are missing', () => {
+    const value = query()
+
+    renderHook(() => useAutoPagination(value, 0, 10))
+
+    expect(value.fetchNextPage).toHaveBeenCalledTimes(1)
+  })
+
+  it('stops when the requested number of items is visible', () => {
+    const value = query()
+
+    renderHook(() => useAutoPagination(value, 10, 10))
+
+    expect(value.fetchNextPage).not.toHaveBeenCalled()
+  })
+
+  it('stops when the server repeats a cursor', () => {
+    const value = query({data: {pageParams: [undefined, 'a', 'a']}})
+
+    renderHook(() => useAutoPagination(value, 0, 10))
+
+    expect(value.fetchNextPage).not.toHaveBeenCalled()
+  })
+
+  it('stops when structured page params repeat a cursor', () => {
+    const value = query({
+      data: {pageParams: [undefined, {cursor: 'a'}, {cursor: 'a'}]},
+    })
+
+    renderHook(() => useAutoPagination(value, 0, 10))
+
+    expect(value.fetchNextPage).not.toHaveBeenCalled()
+  })
+
+  it('stops when a cursor repeats non-adjacently', () => {
+    const value = query({data: {pageParams: [undefined, 'a', 'b', 'a']}})
+
+    renderHook(() => useAutoPagination(value, 0, 10))
+
+    expect(value.fetchNextPage).not.toHaveBeenCalled()
+  })
+
+  it('fills one page after switching to a smaller cached query', () => {
+    const first = query({hasNextPage: false})
+    const second = query()
+    let value = first
+    let itemCount = 10
+    const {rerender} = renderHook(() => useAutoPagination(value, itemCount, 10))
+
+    value = second
+    itemCount = 5
+    rerender(undefined)
+
+    expect(second.fetchNextPage).toHaveBeenCalledTimes(1)
+  })
+
+  it('resets the attempt limit after switching to cached data with the same item count', () => {
+    const fetchNextPage = jest.fn().mockResolvedValue(undefined)
+    const data = {pageParams: [undefined]}
+    let value = query({fetchNextPage, data})
+    const itemCount = 0
+    const {rerender} = renderHook(() => useAutoPagination(value, itemCount, 10))
+
+    for (let i = 1; i < 50; i++) {
+      value = query({
+        fetchNextPage,
+        data,
+      })
+      rerender(undefined)
+    }
+    expect(fetchNextPage).toHaveBeenCalledTimes(49)
+
+    const second = query({
+      data: {
+        pageParams: Array.from({length: 51}, (_, i) => `new-cursor-${i}`),
+      },
+    })
+    value = second
+    rerender(undefined)
+
+    expect(second.fetchNextPage).toHaveBeenCalledTimes(1)
+  })
+})

src/state/queries/util.ts

@@ -1,3 +1,4 @@
+import {useEffect, useRef} from 'react'
 import {type AtUri} from '@atproto/syntax'
 import {
   type InfiniteData,
@@ -8,6 +9,82 @@ import {
 import {app} from '#/lexicons'
 import * as bsky from '#/types/bsky'
 
+type AutoPaginationQuery = {
+  data?: {pageParams: unknown[]}
+  isLoading: boolean
+  isRefetching: boolean
+  isFetchingNextPage: boolean
+  hasNextPage: boolean
+  fetchNextPage: () => Promise<unknown>
+}
+
+export function useAutoPagination(
+  query: AutoPaginationQuery,
+  itemCount: number,
+  pageSize: number,
+) {
+  const lastItemCount = useRef(0)
+  const lastPageParams = useRef(query.data?.pageParams)
+  const wantedItemCount = useRef(pageSize)
+  const attemptCount = useRef(0)
+
+  useEffect(() => {
+    const cursorOf = (param: unknown) =>
+      param && typeof param === 'object' && 'cursor' in param
+        ? param.cursor
+        : param
+    const pageParams = query.data?.pageParams
+    const previousPageParams = lastPageParams.current
+    const continuedPagination =
+      pageParams &&
+      previousPageParams &&
+      pageParams.length > previousPageParams.length &&
+      previousPageParams.every((param, index) =>
+        Object.is(cursorOf(param), cursorOf(pageParams[index])),
+      )
+    if (
+      pageParams !== previousPageParams &&
+      previousPageParams &&
+      !continuedPagination
+    ) {
+      wantedItemCount.current = pageSize
+      attemptCount.current = 0
+    }
+    lastPageParams.current = pageParams
+
+    if (itemCount !== lastItemCount.current) {
+      attemptCount.current = 0
+      if (itemCount < lastItemCount.current) {
+        wantedItemCount.current = Math.max(itemCount, pageSize)
+      }
+      lastItemCount.current = itemCount
+    }
+
+    if (query.isLoading || query.isRefetching) {
+      wantedItemCount.current = pageSize
+      attemptCount.current = 0
+    } else if (query.isFetchingNextPage) {
+      if (itemCount > wantedItemCount.current) {
+        wantedItemCount.current = itemCount + pageSize
+      }
+    } else if (query.hasNextPage) {
+      if (itemCount < wantedItemCount.current) {
+        const currentCursor = cursorOf(pageParams?.at(-1))
+        const repeatedCursor = pageParams
+          ?.slice(0, -1)
+          .some(param => Object.is(cursorOf(param), currentCursor))
+        if (repeatedCursor) return
+        attemptCount.current++
+        if (attemptCount.current < 50) {
+          void query.fetchNextPage()
+        }
+      } else {
+        attemptCount.current = 0
+      }
+    }
+  }, [itemCount, pageSize, query])
+}
+
 export type StructuredQueryKey<T extends Record<string, unknown>> = readonly [
   string,
   T,

src/view/com/feeds/ProfileFeedgens.tsx

@@ -79,7 +79,7 @@ export function ProfileFeedgens({
     error,
     refetch,
   } = useProfileFeedgensQuery(did, opts)
-  const isEmpty = !isPending && !data?.pages[0]?.feeds.length
+  const isEmpty = !isPending && !data?.pages.some(page => page.feeds.length)
   const {data: preferences} = usePreferencesQuery()
   const navigation = useNavigation()
   const {currentAccount} = useSession()

src/view/com/lists/ListMembers.tsx

@@ -90,7 +90,7 @@ export function ListMembers({
     hasNextPage,
     isFetchingNextPage,
   } = useListMembersQuery(list)
-  const isEmpty = !isFetching && !data?.pages[0].items.length
+  const isEmpty = !isFetching && !data?.pages.some(page => page.items.length)
   const isOwner =
     currentAccount && data?.pages[0].list.creator.did === currentAccount.did
 

src/view/com/lists/ProfileLists.tsx

@@ -79,7 +79,7 @@ export function ProfileLists({
     error,
     refetch,
   } = useProfileListsQuery(did, opts)
-  const isEmpty = !isPending && !data?.pages[0]?.lists.length
+  const isEmpty = !isPending && !data?.pages.some(page => page.lists.length)
   const {data: preferences} = usePreferencesQuery()
   const navigation = useNavigation()
   const {currentAccount} = useSession()

src/view/screens/ModerationBlockedAccounts.tsx

@@ -36,7 +36,7 @@ export function ModerationBlockedAccounts({}: Props) {
     fetchNextPage,
     isFetchingNextPage,
   } = useMyBlockedAccountsQuery()
-  const isEmpty = !isFetching && !data?.pages[0]?.blocks.length
+  const isEmpty = !isFetching && !data?.pages.some(page => page.blocks.length)
   const profiles = useMemo(() => {
     if (data?.pages) {
       return data.pages.flatMap(page => page.blocks)

src/view/screens/ModerationMutedAccounts.tsx

@@ -36,7 +36,7 @@ export function ModerationMutedAccounts({}: Props) {
     fetchNextPage,
     isFetchingNextPage,
   } = useMyMutedAccountsQuery()
-  const isEmpty = !isFetching && !data?.pages[0]?.mutes.length
+  const isEmpty = !isFetching && !data?.pages.some(page => page.mutes.length)
   const profiles = useMemo(() => {
     if (data?.pages) {
       return data.pages.flatMap(page => page.mutes)