Only feed state events to widgets if we believe they update the state

[robintown]

As part of the effort to make room state more reliable, we're looking at changing how state is communicated to widgets. Rather than passing all state events from the timeline through (and forcing the widget to assume that they belong to the server's view of the room state), let's try passing only those state events through that we believe actually belong to the server's view of the room state. When combined with a matrix-js-sdk that supports MSC4222, this improves the reliability of state tracking for widgets.

We would like to later explore a solution that has a separate 'state update' widget action in addition to the generic 'incoming timeline event' widget action, to make the widget API better reflect the shape of the data sources that drive it (like the sync responses of Simplified Sliding Sync or MSC4222).

[toger5]

Change to RoomEvent.Timeline

[robintown]

Can you help me understand why RoomEvent.Timeline would be more appropriate here? I'm looking at:

/**
 * Fires whenever the SDK receives a new event.
 * <p>
 * This is only fired for live events received via /sync - it is not fired for
 * events received over context, search, or pagination APIs.
 *
 * @param event - The matrix event which caused this event to fire.
 * @example
 * ```
 * matrixClient.on("event", function(event){
 *   var sender = event.getSender();
 * });
 * ```
 */
[ClientEvent.Event]: (event: MatrixEvent) => void;

versus

/**
 * Fires whenever the timeline in a room is updated.
 * @param event - The matrix event which caused this event to fire.
 * @param room - The room, if any, whose timeline was updated.
 * @param toStartOfTimeline - True if this event was added to the start
 * @param removed - True if this event has just been removed from the timeline
 * (beginning; oldest) of the timeline e.g. due to pagination.
 *
 * @param data - more data about the event
 *
 * @example
 * ```
 * matrixClient.on("Room.timeline",
 *                 function(event, room, toStartOfTimeline, removed, data) {
 *   if (!toStartOfTimeline && data.liveEvent) {
 *     var messageToAppend = room.timeline.[room.timeline.length - 1];
 *   }
 * });
 * ```
 */
[RoomEvent.Timeline]: (
    event: MatrixEvent,
    room: Room | undefined,
    toStartOfTimeline: boolean | undefined,
    removed: boolean,
    data: IRoomTimelineData,
) => void;

and from these comments it sounds like ClientEvent.Event, receiving new live events down /sync, is exactly what we want.

[t3chguy]

ClientEvent.Event fires on every single event the SDK ever sees, state, account data, presence, all before those events are processed and without any metadata on whether that event is state which should be applied (state_after), or state which should be ignored (timeline).

[robintown]

Right okay, and we don't seem to require any metadata or processing in this spot

[t3chguy]

We do if we want to tell the embedded client if the event is in the timeline or state, isState() doesn't tell you that.

[robintown]

For now we're not yet trying to tell widgets that. This is deliberately a very blunt, "filter all state events out of the timeline and then finally send through just those state events that cause a RoomStateEvent.Events" change. (which is why it's in draft)

[toger5]

should the js-sdk bump to support 4222 also be part of this PR? So that we have a fully supported netlify build?

[robintown]

In theory the build should be working even without MSC4222 (it just won't benefit from that MSC). If we wanted to test with the MSC4222 matrix-js-sdk PR, the easiest way would be to use branch matching, so opening a new draft PR with branch name dbkr/stateafter.

[t3chguy]

If we wanted to test with the MSC4222 matrix-js-sdk PR, the easiest way would be to use branch matching, so opening a new draft PR with branch name dbkr/stateafter.

#28398 is already using that branch matching so please do not interrupt it as it'll mean it'll take longer for the MSC4222 support to land

[toger5]

#28398 is already using that branch matching so please do not interrupt it as it'll mean it'll take longer for the MSC4222 support to land

so we need to update 28398 to have the local store fixes?

[t3chguy]

so we need to update #28398 to have the local store fixes?

Please do not do that, my PR is the minimum required to retain compatibility with the js-sdk changes, any larger than that and it'll increase risk of landing it & also make review take longer than it needs to.