From 69cadd8ca4c4a7c5433015ffee76c9419a945291 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sat, 14 Mar 2026 09:44:41 -0400 Subject: [PATCH 1/3] fix(timeline): don't force scroll-to-bottom on sync gap when scrolled up When a sliding-sync limited response fires RoomEvent.TimelineReset (or the app fires RoomEvent.TimelineRefresh) the useLiveTimelineRefresh callback was unconditionally calling setAtBottom(true) and incrementing scrollToBottomRef, scrolling the user to the bottom even if they had scrolled up to read history. Three coordinated changes: 1. Add RoomEvent.TimelineReset handler to useLiveTimelineRefresh. The SDK emits TimelineReset on the EventTimelineSet (not the Room) when a limited sync response replaces the live EventTimeline. Without this listener the stored linkedTimelines reference the old detached chain; back-pagination silently no-ops, freezing the room. 2. Gate the viewport scroll on atBottomRef.current (prior position). Capture wasAtBottom before calling setTimeline(getInitialTimeline). Only call setAtBottom(true) and increment scrollToBottomRef when the user was already at the bottom. When scrolled up we still reinit the timeline (the old chain is gone) but avoid the forced scroll. 3. Add timelineJustResetRef to allow the self-heal effect to run when atBottom=false after a reset. The SDK fires TimelineReset before populating the fresh timeline, so getInitialTimeline sees range.end=0. Without the atBottom guard bypass the range stays at {0,0} as events arrive, leaving the virtual paginator rendering nothing. The ref is cleared on first successful heal, after which normal atBottom-gated behaviour resumes. --- src/app/features/room/RoomTimeline.tsx | 76 +++++++++++++++++--------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 6365f851..8fddfaab 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -535,10 +535,20 @@ const useLiveTimelineRefresh = (room: Room, onRefresh: () => void) => { if (r.roomId !== room.roomId) return; onRefresh(); }; + // The SDK fires RoomEvent.TimelineReset on the EventTimelineSet (not the Room) + // when a limited sliding-sync response replaces the live EventTimeline with a + // fresh one. Without this handler, the stored linkedTimelines reference the old + // detached chain and back-pagination silently no-ops, freezing the room. + const handleTimelineReset: EventTimelineSetHandlerMap[RoomEvent.TimelineReset] = () => { + onRefresh(); + }; + const unfilteredTimelineSet = room.getUnfilteredTimelineSet(); room.on(RoomEvent.TimelineRefresh, handleTimelineRefresh); + unfilteredTimelineSet.on(RoomEvent.TimelineReset, handleTimelineReset); return () => { room.removeListener(RoomEvent.TimelineRefresh, handleTimelineRefresh); + unfilteredTimelineSet.removeListener(RoomEvent.TimelineReset, handleTimelineReset); }; }, [room, onRefresh]); }; @@ -779,6 +789,12 @@ export function RoomTimeline({ atBottomRef.current = val; }, []); + // Set to true by the useLiveTimelineRefresh callback when the timeline is + // re-initialised (TimelineRefresh or TimelineReset). Allows the range self-heal + // effect below to run even when atBottom=false, so the virtual paginator window + // is restored to the live end without forcing a viewport scroll. + const timelineJustResetRef = useRef(false); + const scrollRef = useRef(null); const scrollToBottomRef = useRef({ count: 0, @@ -1058,33 +1074,33 @@ export function RoomTimeline({ useLiveTimelineRefresh( room, useCallback(() => { - // Always reinitialize on TimelineRefresh. With sliding sync, a limited - // response replaces the room's live EventTimeline with a brand-new object, - // firing TimelineRefresh. At that moment liveTimelineLinked is stale-false - // (the stored linkedTimelines still reference the old detached object), - // so the previous guard `if (liveTimelineLinked || ...)` would silently - // skip reinit. Back-pagination then calls paginateEventTimeline against - // the dead old timeline, which no-ops, and the IntersectionObserver never - // re-fires because intersection state didn't change — causing a permanent - // hang at the top of the timeline with no spinner and no history loaded. - // Unconditionally reinitializing is correct: TimelineRefresh signals that - // the SDK has replaced the timeline chain, so any stored range/indices - // against the old chain are invalid anyway. + // Always reinitialize on TimelineRefresh/TimelineReset. With sliding sync, + // a limited response replaces the room's live EventTimeline with a brand-new + // object. At that moment liveTimelineLinked is stale-false (stored + // linkedTimelines reference the old detached chain), so any guard on that + // flag would skip reinit, causing back-pagination to no-op silently and the + // room to appear frozen. Unconditional reinit is correct: both events signal + // that stored range/indices against the old chain are invalid. + // + // Only force the viewport to the bottom if the user was already there. + // When the user has scrolled up to read history and a sync gap fires, we + // must still reinit (the old timeline is gone), but scrolling them back to + // the bottom is jarring. Instead we set timelineJustResetRef=true so the + // self-heal effect below can advance the range as events arrive on the fresh + // timeline, without atBottom=true being required. // - // Also force atBottom=true and queue a scroll-to-bottom. The SDK fires - // TimelineRefresh before adding new events to the fresh live timeline, so - // getInitialTimeline captures range.end=0. Once events arrive the - // rangeAtEnd self-heal useEffect needs atBottom=true to run; the - // IntersectionObserver may have transiently fired isIntersecting=false - // during the render transition, leaving atBottom=false and causing the - // "Jump to Latest" button to stick permanently. Forcing atBottom here is - // correct: TimelineRefresh always reinits to the live end, so the user - // should be repositioned to the bottom regardless. + // When the user WAS at the bottom we still call setAtBottom(true) so a + // transient isIntersecting=false from the IntersectionObserver during the + // DOM transition cannot stick the "Jump to Latest" button on-screen. debugLog.info('timeline', 'Live timeline refresh triggered', { roomId: room.roomId }); + const wasAtBottom = atBottomRef.current; + timelineJustResetRef.current = true; setTimeline(getInitialTimeline(room)); - setAtBottom(true); - scrollToBottomRef.current.count += 1; - scrollToBottomRef.current.smooth = false; + if (wasAtBottom) { + setAtBottom(true); + scrollToBottomRef.current.count += 1; + scrollToBottomRef.current.smooth = false; + } }, [room, setAtBottom]) ); @@ -1111,9 +1127,17 @@ export function RoomTimeline({ // position we want to display. Without this, loading more history makes it look // like we've scrolled up because the range (0, 10) is now showing the old events // instead of the latest ones. + // + // Also runs after a timeline reset (timelineJustResetRef=true) even when + // atBottom=false. After TimelineReset the SDK fires the event before populating + // the fresh timeline, so getInitialTimeline sees range.end=0. When events + // arrive eventsLength grows and we need to heal the range back to the live end + // regardless of the user's scroll position. useEffect(() => { - if (atBottom && liveTimelineLinked && eventsLength > timeline.range.end) { - // More events exist than our current range shows. Adjust to stay at bottom. + const resetPending = timelineJustResetRef.current; + if ((atBottom || resetPending) && liveTimelineLinked && eventsLength > timeline.range.end) { + if (resetPending) timelineJustResetRef.current = false; + // More events exist than our current range shows. Adjust to the live end. setTimeline((ct) => ({ ...ct, range: { From b6bb35a93994f1197bc52968d01e71bc039d1aa9 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sat, 14 Mar 2026 09:49:11 -0400 Subject: [PATCH 2/3] fix(timeline): stable callback ref pattern for room event listener hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useLiveEventArrive, useRelationUpdate, useLiveTimelineRefresh, and useThreadUpdate each passed their callback argument into the useEffect dependency array. This caused the listener to be removed and re-added every time the callback identity changed (e.g. when unreadInfo toggled or when any other parent-scope value included in the useCallback deps changed). Under normal conditions React's synchronous effect cleanup guarantees one listener at a time. But during sliding-sync error/retry cycles the client emits MaxListenersExceededWarning (11 session_ended, 51 RoomState.events) confirming that listeners accumulate. When the callbacks change fast enough — or React defers cleanup in concurrent mode — multiple handleTimelineEvent instances are live simultaneously. Each one independently calls setTimeline({ range: start+1, end+1 }) for the same arriving event, producing the spurious scroll jumps reported. Fix: use the stable callback ref pattern in all four hooks. - Add a ref (onArriveRef / onRelationRef / onRefreshRef / onUpdateRef) - Assign ref.current = callback on every render (always up-to-date) - Remove the callback from the useEffect dep array (effect runs only when `room` changes) - Inside handlers, delegate via ref.current() instead of the captured closure value The event listener identity is now stable for the entire lifetime of each room mount. Listener counts stay at 1 regardless of how many re-renders occur or how many sync error/retry cycles fire. --- src/app/features/room/RoomTimeline.tsx | 48 ++++++++++++++++++-------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 8fddfaab..65455e28 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -460,6 +460,12 @@ const useTimelinePagination = ( }; const useLiveEventArrive = (room: Room, onArrive: (mEvent: MatrixEvent) => void) => { + // Stable ref so the effect dep array only contains `room`. The listener is + // registered once per room mount; onArrive can change freely without causing + // listener churn during rapid re-renders (e.g. sync error/retry cycles). + const onArriveRef = useRef(onArrive); + onArriveRef.current = onArrive; + useEffect(() => { // Capture the live timeline and registration time. Events appended to the // live timeline AFTER this point can be genuinely new even when @@ -486,14 +492,14 @@ const useLiveEventArrive = (room: Room, onArrive: (mEvent: MatrixEvent) => void) data.timeline === liveTimeline && mEvent.getTs() >= registeredAt - 60_000); if (!isLive) return; - onArrive(mEvent); + onArriveRef.current(mEvent); }; const handleRedaction: RoomEventHandlerMap[RoomEvent.Redaction] = ( mEvent: MatrixEvent, eventRoom: Room | undefined ) => { if (eventRoom?.roomId !== room.roomId) return; - onArrive(mEvent); + onArriveRef.current(mEvent); }; room.on(RoomEvent.Timeline, handleTimelineEvent); @@ -502,10 +508,13 @@ const useLiveEventArrive = (room: Room, onArrive: (mEvent: MatrixEvent) => void) room.removeListener(RoomEvent.Timeline, handleTimelineEvent); room.removeListener(RoomEvent.Redaction, handleRedaction); }; - }, [room, onArrive]); + }, [room]); // stable: re-register only when room changes, not on callback identity changes }; const useRelationUpdate = (room: Room, onRelation: () => void) => { + const onRelationRef = useRef(onRelation); + onRelationRef.current = onRelation; + useEffect(() => { const handleTimelineEvent: EventTimelineSetHandlerMap[RoomEvent.Timeline] = ( mEvent: MatrixEvent, @@ -519,28 +528,31 @@ const useRelationUpdate = (room: Room, onRelation: () => void) => { // also need to trigger a re-render so makeReplaced state is reflected. if (eventRoom?.roomId !== room.roomId || data.liveEvent) return; if (mEvent.getRelation()?.rel_type === RelationType.Replace) { - onRelation(); + onRelationRef.current(); } }; room.on(RoomEvent.Timeline, handleTimelineEvent); return () => { room.removeListener(RoomEvent.Timeline, handleTimelineEvent); }; - }, [room, onRelation]); + }, [room]); }; const useLiveTimelineRefresh = (room: Room, onRefresh: () => void) => { + const onRefreshRef = useRef(onRefresh); + onRefreshRef.current = onRefresh; + useEffect(() => { const handleTimelineRefresh: RoomEventHandlerMap[RoomEvent.TimelineRefresh] = (r: Room) => { if (r.roomId !== room.roomId) return; - onRefresh(); + onRefreshRef.current(); }; // The SDK fires RoomEvent.TimelineReset on the EventTimelineSet (not the Room) // when a limited sliding-sync response replaces the live EventTimeline with a // fresh one. Without this handler, the stored linkedTimelines reference the old // detached chain and back-pagination silently no-ops, freezing the room. const handleTimelineReset: EventTimelineSetHandlerMap[RoomEvent.TimelineReset] = () => { - onRefresh(); + onRefreshRef.current(); }; const unfilteredTimelineSet = room.getUnfilteredTimelineSet(); @@ -550,21 +562,27 @@ const useLiveTimelineRefresh = (room: Room, onRefresh: () => void) => { room.removeListener(RoomEvent.TimelineRefresh, handleTimelineRefresh); unfilteredTimelineSet.removeListener(RoomEvent.TimelineReset, handleTimelineReset); }; - }, [room, onRefresh]); + }, [room]); }; // Trigger re-render when thread reply counts change so the thread chip updates. const useThreadUpdate = (room: Room, onUpdate: () => void) => { + const onUpdateRef = useRef(onUpdate); + onUpdateRef.current = onUpdate; + useEffect(() => { - room.on(ThreadEvent.New, onUpdate); - room.on(ThreadEvent.Update, onUpdate); - room.on(ThreadEvent.NewReply, onUpdate); + // Stable wrapper: the same function identity is kept for the lifetime of + // the room so add/removeListener calls always match. + const handler = () => onUpdateRef.current(); + room.on(ThreadEvent.New, handler); + room.on(ThreadEvent.Update, handler); + room.on(ThreadEvent.NewReply, handler); return () => { - room.removeListener(ThreadEvent.New, onUpdate); - room.removeListener(ThreadEvent.Update, onUpdate); - room.removeListener(ThreadEvent.NewReply, onUpdate); + room.removeListener(ThreadEvent.New, handler); + room.removeListener(ThreadEvent.Update, handler); + room.removeListener(ThreadEvent.NewReply, handler); }; - }, [room, onUpdate]); + }, [room]); }; // Returns the number of replies in a thread, counting actual reply events From fbad8f0b33641bf88c8a268a75de101fc4b55742 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sun, 15 Mar 2026 00:31:20 -0400 Subject: [PATCH 3/3] chore: add changeset for fix/timeline-refresh-scroll --- .changeset/fix-timeline-refresh-scroll.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fix-timeline-refresh-scroll.md diff --git a/.changeset/fix-timeline-refresh-scroll.md b/.changeset/fix-timeline-refresh-scroll.md new file mode 100644 index 00000000..36d46aee --- /dev/null +++ b/.changeset/fix-timeline-refresh-scroll.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Fix spurious scroll-to-bottom on sync gap and stabilise room event listener callback refs