Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/message-list-history-loop-while-hidden.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Fixed the message list loading the entire room history while it is hidden behind a full-width contextual bar. On a narrow window the room layout hides the message body, and a hidden element reports its height and scroll offset as `0`, which made the load-older-messages check always true - so every page that arrived triggered the next one until the whole room was in memory, hammering the server until it rate-limited the client. On returning to the list the scroll position was far in the past, because the content had grown underneath it.
34 changes: 34 additions & 0 deletions apps/meteor/client/views/room/body/hooks/useGetMore.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,40 @@ describe('useGetMore', () => {
});
});

it('should not call getMore while the list is hidden, even though it reports being at the top', async () => {
const root = mockAppRoot();
(RoomHistoryManager.isLoading as jest.Mock).mockReturnValue(false);
(RoomHistoryManager.hasMore as jest.Mock).mockReturnValue(true);
(RoomHistoryManager.hasMoreNext as jest.Mock).mockReturnValue(false);
(RoomHistoryManager.getMore as jest.Mock).mockClear();

const Test = () => {
const [atBottom] = useState(false);
const { innerRef } = useGetMore('room-id', atBottom);
return <div ref={innerRef as any} style={{ display: 'none' }} data-testid='scrollable-element' />;
};

// What a display: none element reports. Without a guard `scrollTop <= clientHeight / 3` is
// 0 <= 0, so every observer callback would load another page of history.
(getBoundingClientRect as jest.Mock).mockReturnValue({
scrollTop: 0,
clientHeight: 0,
scrollHeight: 0,
});

render(<Test />, {
wrapper: root.build(),
});

const scrollableElement = screen.getByTestId('scrollable-element');
scrollableElement.dispatchEvent(new Event('wheel'));
scrollableElement.dispatchEvent(new Event('scroll'));

await waitFor(() => {
expect(RoomHistoryManager.getMore).not.toHaveBeenCalled();
});
});

it('should call getMoreNext when scrolling near bottom and hasMoreNext is true', () => {
const root = mockAppRoot();
(RoomHistoryManager.isLoading as jest.Mock).mockReturnValue(false);
Expand Down
7 changes: 7 additions & 0 deletions apps/meteor/client/views/room/body/hooks/useGetMore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ export const useGetMore = (rid: string, isJumpingToMessage: boolean) => {

const { scrollTop, clientHeight, scrollHeight } = getBoundingClientRect(element);

// RoomLayout hides the message body while a contextual bar takes the full room width,
// and a hidden element reports clientHeight 0. Without this guard the position check
// below is always true and pulls in the entire room history, one observer call at a time.
if (clientHeight === 0) {
return;
}

const lastScrollTopRef = scrollTop;
const height = clientHeight;
const hasMore = RoomHistoryManager.hasMore(rid);
Expand Down