fix(dom-element-manipulation): evict chat message elements the client drops

samples/sample-dom-element-manipulation/src/sample-dom-element-manipulation/component.tsx

@@ -18,7 +18,7 @@ function SampleDomElementManipulation(
   useEffect(() => {
     if (response.data) {
       const idsToApply = response.data.filter(
-        (message) => message.message.search(REGEX) !== -1,
+        (message) => !message.deletedAt && message.message.search(REGEX) !== -1,
       ).map((message) => message.messageId);
       setChatIdsToApplyHighlights(idsToApply);
     }

samples/sample-server-commands-plugin/src/components/sample-server-commands-plugin-item/component.tsx

@@ -92,6 +92,7 @@ function SampleServerCommandsPluginItem(
 
     const messagesToStyle = loadedMessages.data.filter(
       (message) => {
+        if (message.deletedAt) return false;
         if (!message.messageMetadata) return false;
 
         const messageMetadata = JSON.parse(message.messageMetadata);

src/data-consumption/domain/chat/loaded-chat-messages/types.ts

@@ -2,10 +2,15 @@ import { GraphqlResponseWrapper } from '../../../../core';
 
 export interface LoadedChatMessage {
   createdAt: string;
-  message: string;
+  message: string | null;
   messageId: string;
   senderUserId: string;
+  senderRole: string;
   messageMetadata: string;
+  deletedAt: string | null;
+  deletedBy: {
+    name: string;
+  } | null;
 }
 
 export type UseLoadedChatMessagesFunction = () => GraphqlResponseWrapper<LoadedChatMessage[]>;

src/dom-element-manipulation/chat/message/hooks.ts

@@ -16,47 +16,40 @@ import { sortedStringify } from '../../../data-consumption/utils';
 
 const messageIdFromDomElement = (element: HTMLDivElement) => (element?.getAttribute('data-chat-message-id') || '');
 
+const isSameElementList = (a: HTMLDivElement[], b: HTMLDivElement[]) => (
+  a.length === b.length && a.every((element, index) => element === b[index])
+);
+
 export const useChatMessageDomElements = (messageIds: string[], pluginUuid: string) => {
-  const [domElements, setDomElements] = useState<RenderedChatMessages>([]);
-  const previousMessageIds = useRef<string[]>([]);
+  const [domElements, setDomElements] = useState<RenderedChatMessages>({});
+  const requestedMessageIds = useRef<string[]>([]);
 
-  const handleDomElementUpdateEvent: EventListener = (
+  useEffect(() => {
+    // The client sends every rendered element requested by any plugin for the page,
+    // so the payload is authoritative: it replaces the page in either direction.
+    const handleDomElementUpdateEvent: EventListener = (
       (event: HookEventWrapper<
         UpdatedEventDetails<UpdatedEventDetailsForChatMessageDomElements>>) => {
         const detail = event.detail as UpdatedEventDetails<
         UpdatedEventDetailsForChatMessageDomElements>;
-        if (detail.hook === DomElementManipulationHooks.CHAT_MESSAGE) {
-          const pageToUpdate = detail.data?.page;
-          if (pageToUpdate === undefined) return;
-          if (detail.data?.messages.length === 0) {
-            // indicates the page was unmounted in the client
-            // so we remove all stored elements for that page,
-            // since they might be invalid.
-            setDomElements((domElementsState) => {
-              const newDomElements = {
-                ...domElementsState,
-              };
-              delete newDomElements[pageToUpdate];
-              return newDomElements;
-            });
-          }
+        if (detail.hook !== DomElementManipulationHooks.CHAT_MESSAGE) return;
+        const pageToUpdate = detail.data?.page;
+        if (pageToUpdate === undefined) return;
 
-          const pageDomElementsFromBbbCore = detail.data?.messages.map(
-            (item) => item.message,
-          );
-          const receivedAnythingNew = pageDomElementsFromBbbCore.some(
-            (pageDomElement) => (
-              !domElements[pageToUpdate]?.includes(pageDomElement)),
-          );
-          if (receivedAnythingNew) {
-            setDomElements((domElementsState) => ({
-              ...domElementsState,
-              [pageToUpdate]: pageDomElementsFromBbbCore,
-            }));
+        const pageDomElementsFromBbbCore = detail.data.messages
+          .filter((item) => requestedMessageIds.current.includes(item.messageId))
+          .map((item) => item.message);
+        setDomElements((domElementsState) => {
+          if (isSameElementList(domElementsState[pageToUpdate] ?? [], pageDomElementsFromBbbCore)) {
+            return domElementsState;
           }
-        }
+          return {
+            ...domElementsState,
+            [pageToUpdate]: pageDomElementsFromBbbCore,
+          };
+        });
       }) as EventListener;
-  useEffect(() => {
+
     window.dispatchEvent(
       new CustomEvent<SubscribedEventDetails>(HookEvents.PLUGIN_SUBSCRIBED_TO_BBB_CORE, {
         detail: {
@@ -70,6 +63,7 @@ export const useChatMessageDomElements = (messageIds: string[], pluginUuid: stri
     );
     window.addEventListener(HookEvents.BBB_CORE_SENT_NEW_DATA, handleDomElementUpdateEvent);
     return () => {
+      window.removeEventListener(HookEvents.BBB_CORE_SENT_NEW_DATA, handleDomElementUpdateEvent);
       window.dispatchEvent(
         new CustomEvent<UnsubscribedEventDetails>(HookEvents.PLUGIN_UNSUBSCRIBED_FROM_BBB_CORE, {
           detail: {
@@ -84,18 +78,8 @@ export const useChatMessageDomElements = (messageIds: string[], pluginUuid: stri
     };
   }, []);
 
-  useEffect(() => {
-    window.addEventListener(HookEvents.BBB_CORE_SENT_NEW_DATA, handleDomElementUpdateEvent);
-    // Runs on code cleanup
-    return () => {
-      // On every `domElements` update, the event listener is removed and re-added to ensure
-      // the handler has access to the latest `domElements` state value.
-      window.removeEventListener(HookEvents.BBB_CORE_SENT_NEW_DATA, handleDomElementUpdateEvent);
-    };
-  }, [domElements]);
-
   const updateRequestedIds = () => {
-    previousMessageIds.current = messageIds;
+    requestedMessageIds.current = messageIds;
     window.dispatchEvent(
       new CustomEvent<
         UpdatedEventDetails<void>>(HookEvents.PLUGIN_SENT_CHANGES_TO_BBB_CORE, {
@@ -111,7 +95,7 @@ export const useChatMessageDomElements = (messageIds: string[], pluginUuid: stri
     );
   };
 
-  if (sortedStringify((messageIds) || []) !== sortedStringify(previousMessageIds.current)) {
+  if (sortedStringify((messageIds) || []) !== sortedStringify(requestedMessageIds.current)) {
     updateRequestedIds();
   }
 

tests/unit/dom-element-manipulation/chat/message/hooks.test.ts

@@ -0,0 +1,99 @@
+import {
+  describe, it, expect, vi,
+} from 'vitest';
+import { renderHook, act } from '@testing-library/react';
+
+import { useChatMessageDomElements } from '../../../../../src/dom-element-manipulation/chat/message/hooks';
+import { HookEvents } from '../../../../../src/core/enum';
+import { DomElementManipulationHooks } from '../../../../../src/dom-element-manipulation/enums';
+
+const PLUGIN_UUID = 'plugin-uuid';
+
+const makeMessageElement = (messageId: string) => {
+  const element = document.createElement('div');
+  element.setAttribute('data-chat-message-id', messageId);
+  return element;
+};
+
+const sendPage = (page: number, elements: HTMLDivElement[]) => {
+  act(() => {
+    window.dispatchEvent(new CustomEvent(HookEvents.BBB_CORE_SENT_NEW_DATA, {
+      detail: {
+        hook: DomElementManipulationHooks.CHAT_MESSAGE,
+        data: {
+          page,
+          messages: elements.map((message) => ({
+            messageId: message.getAttribute('data-chat-message-id'),
+            message,
+          })),
+        },
+      },
+    }));
+  });
+};
+
+const renderChatMessageHook = (initialMessageIds: string[]) => renderHook(
+  ({ messageIds }) => useChatMessageDomElements(messageIds, PLUGIN_UUID),
+  { initialProps: { messageIds: initialMessageIds } },
+);
+
+describe('useChatMessageDomElements', () => {
+  it('returns the elements sent for the requested ids', () => {
+    const a = makeMessageElement('a');
+    const b = makeMessageElement('b');
+    const { result } = renderChatMessageHook(['a', 'b']);
+
+    sendPage(0, [a, b]);
+
+    expect(result.current).toEqual([a, b]);
+  });
+
+  it('drops an element the client stops sending while the id is still requested', () => {
+    const a = makeMessageElement('a');
+    const b = makeMessageElement('b');
+    const { result } = renderChatMessageHook(['a', 'b']);
+
+    sendPage(0, [a, b]);
+    sendPage(0, [a]);
+
+    expect(result.current).toEqual([a]);
+  });
+
+  it('empties a mounted page that renders none of the requested ids, keeping other pages', () => {
+    const a = makeMessageElement('a');
+    const b = makeMessageElement('b');
+    const { result } = renderChatMessageHook(['a', 'b']);
+
+    sendPage(0, [a]);
+    sendPage(1, [b]);
+    sendPage(0, []);
+
+    expect(result.current).toEqual([b]);
+  });
+
+  it('stores only the elements this plugin requested', () => {
+    const a = makeMessageElement('a');
+    const c = makeMessageElement('c');
+    const { result, rerender } = renderChatMessageHook(['a']);
+
+    sendPage(0, [a, c]);
+    rerender({ messageIds: ['a', 'c'] });
+
+    expect(result.current).toEqual([a]);
+  });
+
+  it('stops listening for data after unmounting', () => {
+    const addSpy = vi.spyOn(window, 'addEventListener');
+    const removeSpy = vi.spyOn(window, 'removeEventListener');
+    const { unmount } = renderChatMessageHook(['a']);
+
+    unmount();
+
+    const listenersFor = (spy: typeof addSpy) => spy.mock.calls
+      .filter(([eventName]) => eventName === HookEvents.BBB_CORE_SENT_NEW_DATA)
+      .map(([, listener]) => listener);
+    const removedListeners = listenersFor(removeSpy);
+    expect(listenersFor(addSpy).every((listener) => removedListeners.includes(listener)))
+      .toBe(true);
+  });
+});