diff --git a/packages/liveblocks-react/src/__tests__/useInboxNotifications.test.tsx b/packages/liveblocks-react/src/__tests__/useInboxNotifications.test.tsx index c054e311dc..43090ad491 100644 --- a/packages/liveblocks-react/src/__tests__/useInboxNotifications.test.tsx +++ b/packages/liveblocks-react/src/__tests__/useInboxNotifications.test.tsx @@ -10,7 +10,7 @@ import { waitFor, } from "@testing-library/react"; import { setupServer } from "msw/node"; -import { Suspense } from "react"; +import { Suspense, useEffect } from "react"; import { ErrorBoundary, type FallbackProps } from "react-error-boundary"; import { @@ -1484,4 +1484,159 @@ describe("useInboxNotifications: pagination", () => { unmount(); }); + + test("should auto-paginate all pages when fetchMore is called from useEffect", async () => { + const roomId = nanoid(); + + const thread1 = dummyThreadData({ roomId, createdAt: new Date("2021-01-01") }); // prettier-ignore + const thread2 = dummyThreadData({ roomId, createdAt: new Date("2021-01-02") }); // prettier-ignore + const thread3 = dummyThreadData({ roomId, createdAt: new Date("2021-01-03") }); // prettier-ignore + + const notificationsPage1 = [ + dummyThreadInboxNotificationData({ + roomId, + threadId: thread1.id, + notifiedAt: new Date("2021-01-03"), + }), + ]; + const notificationsPage2 = [ + dummyThreadInboxNotificationData({ + roomId, + threadId: thread2.id, + notifiedAt: new Date("2021-01-02"), + }), + ]; + const notificationsPage3 = [ + dummyThreadInboxNotificationData({ + roomId, + threadId: thread3.id, + notifiedAt: new Date("2021-01-01"), + }), + ]; + + let requestCount = 0; + + server.use( + mockGetInboxNotifications(async (req, res, ctx) => { + requestCount++; + const url = new URL(req.url); + const cursor = url.searchParams.get("cursor"); + + if (cursor === "cursor-1") { + return res( + ctx.json({ + threads: [thread2], + inboxNotifications: notificationsPage2, + subscriptions: [dummySubscriptionData({ subjectId: thread2.id })], + groups: [], + meta: { + requestedAt: new Date().toISOString(), + nextCursor: "cursor-2", + }, + }) + ); + } else if (cursor === "cursor-2") { + return res( + ctx.json({ + threads: [thread3], + inboxNotifications: notificationsPage3, + subscriptions: [dummySubscriptionData({ subjectId: thread3.id })], + groups: [], + meta: { + requestedAt: new Date().toISOString(), + nextCursor: null, + }, + }) + ); + } else { + return res( + ctx.json({ + threads: [thread1], + inboxNotifications: notificationsPage1, + subscriptions: [dummySubscriptionData({ subjectId: thread1.id })], + groups: [], + meta: { + requestedAt: new Date().toISOString(), + nextCursor: "cursor-1", + }, + }) + ); + } + }), + mockGetInboxNotificationsDelta(async (_req, res, ctx) => { + return res( + ctx.json({ + threads: [], + inboxNotifications: [], + subscriptions: [], + deletedThreads: [], + deletedInboxNotifications: [], + deletedSubscriptions: [], + meta: { + requestedAt: new Date().toISOString(), + }, + }) + ); + }) + ); + + const { + liveblocks: { + suspense: { LiveblocksProvider, useInboxNotifications }, + }, + } = createContextsForTest(); + + // Component that auto-paginates using useEffect, matching the reported + // user pattern that triggered the microtask ordering bug. + function AutoPaginator({ + onResult, + }: { + onResult: (result: { count: number; hasFetchedAll: boolean }) => void; + }) { + const result = useInboxNotifications(); + const { isFetchingMore, hasFetchedAll, fetchMore } = result; + + useEffect(() => { + onResult({ + count: result.inboxNotifications.length, + hasFetchedAll, + }); + }, [result.inboxNotifications.length, hasFetchedAll, onResult]); + + useEffect(() => { + if (!hasFetchedAll && !isFetchingMore) { + fetchMore(); + } + }, [hasFetchedAll, isFetchingMore, fetchMore]); + + return null; + } + + let latestResult = { count: 0, hasFetchedAll: false }; + const handleResult = (r: { count: number; hasFetchedAll: boolean }) => { + latestResult = r; + }; + + const { unmount } = render( + + Loading}> + + + + ); + + // All 3 pages should be fetched automatically without getting stuck. + // Before the fix, this would stop at 2 items (page 1 + page 2) because + // .finally() clearing #pendingFetchMore ran in a later microtask than + // React's re-render flush, causing the 3rd fetchMore() call to be skipped. + await waitFor(() => expect(latestResult.hasFetchedAll).toBe(true), { + timeout: 5000, + }); + expect(latestResult.count).toBe(3); + + // Initial page + 2 fetchMore calls = 3 requests total + expect(requestCount).toBe(3); + + unmount(); + }); }); diff --git a/packages/liveblocks-react/src/umbrella-store.ts b/packages/liveblocks-react/src/umbrella-store.ts index 686f9a1192..59eb0af6f5 100644 --- a/packages/liveblocks-react/src/umbrella-store.ts +++ b/packages/liveblocks-react/src/umbrella-store.ts @@ -430,6 +430,16 @@ export class PaginatedResource { this.#patch({ isFetchingMore: true }); try { const nextCursor = await this.#fetchPage(state.data.cursor); + + // Clear #pendingFetchMore BEFORE #patch, so that when the signal + // notification triggers a synchronous React re-render (via + // useSyncExternalStore), any useEffect calling fetchMore() will see + // #pendingFetchMore as null and be able to start a new fetch. + // Previously, this was done in a .finally() on the promise chain, but + // that always runs in a later microtask — after React's flush microtask + // — causing the next fetchMore() call to be silently skipped. + this.#pendingFetchMore = null; + this.#patch({ cursor: nextCursor, hasFetchedAll: nextCursor === null, @@ -437,6 +447,8 @@ export class PaginatedResource { isFetchingMore: false, }); } catch (err) { + this.#pendingFetchMore = null; + this.#patch({ isFetchingMore: false, fetchMoreError: err as Error, @@ -454,9 +466,7 @@ export class PaginatedResource { // Case (3) if (!this.#pendingFetchMore) { - this.#pendingFetchMore = this.#fetchMore().finally(() => { - this.#pendingFetchMore = null; - }); + this.#pendingFetchMore = this.#fetchMore(); } return this.#pendingFetchMore; }