Don't use deprecated room.currentState in useRoomState.

[toger5]

Signed-off-by: Timo K toger5@hotmail.de

Checklist

• Tests written for new code (and old code if feasible).
• New or updated public/exported symbols have accurate TSDoc documentation.
• Linter and other CI checks pass.
• Sign-off given on the changes (see CONTRIBUTING.md).

[t3chguy]

Honestly with the advent of getters in modern JS I think we should undo the deprecation and just have it call the right thing for us, just like we did with get timeline on the Room object with something like

Index: src/models/room.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/models/room.ts b/src/models/room.ts
--- a/src/models/room.ts	(revision 8f8101bf8b16cb7f4ba0edb2521f87cc39b0e1bc)
+++ b/src/models/room.ts	(date 1712597697947)
@@ -383,20 +383,7 @@
      * The room summary.
      */
     public summary: RoomSummary | null = null;
-    /**
-     * oldState The state of the room at the time of the oldest event in the live timeline.
-     *
-     * @deprecated Present for backwards compatibility.
-     *             Use getLiveTimeline().getState(EventTimeline.BACKWARDS) instead
-     */
-    public oldState!: RoomState;
-    /**
-     * currentState The state of the room at the time of the newest event in the timeline.
-     *
-     * @deprecated Present for backwards compatibility.
-     *             Use getLiveTimeline().getState(EventTimeline.FORWARDS) instead.
-     */
-    public currentState!: RoomState;
+
     public readonly relations = new RelationsContainer(this.client, this);
 
     /**
@@ -802,6 +789,28 @@
         return this.getLiveTimeline().getEvents();
     }
 
+    /**
+     * The state of the room at the time of the oldest event in the live timeline.
+     */
+    public get oldState(): RoomState {
+        const roomState = this.getLiveTimeline().getState(EventTimeline.BACKWARDS);
+        if (!roomState) {
+            throw new Error("No oldState");
+        }
+        return roomState;
+    }
+
+    /**
+     * The state of the room at the time of the newest event in the timeline.
+     */
+    public get currentState(): RoomState {
+        const roomState = this.getLiveTimeline().getState(EventTimeline.FORWARDS);
+        if (!roomState) {
+            throw new Error("No currentState");
+        }
+        return roomState;
+    }
+
     /**
      * Get the timestamp of the last message in the room
      *
@@ -1267,12 +1276,6 @@
         const previousOldState = this.oldState;
         const previousCurrentState = this.currentState;
 
-        // maintain this.oldState and this.currentState as references to the
-        // state at the start and end of that timeline. These are more
-        // for backwards-compatibility than anything else.
-        this.oldState = this.getLiveTimeline().getState(EventTimeline.BACKWARDS)!;
-        this.currentState = this.getLiveTimeline().getState(EventTimeline.FORWARDS)!;
-
         // Let people know to register new listeners for the new state
         // references. The reference won't necessarily change every time so only
         // emit when we see a change.

[toger5]

I did find the currentState shortcut useful so I like this proposal.
Is the diff sth that you have as an PR. Or should I go ahead and patch this PR to look more like what you propose?

[t3chguy]

It is just a bodged patch, untested

[toger5]

Adding the changes i am wondering how this would behave with event emitters?
Since currentState returns a new obj on each call registering event listeners to it might be confusing?
Especially since RoomEvent.CurrentStateUpdated and RoomEvent.OldStateUpdated would not be emitted anymore:

const previousOldState = this.oldState;
        const previousCurrentState = this.currentState;

        // Let people know to register new listeners for the new state
        // references. The reference won't necessarily change every time so only
        // emit when we see a change.
        if (previousOldState !== this.oldState) {
            this.emit(RoomEvent.OldStateUpdated, this, previousOldState, this.oldState);
        }

        if (previousCurrentState !== this.currentState) {
            this.emit(RoomEvent.CurrentStateUpdated, this, previousCurrentState, this.currentState);
            ...

[t3chguy]

Since currentState returns a new obj

Does it? Assuming the same LiveTimeline the same state object would be returned.

Especially since RoomEvent.CurrentStateUpdated and RoomEvent.OldStateUpdated would not be emitted anymore:

Yes that would need fixing via some other means as you need to know when the RoomState is changed out from under you

[toger5]

Does it? Assuming the same LiveTimeline the same state object would be returned.

Sure only if the oldState changed.

Is it as simple that whenever there is a new state object both (current and old) get changed?

[t3chguy]

Is it as simple that whenever there is a new state object both (current and old) get changed?

No, the each EventTimeline has 2 RoomState objects, they never change. They are set in the c'tor only. So only when your EventTimeline changes would you need to update room states. Except whatever magic forkLive is clones and replaces it

[toger5]

It seems we never use the RoomEvent.CurrentStateUpdated and RoomEvent.OldStateUpdated events.
Is there a potential case we need those? Could those be deprecated? Is there a project using them?

[t3chguy]

I think the emitted events could possibly be deprecated, but we'd still need the logic for reEmitter.stopReEmitting, reEmit, etc so they might as well stay given they are in the same place