Avoid excessive re-render when setting same reference/value in snapshot (#32918)

* perf: avoid excessive re-render when setting same value in snapshot

* test: update tests

apps/web/test/viewmodels/message-body/EventContentBodyViewModel-test.tsx

@@ -280,7 +280,7 @@ describe("EventContentBodyViewModel", () => {
         expect(vm.getSnapshot().body).toBe("Updated");
     });
 
-    it("emits updates when setters are called with unchanged values", () => {
+    it("doesn't emit updates when setters are called with unchanged values", () => {
         const replacer = jest.fn();
         mockedCombineRenderers.mockReturnValue(() => replacer);
         mockedBodyToNode.mockReturnValue({
@@ -298,7 +298,7 @@ describe("EventContentBodyViewModel", () => {
         vm.setEventContent(undefined, defaultContent);
         vm.setAs("span");
 
-        expect(subscriber).toHaveBeenCalledTimes(2);
+        expect(subscriber).toHaveBeenCalledTimes(0);
         expect(vm.getSnapshot()).toEqual(previousSnapshot);
     });
 

apps/web/test/viewmodels/message-body/ReactionsRowButtonViewModel-test.tsx

@@ -80,7 +80,7 @@ describe("ReactionsRowButtonViewModel", () => {
         expect(listener).toHaveBeenCalledTimes(1);
         expect(tooltipSetPropsSpy).not.toHaveBeenCalled();
 
-        vm.setCount(5);
+        vm.setCount(6);
 
         expect(listener).toHaveBeenCalledTimes(2);
     });

apps/web/test/viewmodels/message-body/ReactionsRowViewModel-test.tsx

@@ -86,7 +86,7 @@ describe("ReactionsRowViewModel", () => {
         expect(onAddReactionContextMenu).toHaveBeenCalledWith(clickEvent);
     });
 
-    it("emits only for setters that always merge when values are unchanged", () => {
+    it("doesn't emit only setters that always merge when values are unchanged", () => {
         const vm = createVm();
         const previousSnapshot = vm.getSnapshot();
         const listener = jest.fn();
@@ -99,7 +99,7 @@ describe("ReactionsRowViewModel", () => {
 
         // `setReactionGroupCount` is optimized and skips emit for unchanged derived values.
         // The other setters always merge and therefore emit.
-        expect(listener).toHaveBeenCalledTimes(3);
+        expect(listener).toHaveBeenCalledTimes(0);
         expect(vm.getSnapshot()).toEqual(previousSnapshot);
     });
 });

packages/shared-components/src/viewmodel/Snapshot.ts

@@ -27,9 +27,13 @@ export class Snapshot<T> {
 
     /**
      * Update a part of the current snapshot by merging into the existing snapshot.
+     * Only emits if at least one of the merged fields has a different reference than the current value.
      * @param snapshot A subset of the snapshot to merge into the current snapshot.
      */
     public merge(snapshot: Partial<T>): void {
+        const keys = Object.keys(snapshot) as Array<keyof T>;
+        const hasChanged = keys.some((key) => !Object.is(snapshot[key], this.snapshot[key]));
+        if (!hasChanged) return;
         this.snapshot = { ...this.snapshot, ...snapshot };
         this.emit();
     }

packages/shared-components/src/viewmodel/tests/Snapshot.test.ts

@@ -40,4 +40,20 @@ describe("Snapshot", () => {
         snapshot.merge({ key2: 10 });
         expect(snapshot.current).toStrictEqual({ key1: "foo", key2: 10, key3: false });
     });
+
+    it("should not emit when merged values are unchanged", () => {
+        const emit = vi.fn();
+        const snapshot = new Snapshot<TestSnapshot>({ key1: "foo", key2: 5, key3: false }, emit);
+        snapshot.merge({ key1: "foo", key2: 5 });
+        expect(emit).not.toHaveBeenCalled();
+        expect(snapshot.current).toStrictEqual({ key1: "foo", key2: 5, key3: false });
+    });
+
+    it("should emit when at least one merged value differs", () => {
+        const emit = vi.fn();
+        const snapshot = new Snapshot<TestSnapshot>({ key1: "foo", key2: 5, key3: false }, emit);
+        snapshot.merge({ key1: "foo", key2: 10 });
+        expect(emit).toHaveBeenCalledTimes(1);
+        expect(snapshot.current).toStrictEqual({ key1: "foo", key2: 10, key3: false });
+    });
 });