Fix TimelinePanel re-renders on duplicate sync state events (#33268)
* Avoid TimelinePanel re-renders on duplicate sync state events * A better solution avoiding to store the entire syncState
This commit is contained in:
@@ -24,7 +24,7 @@ import {
|
|||||||
type MatrixClient,
|
type MatrixClient,
|
||||||
type Relations,
|
type Relations,
|
||||||
type MatrixError,
|
type MatrixError,
|
||||||
type SyncState,
|
SyncState,
|
||||||
TimelineWindow,
|
TimelineWindow,
|
||||||
Thread,
|
Thread,
|
||||||
ThreadEvent,
|
ThreadEvent,
|
||||||
@@ -192,9 +192,6 @@ interface IState {
|
|||||||
backPaginating: boolean;
|
backPaginating: boolean;
|
||||||
forwardPaginating: boolean;
|
forwardPaginating: boolean;
|
||||||
|
|
||||||
// cache of matrixClient.getSyncState() (but from the 'sync' event)
|
|
||||||
clientSyncState: SyncState | null;
|
|
||||||
|
|
||||||
// should the event tiles have twelve hour times
|
// should the event tiles have twelve hour times
|
||||||
isTwelveHour: boolean;
|
isTwelveHour: boolean;
|
||||||
|
|
||||||
@@ -251,12 +248,17 @@ class TimelinePanel extends React.Component<IProps, IState> {
|
|||||||
// A map of <callId, LegacyCallEventGrouper>
|
// A map of <callId, LegacyCallEventGrouper>
|
||||||
private callEventGroupers = new Map<string, LegacyCallEventGrouper>();
|
private callEventGroupers = new Map<string, LegacyCallEventGrouper>();
|
||||||
private initialReadMarkerId: string | null = null;
|
private initialReadMarkerId: string | null = null;
|
||||||
|
private syncImpliesForwardPaginating: boolean;
|
||||||
|
|
||||||
public constructor(props: IProps, context: React.ContextType<typeof RoomContext>) {
|
public constructor(props: IProps, context: React.ContextType<typeof RoomContext>) {
|
||||||
super(props, context);
|
super(props, context);
|
||||||
|
|
||||||
debuglog("mounting");
|
debuglog("mounting");
|
||||||
|
|
||||||
|
this.syncImpliesForwardPaginating = TimelinePanel.isSyncForwardPaginating(
|
||||||
|
MatrixClientPeg.safeGet().getSyncState(),
|
||||||
|
);
|
||||||
|
|
||||||
// XXX: we could track RM per TimelineSet rather than per Room.
|
// XXX: we could track RM per TimelineSet rather than per Room.
|
||||||
// but for now we just do it per room for simplicity.
|
// but for now we just do it per room for simplicity.
|
||||||
if (this.props.manageReadMarkers) {
|
if (this.props.manageReadMarkers) {
|
||||||
@@ -278,7 +280,6 @@ class TimelinePanel extends React.Component<IProps, IState> {
|
|||||||
readMarkerEventId: this.initialReadMarkerId,
|
readMarkerEventId: this.initialReadMarkerId,
|
||||||
backPaginating: false,
|
backPaginating: false,
|
||||||
forwardPaginating: false,
|
forwardPaginating: false,
|
||||||
clientSyncState: MatrixClientPeg.safeGet().getSyncState(),
|
|
||||||
isTwelveHour: SettingsStore.getValue("showTwelveHourTimestamps"),
|
isTwelveHour: SettingsStore.getValue("showTwelveHourTimestamps"),
|
||||||
alwaysShowTimestamps: SettingsStore.getValue("alwaysShowTimestamps"),
|
alwaysShowTimestamps: SettingsStore.getValue("alwaysShowTimestamps"),
|
||||||
readMarkerInViewThresholdMs: SettingsStore.getValue("readMarkerInViewThresholdMs"),
|
readMarkerInViewThresholdMs: SettingsStore.getValue("readMarkerInViewThresholdMs"),
|
||||||
@@ -899,9 +900,17 @@ class TimelinePanel extends React.Component<IProps, IState> {
|
|||||||
|
|
||||||
private onSync = (clientSyncState: SyncState, prevState: SyncState | null, data?: object): void => {
|
private onSync = (clientSyncState: SyncState, prevState: SyncState | null, data?: object): void => {
|
||||||
if (this.unmounted) return;
|
if (this.unmounted) return;
|
||||||
this.setState({ clientSyncState });
|
const nextSyncImpliesForwardPaginating = TimelinePanel.isSyncForwardPaginating(clientSyncState);
|
||||||
|
if (nextSyncImpliesForwardPaginating === this.syncImpliesForwardPaginating) return;
|
||||||
|
|
||||||
|
this.syncImpliesForwardPaginating = nextSyncImpliesForwardPaginating;
|
||||||
|
this.forceUpdate();
|
||||||
};
|
};
|
||||||
|
|
||||||
|
private static isSyncForwardPaginating(syncState: SyncState | null): boolean {
|
||||||
|
return syncState === SyncState.Prepared || syncState === SyncState.Catchup;
|
||||||
|
}
|
||||||
|
|
||||||
private readMarkerTimeout(readMarkerPosition: number | null): number {
|
private readMarkerTimeout(readMarkerPosition: number | null): number {
|
||||||
return readMarkerPosition === 0
|
return readMarkerPosition === 0
|
||||||
? (this.context?.readMarkerInViewThresholdMs ?? this.state.readMarkerInViewThresholdMs)
|
? (this.context?.readMarkerInViewThresholdMs ?? this.state.readMarkerInViewThresholdMs)
|
||||||
@@ -1832,8 +1841,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
|
|||||||
|
|
||||||
// If the state is PREPARED or CATCHUP, we're still waiting for the js-sdk to sync with
|
// If the state is PREPARED or CATCHUP, we're still waiting for the js-sdk to sync with
|
||||||
// the HS and fetch the latest events, so we are effectively forward paginating.
|
// the HS and fetch the latest events, so we are effectively forward paginating.
|
||||||
const forwardPaginating =
|
const forwardPaginating = this.state.forwardPaginating || this.syncImpliesForwardPaginating;
|
||||||
this.state.forwardPaginating || ["PREPARED", "CATCHUP"].includes(this.state.clientSyncState!);
|
|
||||||
const events = this.state.events;
|
const events = this.state.events;
|
||||||
return (
|
return (
|
||||||
<MessagePanel
|
<MessagePanel
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ Please see LICENSE files in the repository root for full details.
|
|||||||
|
|
||||||
import { render, waitFor, screen, act, cleanup } from "jest-matrix-react";
|
import { render, waitFor, screen, act, cleanup } from "jest-matrix-react";
|
||||||
import {
|
import {
|
||||||
|
ClientEvent,
|
||||||
ReceiptType,
|
ReceiptType,
|
||||||
EventTimelineSet,
|
EventTimelineSet,
|
||||||
EventType,
|
EventType,
|
||||||
@@ -19,6 +20,7 @@ import {
|
|||||||
RoomEvent,
|
RoomEvent,
|
||||||
RoomMember,
|
RoomMember,
|
||||||
RoomState,
|
RoomState,
|
||||||
|
SyncState,
|
||||||
TimelineWindow,
|
TimelineWindow,
|
||||||
EventTimeline,
|
EventTimeline,
|
||||||
FeatureSupport,
|
FeatureSupport,
|
||||||
@@ -483,6 +485,39 @@ describe("TimelinePanel", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("only re-renders when sync changes forward pagination state", async () => {
|
||||||
|
const [client, room, events] = setupTestData();
|
||||||
|
let timelinePanel: TimelinePanel | null = null;
|
||||||
|
|
||||||
|
render(
|
||||||
|
<TimelinePanel
|
||||||
|
{...getProps(room, events)}
|
||||||
|
ref={(ref) => {
|
||||||
|
timelinePanel = ref;
|
||||||
|
}}
|
||||||
|
/>,
|
||||||
|
clientAndSDKContextRenderOptions(client, sdkContext),
|
||||||
|
);
|
||||||
|
await flushPromises();
|
||||||
|
await waitFor(() => expect(timelinePanel).toBeTruthy());
|
||||||
|
|
||||||
|
const forceUpdateSpy = jest.spyOn(timelinePanel!, "forceUpdate");
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
client.emit(ClientEvent.Sync, SyncState.Syncing, SyncState.Syncing);
|
||||||
|
await flushPromises();
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(forceUpdateSpy).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
client.emit(ClientEvent.Sync, SyncState.Prepared, SyncState.Syncing);
|
||||||
|
await flushPromises();
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(forceUpdateSpy).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
describe("onRoomTimeline", () => {
|
describe("onRoomTimeline", () => {
|
||||||
it("ignores events for other timelines", () => {
|
it("ignores events for other timelines", () => {
|
||||||
const [client, room, events] = setupTestData();
|
const [client, room, events] = setupTestData();
|
||||||
|
|||||||
Reference in New Issue
Block a user