fix(VML): prevent recalculation of prepended items when messages are swapped due to status change (#2203)

(cherry picked from commit 565e2f9)

Author: MartinCupela

src/components/MessageList/hooks/VirtualizedMessageList/usePrependMessagesCount.ts

@@ -4,14 +4,17 @@ import type { StreamMessage } from '../../../../context/ChannelStateContext';
 
 import type { DefaultStreamChatGenerics } from '../../../../types/types';
 
-const STATUSES_EXCLUDED_FROM_PREPEND = ['sending', 'failed'];
+const STATUSES_EXCLUDED_FROM_PREPEND = ({
+  failed: true,
+  sending: true,
+} as const) as Record<string, boolean>;
 
 export function usePrependedMessagesCount<
   StreamChatGenerics extends DefaultStreamChatGenerics = DefaultStreamChatGenerics
 >(messages: StreamMessage<StreamChatGenerics>[], hasDateSeparator: boolean) {
   const firstRealMessageIndex = hasDateSeparator ? 1 : 0;
-  const firstMessageId = useRef<string>();
-  const earliestMessageId = useRef<string>();
+  const firstMessageOnFirstLoadedPage = useRef<StreamMessage<StreamChatGenerics>>();
+  const previousFirstMessageOnFirstLoadedPage = useRef<StreamMessage<StreamChatGenerics>>();
   const previousNumItemsPrepended = useRef(0);
 
   const numItemsPrepended = useMemo(() => {
@@ -20,51 +23,56 @@ export function usePrependedMessagesCount<
       return 0;
     }
 
-    const currentFirstMessageId = messages?.[firstRealMessageIndex]?.id;
-    // if no new messages were prepended, return early (same amount as before)
-    if (currentFirstMessageId === earliestMessageId.current) {
+    const currentFirstMessage = messages?.[firstRealMessageIndex];
+
+    const noNewMessages =
+      currentFirstMessage?.id === previousFirstMessageOnFirstLoadedPage.current?.id;
+
+    // This is possible only, when sending messages very quickly (basically single char messages submitted like a crazy) in empty channel (first page)
+    // Optimistic UI update, when sending messages, can lead to a situation, when
+    // the order of the messages changes for a moment. This can happen, when a user
+    // sends multiple messages withing few milliseconds. E.g. we send a message A
+    // then message B. At first we have message array with both messages of status "sending"
+    // then response for message A is received with a new - later - created_at timestamp
+    // this leads to rearrangement of 1.B ("sending"), 2.A ("received"). Still firstMessageOnFirstLoadedPage.current
+    // points to message A, but now this message has index 1 => previousNumItemsPrepended.current === 1
+    // That in turn leads to incorrect index calculation in VirtualizedMessageList trying to access a message
+    // at non-existent index. Therefore, we ignore messages of status "sending" / "failed" in order they are
+    // not considered as prepended messages.
+    const firstMsgMovedAfterMessagesInExcludedStatus =
+      currentFirstMessage?.status && STATUSES_EXCLUDED_FROM_PREPEND[currentFirstMessage.status];
+
+    if (noNewMessages || firstMsgMovedAfterMessagesInExcludedStatus) {
       return previousNumItemsPrepended.current;
     }
 
-    if (!firstMessageId.current) {
-      firstMessageId.current = currentFirstMessageId;
+    if (!firstMessageOnFirstLoadedPage.current) {
+      firstMessageOnFirstLoadedPage.current = currentFirstMessage;
     }
-    earliestMessageId.current = currentFirstMessageId;
+    previousFirstMessageOnFirstLoadedPage.current = currentFirstMessage;
     // if new messages were prepended, find out how many
     // start with this number because there cannot be fewer prepended items than before
-    let adjustPrependedMessageCount = 0;
-    for (let i = previousNumItemsPrepended.current; i < messages.length; i += 1) {
-      // Optimistic UI update, when sending messages, can lead to a situation, when
-      // the order of the messages changes for a moment. This can happen, when a user
-      // sends multiple messages withing few milliseconds. E.g. we send a message A
-      // then message B. At first we have message array with both messages of status "sending"
-      // then response for message A is received with a new - later - created_at timestamp
-      // this leads to rearrangement of 1.B ("sending"), 2.A ("received"). Still firstMessageId.current
-      // points to message A, but now this message has index 1 => previousNumItemsPrepended.current === 1
-      // That in turn leads to incorrect index calculation in VirtualizedMessageList trying to access a message
-      // at non-existent index. Therefore, we ignore messages of status "sending" / "failed" in order they are
-      // not considered as prepended messages.
-      if (
-        messages[i]?.status &&
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-        STATUSES_EXCLUDED_FROM_PREPEND.includes(messages[i].status!) &&
-        messages[i].id !== firstMessageId.current
-      ) {
-        adjustPrependedMessageCount++;
-      }
-      if (messages[i].id === firstMessageId.current) {
-        previousNumItemsPrepended.current = i - adjustPrependedMessageCount;
-        return previousNumItemsPrepended.current;
+    for (
+      let prependedMessageCount = previousNumItemsPrepended.current;
+      prependedMessageCount < messages.length;
+      prependedMessageCount += 1
+    ) {
+      const messageIsFirstOnFirstLoadedPage =
+        messages[prependedMessageCount].id === firstMessageOnFirstLoadedPage.current?.id;
+
+      if (messageIsFirstOnFirstLoadedPage) {
+        previousNumItemsPrepended.current = prependedMessageCount;
+        return prependedMessageCount;
       }
     }
 
     // if no match has found, we have jumped - reset the prepended item count.
-    firstMessageId.current = currentFirstMessageId;
+    firstMessageOnFirstLoadedPage.current = currentFirstMessage;
     previousNumItemsPrepended.current = 0;
     return 0;
     // TODO: there's a bug here, the messages prop is the same array instance (something mutates it)
     // that's why the second dependency is necessary
-  }, [messages, messages?.length]);
+  }, [firstRealMessageIndex, messages, messages?.length]);
 
   return numItemsPrepended;
 }

src/components/MessageList/hooks/__tests__/usePrependMessagesCount.test.js

@@ -0,0 +1,145 @@
+import { renderHook } from '@testing-library/react-hooks';
+import { usePrependedMessagesCount } from '../VirtualizedMessageList';
+import { generateMessage } from '../../../../mock-builders';
+
+const defaultMsgStatus = 'received';
+const getMessages = (length, statuses = []) =>
+  Array.from({ length }, (_, i) =>
+    generateMessage({
+      status: statuses?.length ? statuses[i] || defaultMsgStatus : defaultMsgStatus,
+    }),
+  );
+
+const messagesWithDateSeparator = ({ length, messages } = {}) => [
+  generateMessage({ customType: 'message.date' }),
+  ...(messages || getMessages(length)),
+];
+
+const hasDateSeparator = true;
+const page1 = getMessages(5);
+const page2 = getMessages(10);
+const page1WithDateSeparator = messagesWithDateSeparator({ messages: page1 });
+const page2WithDateSeparator = messagesWithDateSeparator({ messages: page2 });
+
+const render = ({ hasDateSeparator, messages } = {}) =>
+  renderHook((props) => usePrependedMessagesCount(props.messages, props.hasDateSeparator), {
+    initialProps: { hasDateSeparator, messages },
+  });
+describe('usePrependMessagesCount', function () {
+  it('is 0 when not messages are available', () => {
+    const { result: resultMessagesUndefined } = render();
+    const { result: resultMessagesUndefinedHasDateSeparator } = render({ hasDateSeparator: true });
+    const { result: resultMessagesEmpty } = render({ messages: [] });
+    const { result: resultMessagesEmptyHasDateSeparator } = render({
+      hasDateSeparator: true,
+      messages: [],
+    });
+    expect(resultMessagesUndefinedHasDateSeparator.current).toBe(0);
+    expect(resultMessagesUndefined.current).toBe(0);
+    expect(resultMessagesEmpty.current).toBe(0);
+    expect(resultMessagesEmptyHasDateSeparator.current).toBe(0);
+  });
+
+  it('is 0 on first loaded page and date separator disabled', () => {
+    const { result } = render({ messages: page1 });
+    expect(result.current).toBe(0);
+  });
+
+  it('is 1 on first loaded page and date separator enabled', () => {
+    const { result } = render({
+      hasDateSeparator,
+      messages: page1,
+    });
+    expect(result.current).toBe(1);
+  });
+
+  it('is 0 when re-rendered with no new messages and date separator disabled', () => {
+    const props = {
+      messages: page1,
+    };
+    const { rerender, result } = render(props);
+    rerender(props);
+    expect(result.current).toBe(0);
+  });
+
+  it('is 1 when re-rendered with no new messages and date separator enabled', () => {
+    const props = {
+      hasDateSeparator,
+      messages: page1,
+    };
+    const { rerender, result } = render(props);
+    rerender(props);
+    expect(result.current).toBe(1);
+  });
+
+  it('is 0 when re-rendered with no new but swapped messages and date separator disabled', () => {
+    const messages = getMessages(2, ['sending', 'sending']);
+    const { rerender, result } = render({
+      messages,
+    });
+
+    rerender({ messages: [messages[1], { ...messages[0], status: 'received' }] });
+    expect(result.current).toBe(0);
+  });
+
+  it('is 0 when re-rendered with no new but swapped messages and date separator enabled', () => {
+    const messages = getMessages(2, ['sending', 'sending']);
+    const messages1 = messagesWithDateSeparator({
+      messages,
+    });
+    const messages2 = messagesWithDateSeparator({
+      messages: [
+        messages[1],
+        ...getMessages(1, ['sending']),
+        { ...messages[0], status: 'received' },
+      ],
+    });
+    const { rerender, result } = render({
+      hasDateSeparator,
+      messages: messages1,
+    });
+    rerender({
+      hasDateSeparator,
+      messages: messages2,
+    });
+
+    expect(result.current).toBe(0);
+  });
+
+  it('is equal to newly loaded page size when date separator disabled', () => {
+    const { rerender, result } = render({
+      messages: page1,
+    });
+    rerender({ messages: [...page2, ...page1] });
+    expect(result.current).toBe(page2.length);
+  });
+
+  it('is equal to newly loaded page size + 1 when date separator enabled', () => {
+    const { rerender, result } = render({
+      hasDateSeparator,
+      messages: page1WithDateSeparator,
+    });
+    rerender({
+      hasDateSeparator,
+      messages: messagesWithDateSeparator({ messages: [...page2, ...page1] }),
+    });
+    expect(result.current).toBe(page2.length + 1);
+  });
+
+  it('is 0 when jumped to a message and date separator disabled', () => {
+    const { rerender, result } = render({
+      messages: page1,
+    });
+    rerender({ messages: page2 });
+    expect(result.current).toBe(0);
+  });
+
+  it('is 0 when jumped to a message and date separator enabled', () => {
+    const { rerender, result } = render({
+      hasDateSeparator,
+      messages: page1WithDateSeparator,
+    });
+    rerender({ hasDateSeparator, messages: page2WithDateSeparator });
+    expect(result.current).toBe(0);
+  });
+});