Ensure EventListSummary key does not change during backpagination (#7915)

Author: t3chguy

src/components/structures/MessagePanel.tsx

@@ -254,6 +254,9 @@ export default class MessagePanel extends React.Component<IProps, IState> {
     private readonly showTypingNotificationsWatcherRef: string;
     private eventTiles: Record<string, EventTile> = {};
 
+    // A map to allow groupers to maintain consistent keys even if their first event is uprooted due to back-pagination.
+    public grouperKeyMap = new WeakMap<MatrixEvent, string>();
+
     constructor(props, context) {
         super(props, context);
 
@@ -684,14 +687,13 @@ export default class MessagePanel extends React.Component<IProps, IState> {
                     break; // break on first grouper
                 }
             }
+
             if (!grouper) {
-                const wantTile = this.shouldShowEvent(mxEv);
-                const isGrouped = false;
-                if (wantTile) {
+                if (this.shouldShowEvent(mxEv)) {
                     // make sure we unpack the array returned by getTilesForEvent,
-                    // otherwise react will auto-generate keys and we will end up
-                    // replacing all of the DOM elements every time we paginate.
-                    ret.push(...this.getTilesForEvent(prevEvent, mxEv, last, isGrouped, nextEvent, nextTile));
+                    // otherwise React will auto-generate keys, and we will end up
+                    // replacing all the DOM elements every time we paginate.
+                    ret.push(...this.getTilesForEvent(prevEvent, mxEv, last, false, nextEvent, nextTile));
                     prevEvent = mxEv;
                 }
 
@@ -1127,15 +1129,15 @@ class CreationGrouper extends BaseGrouper {
         if (!this.events || !this.events.length) return [];
 
         const panel = this.panel;
-        const ret = [];
+        const ret: ReactNode[] = [];
         const isGrouped = true;
         const createEvent = this.event;
         const lastShownEvent = this.lastShownEvent;
 
         if (panel.wantsDateSeparator(this.prevEvent, createEvent.getDate())) {
             const ts = createEvent.getTs();
             ret.push(
-                <li key={ts+'~'}><DateSeparator key={ts+'~'} roomId={createEvent.getRoomId()} ts={ts} /></li>,
+                <li key={ts+'~'}><DateSeparator roomId={createEvent.getRoomId()} ts={ts} /></li>,
             );
         }
 
@@ -1262,6 +1264,10 @@ class MainGrouper extends BaseGrouper {
         this.events.push(ev);
     }
 
+    private generateKey(): string {
+        return "eventlistsummary-" + this.events[0].getId();
+    }
+
     public getTiles(): ReactNode[] {
         // If we don't have any events to group, don't even try to group them. The logic
         // below assumes that we have a group of events to deal with, but we might not if
@@ -1271,24 +1277,28 @@ class MainGrouper extends BaseGrouper {
         const isGrouped = true;
         const panel = this.panel;
         const lastShownEvent = this.lastShownEvent;
-        const ret = [];
+        const ret: ReactNode[] = [];
 
         if (panel.wantsDateSeparator(this.prevEvent, this.events[0].getDate())) {
             const ts = this.events[0].getTs();
             ret.push(
-                <li key={ts+'~'}><DateSeparator key={ts+'~'} roomId={this.events[0].getRoomId()} ts={ts} /></li>,
+                <li key={ts+'~'}><DateSeparator roomId={this.events[0].getRoomId()} ts={ts} /></li>,
             );
         }
 
-        // Ensure that the key of the EventListSummary does not change with new events.
-        // This will prevent it from being re-created unnecessarily, and
-        // instead will allow new props to be provided. In turn, the shouldComponentUpdate
-        // method on ELS can be used to prevent unnecessary renderings.
-        //
-        // Whilst back-paginating with an ELS at the top of the panel, prevEvent will be null,
-        // so use the key "eventlistsummary-initial". Otherwise, use the ID of the first
-        // membership event, which will not change during forward pagination.
-        const key = "eventlistsummary-" + (this.prevEvent ? this.events[0].getId() : "initial");
+        // Ensure that the key of the EventListSummary does not change with new events in either direction.
+        // This will prevent it from being re-created unnecessarily, and instead will allow new props to be provided.
+        // In turn, the shouldComponentUpdate method on ELS can be used to prevent unnecessary renderings.
+        const keyEvent = this.events.find(e => this.panel.grouperKeyMap.get(e));
+        const key = keyEvent ? this.panel.grouperKeyMap.get(keyEvent) : this.generateKey();
+        if (!keyEvent) {
+            // Populate the weak map with the key that we are using for all related events.
+            this.events.forEach(e => {
+                if (!this.panel.grouperKeyMap.has(e)) {
+                    this.panel.grouperKeyMap.set(e, key);
+                }
+            });
+        }
 
         let highlightInSummary = false;
         let eventTiles = this.events.map((e, i) => {

test/components/structures/MessagePanel-test.js

@@ -496,4 +496,80 @@ describe('MessagePanel', function() {
 
         expect(Dates.length).toEqual(1);
     });
+
+    it('appends events into summaries during forward pagination without changing key', () => {
+        const events = mkMelsEvents().slice(1, 11);
+
+        const res = mount(<WrappedMessagePanel events={events} />);
+        let els = res.find("EventListSummary");
+        expect(els.length).toEqual(1);
+        expect(els.key()).toEqual("eventlistsummary-" + events[0].getId());
+        expect(els.prop("events").length).toEqual(10);
+
+        res.setProps({
+            events: [
+                ...events,
+                TestUtilsMatrix.mkMembership({
+                    event: true,
+                    room: "!room:id",
+                    user: "@user:id",
+                    target: {
+                        userId: "@user:id",
+                        name: "Bob",
+                        getAvatarUrl: () => {
+                            return "avatar.jpeg";
+                        },
+                        getMxcAvatarUrl: () => 'mxc://avatar.url/image.png',
+                    },
+                    ts: Date.now(),
+                    mship: 'join',
+                    prevMship: 'join',
+                    name: 'A user',
+                }),
+            ],
+        });
+
+        els = res.find("EventListSummary");
+        expect(els.length).toEqual(1);
+        expect(els.key()).toEqual("eventlistsummary-" + events[0].getId());
+        expect(els.prop("events").length).toEqual(11);
+    });
+
+    it('prepends events into summaries during backward pagination without changing key', () => {
+        const events = mkMelsEvents().slice(1, 11);
+
+        const res = mount(<WrappedMessagePanel events={events} />);
+        let els = res.find("EventListSummary");
+        expect(els.length).toEqual(1);
+        expect(els.key()).toEqual("eventlistsummary-" + events[0].getId());
+        expect(els.prop("events").length).toEqual(10);
+
+        res.setProps({
+            events: [
+                TestUtilsMatrix.mkMembership({
+                    event: true,
+                    room: "!room:id",
+                    user: "@user:id",
+                    target: {
+                        userId: "@user:id",
+                        name: "Bob",
+                        getAvatarUrl: () => {
+                            return "avatar.jpeg";
+                        },
+                        getMxcAvatarUrl: () => 'mxc://avatar.url/image.png',
+                    },
+                    ts: Date.now(),
+                    mship: 'join',
+                    prevMship: 'join',
+                    name: 'A user',
+                }),
+                ...events,
+            ],
+        });
+
+        els = res.find("EventListSummary");
+        expect(els.length).toEqual(1);
+        expect(els.key()).toEqual("eventlistsummary-" + events[0].getId());
+        expect(els.prop("events").length).toEqual(11);
+    });
 });