fix(timeline): stable callback refs & prevent spurious scroll-to-bottom on sync gap#277
Closed
Just-Insane wants to merge 3 commits intoSableClient:devfrom
Closed
fix(timeline): stable callback refs & prevent spurious scroll-to-bottom on sync gap#277Just-Insane wants to merge 3 commits intoSableClient:devfrom
Just-Insane wants to merge 3 commits intoSableClient:devfrom
Conversation
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.
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.
Just-Insane
force-pushed
the
fix/timeline-refresh-scroll
branch
from
3d17e50 to
fbad8f0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five related fixes that address scroll stability and event-listener accumulation triggered by sync error/retry cycles.