From cb2f0413654405c2360980427823c0bb7cce2767 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Thu, 24 Jul 2025 17:31:18 +0530 Subject: [PATCH 01/10] Introduce snapshot class to track snapshot updates This avoids the hassle of having to manually call emit. --- src/viewmodels/base/Snapshot.ts | 43 +++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 src/viewmodels/base/Snapshot.ts diff --git a/src/viewmodels/base/Snapshot.ts b/src/viewmodels/base/Snapshot.ts new file mode 100644 index 00000000000..364f118746f --- /dev/null +++ b/src/viewmodels/base/Snapshot.ts @@ -0,0 +1,43 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +*/ + +/** + * This is the output of the viewmodel that the view consumes. + * Updating snapshot through this object will make react re-render + * components. + */ +export class Snapshot { + public constructor( + private snapshot: T, + private emit: () => void, + ) {} + + /** + * Replace current snapshot with a new snapshot value. + * @param snapshot New snapshot value + */ + public set(snapshot: T): void { + this.snapshot = snapshot; + this.emit(); + } + + /** + * Update a part of the current snapshot by merging into the existing snapshot. + * @param snapshot A subset of the snapshot to merge into the current snapshot. + */ + public merge(snapshot: Partial): void { + this.snapshot = { ...this.snapshot, snapshot }; + this.emit(); + } + + /** + * The current value of the snapshot. + */ + public get current(): T { + return this.snapshot; + } +} From c45d79417523426fdf574842da90774819a96657 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Thu, 24 Jul 2025 17:33:46 +0530 Subject: [PATCH 02/10] Better viewmodel ergonomics - Rename `SubscriptionViewModel` to `BaseViewModel`. I feel this is appropriate since that class does more than just manage subscriptions. - `getSnapshot` is no longer an abstract method. It's simply a method that returns the current snapshot state. This ensures that getSnapshot result is cached by default which is required by `useSyncExternalStore`. - `props` are a property of the base vm class so that actual VMs don't have to keep creating this property. --- .../BaseViewModel.ts} | 17 +++++++++++++---- .../{ => base}/ViewModelSubscriptions.ts | 0 2 files changed, 13 insertions(+), 4 deletions(-) rename src/viewmodels/{SubscriptionViewModel.ts => base/BaseViewModel.ts} (76%) rename src/viewmodels/{ => base}/ViewModelSubscriptions.ts (100%) diff --git a/src/viewmodels/SubscriptionViewModel.ts b/src/viewmodels/base/BaseViewModel.ts similarity index 76% rename from src/viewmodels/SubscriptionViewModel.ts rename to src/viewmodels/base/BaseViewModel.ts index 65147d4cc4e..942cb6d210f 100644 --- a/src/viewmodels/SubscriptionViewModel.ts +++ b/src/viewmodels/base/BaseViewModel.ts @@ -5,17 +5,24 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import { type ViewModel } from "../shared-components/ViewModel"; +import { type ViewModel } from "../../shared-components/ViewModel"; +import { Snapshot } from "./Snapshot"; import { ViewModelSubscriptions } from "./ViewModelSubscriptions"; -export abstract class SubscriptionViewModel implements ViewModel { +export abstract class BaseViewModel implements ViewModel { protected subs: ViewModelSubscriptions; + protected snapshot: Snapshot; + protected props: P; - protected constructor() { + protected constructor(props: P, initialSnapshot: T) { + this.props = props; this.subs = new ViewModelSubscriptions( this.addDownstreamSubscriptionWrapper, this.removeDownstreamSubscriptionWrapper, ); + this.snapshot = new Snapshot(initialSnapshot, () => { + this.subs.emit(); + }); } public subscribe = (listener: () => void): (() => void) => { @@ -52,5 +59,7 @@ export abstract class SubscriptionViewModel implements ViewModel { /** * Returns the current snapshot of the view model. */ - public abstract getSnapshot: () => T; + public getSnapshot = (): T => { + return this.snapshot.current; + }; } diff --git a/src/viewmodels/ViewModelSubscriptions.ts b/src/viewmodels/base/ViewModelSubscriptions.ts similarity index 100% rename from src/viewmodels/ViewModelSubscriptions.ts rename to src/viewmodels/base/ViewModelSubscriptions.ts From 2c5d086425418432e5cf618dcfb31feeccc1ab88 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Thu, 24 Jul 2025 17:40:01 +0530 Subject: [PATCH 03/10] Update `TextualEventViewModel` --- .../event-tiles/TextualEventViewModel.ts | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/viewmodels/event-tiles/TextualEventViewModel.ts b/src/viewmodels/event-tiles/TextualEventViewModel.ts index d2f56482d70..a9fd64c60d3 100644 --- a/src/viewmodels/event-tiles/TextualEventViewModel.ts +++ b/src/viewmodels/event-tiles/TextualEventViewModel.ts @@ -11,28 +11,24 @@ import { type EventTileTypeProps } from "../../events/EventTileFactory"; import { MatrixClientPeg } from "../../MatrixClientPeg"; import { textForEvent } from "../../TextForEvent"; import { type TextualEventViewSnapshot } from "../../shared-components/event-tiles/TextualEvent/TextualEvent"; -import { SubscriptionViewModel } from "../SubscriptionViewModel"; +import { BaseViewModel } from "../base/BaseViewModel"; -export class TextualEventViewModel extends SubscriptionViewModel { - public constructor(private eventTileProps: EventTileTypeProps) { - super(); +export class TextualEventViewModel extends BaseViewModel { + public constructor(props: EventTileTypeProps) { + super(props, ""); + this.setTextFromEvent(); } - protected addDownstreamSubscription = (): void => { - this.eventTileProps.mxEvent.on(MatrixEventEvent.SentinelUpdated, this.subs.emit); + private setTextFromEvent = (): void => { + const text = textForEvent(this.props.mxEvent, MatrixClientPeg.safeGet(), true, this.props.showHiddenEvents); + this.snapshot.set(text); }; - protected removeDownstreamSubscription = (): void => { - this.eventTileProps.mxEvent.off(MatrixEventEvent.SentinelUpdated, this.subs.emit); + protected addDownstreamSubscription = (): void => { + this.props.mxEvent.on(MatrixEventEvent.SentinelUpdated, this.setTextFromEvent); }; - public getSnapshot = (): TextualEventViewSnapshot => { - const text = textForEvent( - this.eventTileProps.mxEvent, - MatrixClientPeg.safeGet(), - true, - this.eventTileProps.showHiddenEvents, - ); - return text; + protected removeDownstreamSubscription = (): void => { + this.props.mxEvent.off(MatrixEventEvent.SentinelUpdated, this.setTextFromEvent); }; } From e8e50816f10e503afddde4d4ad38cdd98545c148 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Thu, 24 Jul 2025 19:35:26 +0530 Subject: [PATCH 04/10] Fix test --- test/viewmodels/event-tiles/TextualEventViewModel-test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/viewmodels/event-tiles/TextualEventViewModel-test.ts b/test/viewmodels/event-tiles/TextualEventViewModel-test.ts index 92037df9fc4..735ecd25420 100644 --- a/test/viewmodels/event-tiles/TextualEventViewModel-test.ts +++ b/test/viewmodels/event-tiles/TextualEventViewModel-test.ts @@ -8,10 +8,16 @@ Please see LICENSE files in the repository root for full details. import { MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/matrix"; import { TextualEventViewModel } from "../../../src/viewmodels/event-tiles/TextualEventViewModel"; +import { stubClient } from "../../test-utils"; + +jest.mock("../../../src/TextForEvent.tsx", () => ({ + textForEvent: jest.fn().mockReturnValue("Test Message"), +})); describe("TextualEventViewModel", () => { it("should update when the sentinel updates", () => { const fakeEvent = new MatrixEvent({}); + stubClient(); const vm = new TextualEventViewModel({ showHiddenEvents: false, From 2b366e82c5855176906925b30b3068cb44133bb8 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Thu, 31 Jul 2025 15:26:24 +0530 Subject: [PATCH 05/10] Rename `TextualEvent` to `TextualEventView` --- ...Event.stories.tsx => TextualEventView.stories.tsx} | 4 ++-- ...extualEvent.test.tsx => TextualEventView.test.tsx} | 4 ++-- .../{TextualEvent.tsx => TextualEventView.tsx} | 11 ++++++----- ...t.test.tsx.snap => TextualEventView.test.tsx.snap} | 0 .../event-tiles/TextualEvent/index.ts | 2 +- src/viewmodels/event-tiles/TextualEventViewModel.ts | 8 ++++---- 6 files changed, 15 insertions(+), 14 deletions(-) rename src/shared-components/event-tiles/TextualEvent/{TextualEvent.stories.tsx => TextualEventView.stories.tsx} (80%) rename src/shared-components/event-tiles/TextualEvent/{TextualEvent.test.tsx => TextualEventView.test.tsx} (84%) rename src/shared-components/event-tiles/TextualEvent/{TextualEvent.tsx => TextualEventView.tsx} (63%) rename src/shared-components/event-tiles/TextualEvent/__snapshots__/{TextualEvent.test.tsx.snap => TextualEventView.test.tsx.snap} (100%) diff --git a/src/shared-components/event-tiles/TextualEvent/TextualEvent.stories.tsx b/src/shared-components/event-tiles/TextualEvent/TextualEventView.stories.tsx similarity index 80% rename from src/shared-components/event-tiles/TextualEvent/TextualEvent.stories.tsx rename to src/shared-components/event-tiles/TextualEvent/TextualEventView.stories.tsx index 1746bc14b23..836641a84c4 100644 --- a/src/shared-components/event-tiles/TextualEvent/TextualEvent.stories.tsx +++ b/src/shared-components/event-tiles/TextualEvent/TextualEventView.stories.tsx @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { type Meta, type StoryFn } from "@storybook/react-vite"; -import { TextualEvent as TextualEventComponent } from "./TextualEvent"; +import { TextualEventView as TextualEventComponent } from "./TextualEventView"; import { MockViewModel } from "../../MockViewModel"; export default { @@ -16,7 +16,7 @@ export default { component: TextualEventComponent, tags: ["autodocs"], args: { - vm: new MockViewModel("Dummy textual event text"), + vm: new MockViewModel({ content: "Dummy textual event text" }), }, } as Meta; diff --git a/src/shared-components/event-tiles/TextualEvent/TextualEvent.test.tsx b/src/shared-components/event-tiles/TextualEvent/TextualEventView.test.tsx similarity index 84% rename from src/shared-components/event-tiles/TextualEvent/TextualEvent.test.tsx rename to src/shared-components/event-tiles/TextualEvent/TextualEventView.test.tsx index b1ef5e8f525..5d2dd912efb 100644 --- a/src/shared-components/event-tiles/TextualEvent/TextualEvent.test.tsx +++ b/src/shared-components/event-tiles/TextualEvent/TextualEventView.test.tsx @@ -9,11 +9,11 @@ import { composeStories } from "@storybook/react-vite"; import { render } from "jest-matrix-react"; import React from "react"; -import * as stories from "./TextualEvent.stories.tsx"; +import * as stories from "./TextualEventView.stories.tsx"; const { Default } = composeStories(stories); -describe("TextualEvent", () => { +describe("TextualEventView", () => { it("renders a textual event", () => { const { container } = render(); expect(container).toMatchSnapshot(); diff --git a/src/shared-components/event-tiles/TextualEvent/TextualEvent.tsx b/src/shared-components/event-tiles/TextualEvent/TextualEventView.tsx similarity index 63% rename from src/shared-components/event-tiles/TextualEvent/TextualEvent.tsx rename to src/shared-components/event-tiles/TextualEvent/TextualEventView.tsx index 1dec80905ec..fa18a4c599b 100644 --- a/src/shared-components/event-tiles/TextualEvent/TextualEvent.tsx +++ b/src/shared-components/event-tiles/TextualEvent/TextualEventView.tsx @@ -10,14 +10,15 @@ import React, { type ReactNode, type JSX } from "react"; import { type ViewModel } from "../../ViewModel"; import { useViewModel } from "../../useViewModel"; -export type TextualEventViewSnapshot = string | ReactNode; +export type TextualEventViewSnapshot = { + content: string | ReactNode; +}; export interface Props { vm: ViewModel; } -export function TextualEvent({ vm }: Props): JSX.Element { - const contents = useViewModel(vm); - - return
{contents}
; +export function TextualEventView({ vm }: Props): JSX.Element { + const snapshot = useViewModel(vm); + return
{snapshot.content}
; } diff --git a/src/shared-components/event-tiles/TextualEvent/__snapshots__/TextualEvent.test.tsx.snap b/src/shared-components/event-tiles/TextualEvent/__snapshots__/TextualEventView.test.tsx.snap similarity index 100% rename from src/shared-components/event-tiles/TextualEvent/__snapshots__/TextualEvent.test.tsx.snap rename to src/shared-components/event-tiles/TextualEvent/__snapshots__/TextualEventView.test.tsx.snap diff --git a/src/shared-components/event-tiles/TextualEvent/index.ts b/src/shared-components/event-tiles/TextualEvent/index.ts index 96f257fbfa1..58796a3dd79 100644 --- a/src/shared-components/event-tiles/TextualEvent/index.ts +++ b/src/shared-components/event-tiles/TextualEvent/index.ts @@ -5,4 +5,4 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -export { TextualEvent } from "./TextualEvent"; +export { TextualEventView } from "./TextualEventView"; diff --git a/src/viewmodels/event-tiles/TextualEventViewModel.ts b/src/viewmodels/event-tiles/TextualEventViewModel.ts index a9fd64c60d3..e3f6184f7e3 100644 --- a/src/viewmodels/event-tiles/TextualEventViewModel.ts +++ b/src/viewmodels/event-tiles/TextualEventViewModel.ts @@ -10,18 +10,18 @@ import { MatrixEventEvent } from "matrix-js-sdk/src/matrix"; import { type EventTileTypeProps } from "../../events/EventTileFactory"; import { MatrixClientPeg } from "../../MatrixClientPeg"; import { textForEvent } from "../../TextForEvent"; -import { type TextualEventViewSnapshot } from "../../shared-components/event-tiles/TextualEvent/TextualEvent"; +import { type TextualEventViewSnapshot } from "../../shared-components/event-tiles/TextualEvent/TextualEventView"; import { BaseViewModel } from "../base/BaseViewModel"; export class TextualEventViewModel extends BaseViewModel { public constructor(props: EventTileTypeProps) { - super(props, ""); + super(props, { content: "" }); this.setTextFromEvent(); } private setTextFromEvent = (): void => { - const text = textForEvent(this.props.mxEvent, MatrixClientPeg.safeGet(), true, this.props.showHiddenEvents); - this.snapshot.set(text); + const content = textForEvent(this.props.mxEvent, MatrixClientPeg.safeGet(), true, this.props.showHiddenEvents); + this.snapshot.set({ content }); }; protected addDownstreamSubscription = (): void => { From 8123c14d0275303c6089713993f44d0ec32caa7e Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Thu, 31 Jul 2025 15:30:40 +0530 Subject: [PATCH 06/10] Fix snapshot object not being merged --- src/viewmodels/base/Snapshot.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viewmodels/base/Snapshot.ts b/src/viewmodels/base/Snapshot.ts index 364f118746f..e8d0b7412ce 100644 --- a/src/viewmodels/base/Snapshot.ts +++ b/src/viewmodels/base/Snapshot.ts @@ -30,7 +30,7 @@ export class Snapshot { * @param snapshot A subset of the snapshot to merge into the current snapshot. */ public merge(snapshot: Partial): void { - this.snapshot = { ...this.snapshot, snapshot }; + this.snapshot = { ...this.snapshot, ...snapshot }; this.emit(); } From c80c6f0ba8243767c10594f4dfdf816eaef3df57 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Thu, 31 Jul 2025 15:33:11 +0530 Subject: [PATCH 07/10] Rename directory to `EventTileView` --- src/events/EventTileFactory.tsx | 4 ++-- .../TextualEventView.stories.tsx | 0 .../TextualEventView.test.tsx | 0 .../{TextualEvent => TextualEventView}/TextualEventView.tsx | 0 .../__snapshots__/TextualEventView.test.tsx.snap | 0 .../event-tiles/{TextualEvent => TextualEventView}/index.ts | 0 src/viewmodels/event-tiles/TextualEventViewModel.ts | 2 +- 7 files changed, 3 insertions(+), 3 deletions(-) rename src/shared-components/event-tiles/{TextualEvent => TextualEventView}/TextualEventView.stories.tsx (100%) rename src/shared-components/event-tiles/{TextualEvent => TextualEventView}/TextualEventView.test.tsx (100%) rename src/shared-components/event-tiles/{TextualEvent => TextualEventView}/TextualEventView.tsx (100%) rename src/shared-components/event-tiles/{TextualEvent => TextualEventView}/__snapshots__/TextualEventView.test.tsx.snap (100%) rename src/shared-components/event-tiles/{TextualEvent => TextualEventView}/index.ts (100%) diff --git a/src/events/EventTileFactory.tsx b/src/events/EventTileFactory.tsx index 9d0daadb80a..9187e05546b 100644 --- a/src/events/EventTileFactory.tsx +++ b/src/events/EventTileFactory.tsx @@ -44,7 +44,7 @@ import { ElementCall } from "../models/Call"; import { type IBodyProps } from "../components/views/messages/IBodyProps"; import ModuleApi from "../modules/Api"; import { TextualEventViewModel } from "../viewmodels/event-tiles/TextualEventViewModel"; -import { TextualEvent } from "../shared-components/event-tiles/TextualEvent"; +import { TextualEventView } from "../shared-components/event-tiles/TextualEventView"; // Subset of EventTile's IProps plus some mixins export interface EventTileTypeProps @@ -81,7 +81,7 @@ const LegacyCallEventFactory: Factory ; export const TextualEventFactory: Factory = (ref, props) => { const vm = new TextualEventViewModel(props); - return ; + return ; }; const VerificationReqFactory: Factory = (_ref, props) => ; const HiddenEventFactory: Factory = (ref, props) => ; diff --git a/src/shared-components/event-tiles/TextualEvent/TextualEventView.stories.tsx b/src/shared-components/event-tiles/TextualEventView/TextualEventView.stories.tsx similarity index 100% rename from src/shared-components/event-tiles/TextualEvent/TextualEventView.stories.tsx rename to src/shared-components/event-tiles/TextualEventView/TextualEventView.stories.tsx diff --git a/src/shared-components/event-tiles/TextualEvent/TextualEventView.test.tsx b/src/shared-components/event-tiles/TextualEventView/TextualEventView.test.tsx similarity index 100% rename from src/shared-components/event-tiles/TextualEvent/TextualEventView.test.tsx rename to src/shared-components/event-tiles/TextualEventView/TextualEventView.test.tsx diff --git a/src/shared-components/event-tiles/TextualEvent/TextualEventView.tsx b/src/shared-components/event-tiles/TextualEventView/TextualEventView.tsx similarity index 100% rename from src/shared-components/event-tiles/TextualEvent/TextualEventView.tsx rename to src/shared-components/event-tiles/TextualEventView/TextualEventView.tsx diff --git a/src/shared-components/event-tiles/TextualEvent/__snapshots__/TextualEventView.test.tsx.snap b/src/shared-components/event-tiles/TextualEventView/__snapshots__/TextualEventView.test.tsx.snap similarity index 100% rename from src/shared-components/event-tiles/TextualEvent/__snapshots__/TextualEventView.test.tsx.snap rename to src/shared-components/event-tiles/TextualEventView/__snapshots__/TextualEventView.test.tsx.snap diff --git a/src/shared-components/event-tiles/TextualEvent/index.ts b/src/shared-components/event-tiles/TextualEventView/index.ts similarity index 100% rename from src/shared-components/event-tiles/TextualEvent/index.ts rename to src/shared-components/event-tiles/TextualEventView/index.ts diff --git a/src/viewmodels/event-tiles/TextualEventViewModel.ts b/src/viewmodels/event-tiles/TextualEventViewModel.ts index e3f6184f7e3..40121ecf2b7 100644 --- a/src/viewmodels/event-tiles/TextualEventViewModel.ts +++ b/src/viewmodels/event-tiles/TextualEventViewModel.ts @@ -10,7 +10,7 @@ import { MatrixEventEvent } from "matrix-js-sdk/src/matrix"; import { type EventTileTypeProps } from "../../events/EventTileFactory"; import { MatrixClientPeg } from "../../MatrixClientPeg"; import { textForEvent } from "../../TextForEvent"; -import { type TextualEventViewSnapshot } from "../../shared-components/event-tiles/TextualEvent/TextualEventView"; +import { type TextualEventViewSnapshot } from "../../shared-components/event-tiles/TextualEventView/TextualEventView"; import { BaseViewModel } from "../base/BaseViewModel"; export class TextualEventViewModel extends BaseViewModel { From d86df4a7bb45109797a1f2ec05e05e993a1da798 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Thu, 31 Jul 2025 15:46:38 +0530 Subject: [PATCH 08/10] Fix broken snapshot --- .../__snapshots__/TextualEventView.test.tsx.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared-components/event-tiles/TextualEventView/__snapshots__/TextualEventView.test.tsx.snap b/src/shared-components/event-tiles/TextualEventView/__snapshots__/TextualEventView.test.tsx.snap index 186e5f0afd8..a0be215af7b 100644 --- a/src/shared-components/event-tiles/TextualEventView/__snapshots__/TextualEventView.test.tsx.snap +++ b/src/shared-components/event-tiles/TextualEventView/__snapshots__/TextualEventView.test.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`TextualEvent renders a textual event 1`] = ` +exports[`TextualEventView renders a textual event 1`] = `
Date: Thu, 31 Jul 2025 15:47:35 +0530 Subject: [PATCH 09/10] Add test for snapshot class --- test/viewmodels/base/Snapshot-test.ts | 41 +++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test/viewmodels/base/Snapshot-test.ts diff --git a/test/viewmodels/base/Snapshot-test.ts b/test/viewmodels/base/Snapshot-test.ts new file mode 100644 index 00000000000..796caa65ab4 --- /dev/null +++ b/test/viewmodels/base/Snapshot-test.ts @@ -0,0 +1,41 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +*/ + +import { Snapshot } from "../../../src/viewmodels/base/Snapshot"; + +interface TestSnapshot { + key1: string; + key2: number; + key3: boolean; +} + +describe("Snapshot", () => { + it("should accept an initial value", () => { + const snapshot = new Snapshot({ key1: "foo", key2: 5, key3: false }, jest.fn()); + expect(snapshot.current).toStrictEqual({ key1: "foo", key2: 5, key3: false }); + }); + + it("should call emit callback when state changes", () => { + const emit = jest.fn(); + const snapshot = new Snapshot({ key1: "foo", key2: 5, key3: false }, emit); + snapshot.merge({ key3: true }); + expect(emit).toHaveBeenCalledTimes(1); + }); + + it("should swap out entire snapshot on set call", () => { + const snapshot = new Snapshot({ key1: "foo", key2: 5, key3: false }, jest.fn()); + const newValue = { key1: "bar", key2: 8, key3: true }; + snapshot.set(newValue); + expect(snapshot.current).toStrictEqual(newValue); + }); + + it("should merge partial snapshot on merge call", () => { + const snapshot = new Snapshot({ key1: "foo", key2: 5, key3: false }, jest.fn()); + snapshot.merge({ key2: 10 }); + expect(snapshot.current).toStrictEqual({ key1: "foo", key2: 10, key3: false }); + }); +}); From c64104ffeea69da579ad601f9a4e0af8624e05a9 Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Thu, 31 Jul 2025 19:41:22 +0530 Subject: [PATCH 10/10] Add demo test --- .../ViewModelSubIssueDemo.test.tsx | 182 ++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 src/shared-components/ViewModelSubIssueDemo.test.tsx diff --git a/src/shared-components/ViewModelSubIssueDemo.test.tsx b/src/shared-components/ViewModelSubIssueDemo.test.tsx new file mode 100644 index 00000000000..7f320517319 --- /dev/null +++ b/src/shared-components/ViewModelSubIssueDemo.test.tsx @@ -0,0 +1,182 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +*/ + +import React, { useState } from "react"; +import EventEmitter from "events"; +import { fireEvent, render } from "jest-matrix-react"; + +import { BaseViewModel } from "../viewmodels/base/BaseViewModel"; +import type { ViewModel } from "./ViewModel"; +import { useViewModel } from "./useViewModel"; + +/** + * View-models will need to listen to events so that it can do some necessary recalculation. + * Let's create an example of such an event emitter. + * Once start() is called and until stop() is called, MessageEmitter will continuously emit a "NEW_MESSAGE" + * event every 500ms. + */ +class MessageEmitter extends EventEmitter { + public count = 0; + private timeout?: NodeJS.Timeout; + + public constructor() { + super(); + } + + public start(): void { + this.timeout = setInterval(() => { + this.count = this.count + 1; + this.emit("NEW_MESSAGE"); + }, 500); + } + + public stop(): void { + clearInterval(this.timeout); + } +} + +/** + * We're going to create a message counter component that just renders the total message count. + */ + +interface MessageCounterViewModelProps { + /** + * An emitter that lets the vm know when a new message has arrived. + */ + emitter: MessageEmitter; +} + +interface MessageCounterViewState { + /** + * The number of messages in this room + */ + count: number; +} + +/** + * This view model is written with the API that we have today. + */ +class MessageCounterViewModel extends BaseViewModel { + public constructor(props: MessageCounterViewModelProps) { + super(props, { count: 0 }); + } + + private onMessage = (): void => { + // Increase the count by 1 on new message event + const count = this.snapshot.current.count + 1; + this.snapshot.set({ count }); + }; + + protected addDownstreamSubscription(): void { + this.props.emitter.on("NEW_MESSAGE", this.onMessage); + } + + protected removeDownstreamSubscription(): void { + this.props.emitter.off("NEW_MESSAGE", this.onMessage); + } +} + +/** + * This is the fixed version of above with the lifetime of the event listeners being + * equal to the lifetime of the view-model itself. + */ +class MessageCounterViewModelFixed extends BaseViewModel { + public constructor(props: MessageCounterViewModelProps) { + super(props, { count: 0 }); + this.props.emitter.on("NEW_MESSAGE", this.onMessage); + } + + private onMessage = (): void => { + // Increase the count by 1 on new message event + const count = this.snapshot.current.count + 1; + this.snapshot.set({ count }); + }; + + protected addDownstreamSubscription(): void {} + + protected removeDownstreamSubscription(): void {} +} + +interface MessageCounterViewProps { + vm: ViewModel; +} +const MessageCounterView: React.FC = ({ vm }) => { + const snapshot = useViewModel(vm); + // Nothing too interesting, just render the count from the vm. + return
{snapshot.count}
; +}; + +const emitter = new MessageEmitter(); + +const RoomTileView = ({ vm }: { vm: ViewModel }): React.ReactNode => { + const [hovered, setHovered] = useState(false); + // const vm = useMemo(() => new MessageCounterViewModelFixed({ emitter }), []); + + /** + * This is similar to the room tile in the room list. + * It shows the count when your mouse cursor is outside the tile. + * It shows some icon (say 'i') when the mouse cursor is inside the tile. In our + * actual room tile in element web, this would be the notification options icon. + */ + return ( +
{ + setHovered(true); + }} + onMouseLeave={() => { + setHovered(false); + }} + > +
F
+
Foo Room
+
{hovered ?
i
: }
+
+ ); +}; + +it.each([ + ["Unfixed ViewModel", new MessageCounterViewModel({ emitter })], + ["Fixed ViewModel", new MessageCounterViewModelFixed({ emitter })], +])( + "view has stale state demo, vm type = %s", + async (_, vm) => { + // 1. First let's just render our component + render(); + // 2. Let's instruct the emitter to start spawning new events + emitter.start(); + // 3. Let's wait three seconds so that the counts can actually increment. + await new Promise((r) => setTimeout(r, 3000)); + // 4. Stopping the emitter while we do our assert + emitter.stop(); + // 5. We haven't moved our mouse inside the tile, so we expect the count + // in MessageCounterView component to match the total number of events that + // have been emitted so far. + expect(document.querySelector(".icon")).toHaveTextContent(`${emitter.count}`); + + // 6. Let's start emitting events again + emitter.start(); + // 7. This time, we're going to move the cursor into the tile + fireEvent.mouseEnter(document.querySelector(".root")!); + // 8. So now we expect the icon to be shown instead of message counter + expect(document.querySelector(".icon")).toHaveTextContent("i"); + // 9. Let's say that the mouse is inside for 3 seconds + await new Promise((r) => setTimeout(r, 3000)); + // 10. Let's move the cursor out of the tile + fireEvent.mouseLeave(document.querySelector(".root")!); + // 11. As before, the icon should be unmounted and the message counter should be shown again. + expect(document.querySelector(".icon")).not.toHaveTextContent("i"); + // 12. Stop the emitter before assert + emitter.stop(); + + // 13. This is where the issue arises. We would expect the message counter to show the correct + // count. But it's going to be incorrect because the view model did not respond to any of the + // events from the emitter while the view was unmounted (when the icon 'i' was rendered). + expect(document.querySelector(".icon")).toHaveTextContent(`${emitter.count}`); + }, + 30000, +);