fix: do not increase count of prepended VirtualizedMessageList messages of status "sending" or "failed" (#1972)

* fix: do not increase count of prepended VirtualizedMessageList messages of status "sending"

* fix: do not increase count of prepended VirtualizedMessageList messages of status "failed"

* fix: keep messages without status in the prepend count

* test: ignore messages of status "sending" & "failed" from prepend count

Author: MartinCupela

src/components/MessageList/__tests__/VirtualizedMessageList.test.js

@@ -110,29 +110,59 @@ describe('usePrependedMessagesCount', () => {
     return <div>{prependCount}</div>;
   };
 
-  it('calculates the prepended messages using the id prop', async () => {
+  const expectPrependCount = (count, root) => {
+    expect(root.findByType('div').props.children).toStrictEqual(count);
+  };
+
+  it('determines 0 prepended messages for empty message list', async () => {
     const render = await renderer.create(<TestCase messages={[]} />);
-    const expectPrependCount = (count) => {
-      expect(render.root.findByType('div').props.children).toStrictEqual(count);
-    };
 
-    expectPrependCount(0);
+    expectPrependCount(0, render.root);
+  });
+
+  const messageBatch = (ids, status) => ids.map((id) => ({ id, status }));
+  const firstBatch = (status) => messageBatch(['a'], status);
+  const secondBatch = (status) => messageBatch(['c', 'b'], status);
+  const thirdBatch = (status) => messageBatch(['e', 'd'], status);
+
+  const testPrependCount = async (status, first, second, third) => {
+    const firstMessage = firstBatch(status);
+    const secondMsgBatch = secondBatch(status);
+    const thirdMsgBatch = thirdBatch(status);
+
+    const render = await renderer.create(<TestCase messages={[]} />);
 
     await renderer.act(async () => {
-      await render.update(<TestCase messages={[{ id: 'a' }]} />);
-      expectPrependCount(0);
+      await render.update(<TestCase messages={[...firstMessage]} />);
+      expectPrependCount(first, render.root);
     });
 
     await renderer.act(async () => {
-      await render.update(<TestCase messages={[{ id: 'c' }, { id: 'b' }, { id: 'a' }]} />);
-      expectPrependCount(2);
+      await render.update(<TestCase messages={[...secondMsgBatch, ...firstMessage]} />);
+      expectPrependCount(second, render.root);
     });
 
     await renderer.act(async () => {
       await render.update(
-        <TestCase messages={[{ id: 'e' }, { id: 'd' }, { id: 'c' }, { id: 'b' }, { id: 'a' }]} />,
+        <TestCase messages={[...thirdMsgBatch, ...secondMsgBatch, ...firstMessage]} />,
       );
-      expectPrependCount(4);
+      expectPrependCount(third, render.root);
     });
+  };
+
+  it('calculates the prepended count for messages of status "received"', async () => {
+    await testPrependCount('received', 0, 2, 4);
+  });
+
+  it('ignores the messages of status "sending" from the prepended messages count', async () => {
+    await testPrependCount('sending', 0, 0, 0);
+  });
+
+  it('ignores the messages of status "failed" from the prepended messages count', async () => {
+    await testPrependCount('failed', 0, 0, 0);
+  });
+
+  it('calculates the prepended messages count for the messages of status undefined', async () => {
+    await testPrependCount(undefined, 0, 2, 4);
   });
 });

src/components/MessageList/hooks/usePrependMessagesCount.ts

@@ -4,6 +4,8 @@ import type { StreamMessage } from '../../../context/ChannelStateContext';
 
 import type { DefaultStreamChatGenerics } from '../../../types/types';
 
+const STATUSES_EXCLUDED_FROM_PREPEND = ['sending', 'failed'];
+
 export function usePrependedMessagesCount<
   StreamChatGenerics extends DefaultStreamChatGenerics = DefaultStreamChatGenerics
 >(messages: StreamMessage<StreamChatGenerics>[], hasDateSeparator: boolean) {
@@ -30,14 +32,33 @@ export function usePrependedMessagesCount<
     earliestMessageId.current = currentFirstMessageId;
     // 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;
-        return i;
+        previousNumItemsPrepended.current = i - adjustPrependedMessageCount;
+        return previousNumItemsPrepended.current;
       }
     }
 
-    // if no match has found, we have jumped - reset the prepend item count.
+    // if no match has found, we have jumped - reset the prepended item count.
     firstMessageId.current = currentFirstMessageId;
     previousNumItemsPrepended.current = 0;
     return 0;