fix: scroll-to-bottom regression and timeline pagination fixes#529
Merged
7w1 merged 17 commits intoSableClient:devfrom Mar 28, 2026
Merged
fix: scroll-to-bottom regression and timeline pagination fixes#5297w1 merged 17 commits intoSableClient:devfrom
7w1 merged 17 commits intoSableClient:devfrom
Conversation
11 tasks
Just-Insane
changed the title
Contributor
Author
dozro
requested changes
Mar 25, 2026
## 1. useTimelinePagination: finally-block resets fetchingRef prematurely
When a paginated page returns 1–4 events and more history exists, the
paginator fires a recursive continuation call (`paginate(backwards)`)
before the outer `finally` block runs. The sequence was:
1. outer try: `fetchingRef.current[dir] = false; paginate(backwards);`
→ inner paginate runs synchronously:
checks lock (false) → sets it to `true` → hits `await` → suspends
2. outer finally: `fetchingRef.current[dir] = false;`
→ overwrites the `true` that inner paginate just wrote
This left `fetchingRef.current[dir] = false` while the inner paginate
was mid-flight. The `backwardStatus === 'loading'` guard on the scroll
handler masked the problem in most cases, but for a long chain of sparse
pages each returning 1–4 events, multiple overlapping paginations could
start. If the SDK's internal per-timeline lock was also released (or a
forward-direction call raced), events could be requested from stale
pagination tokens and be inserted out of order.
Fix: add a `continuing` local boolean. When a recursive continuation is
fired, `continuing = true` is set first. The `finally` block skips the
reset when `continuing` is true, leaving the lock in the inner paginate's
hands. The inner paginate's own `finally` releases it when it finishes.
## 2. getLinkedTimelines: O(N²) array copies in recursive accumulator
The previous recursive `collectTimelines(tl, dir, [...acc, tl])` created
N intermediate arrays for an N-timeline chain (sizes 1, 2, 3, … N),
giving O(N²) total allocations. For a power user who paginates back
through hundreds of pages of history, this becomes measurable GC pressure.
Replace with a simple iterative loop (`while (current) { push; next }`)
that builds exactly one output array in O(N) time.
Multiple UrlPreviewCard instances rendering simultaneously (e.g. after a large event batch loads) each called mx.getUrlPreview(url, ts). The SDK caches by bucketed-timestamp + URL, so identical URLs from different messages produced N separate HTTP requests. Add a module-level previewRequestCache (Map<url, Promise>) that dedups in-flight requests across all instances. Resolved promises remain cached for the session; rejected ones are evicted so later renders can retry.
Just-Insane
force-pushed
the
fix/bug-fixes
branch
from
12f04f0 to
5becfc8
Compare
Just-Insane
changed the title
Just-Insane
changed the title
Just-Insane
force-pushed
the
fix/bug-fixes
branch
from
805f273 to
3146a71
Compare
Just-Insane
changed the title
Contributor
Author
|
The sliding sync reconnect fix doesn't seem to be working properly, messages are still not showing up where they should be when re-opening the PWA. |
Just-Insane
force-pushed
the
fix/bug-fixes
branch
from
98d182b to
9f45a73
Compare
Just-Insane
changed the title
Contributor
Author
|
Removed the attempted sliding sync fixes and issues seem to have mostly gone away? Will do some more testing... |
Just-Insane
force-pushed
the
fix/bug-fixes
branch
from
fb76028 to
da3015f
Compare
Just-Insane
changed the title
The Matrix SDK for sliding sync deliberately does not call resetLiveTimeline() on limited:true (the handling is in a commented-out TODO block). This means events from a previous visit remain in the live timeline and new events get appended after them, with the backward pagination token pointing to the gap between old and new events rather than to before all events. The result is that new messages appear out of order or the gap is unreachable via the UI. Two fixes: 1. useSlidingSyncActiveRoom: reset the live timeline synchronously before the subscription request is sent on room navigation. This ensures the fresh initial:true response populates a clean timeline, not one polluted with events from the previous visit. Also removes the 100ms delay that was added as an incomplete workaround for the same issue. 2. SlidingSyncManager.onLifecycle: in RequestFinished state (fires before any room data listeners run), reset the live timeline for active-room subscriptions that receive initial:true or limited:true. This covers the reconnect/background case where the pos token expires and all active rooms get a fresh initial:true in the same sync cycle.
Just-Insane
force-pushed
the
fix/bug-fixes
branch
from
da3015f to
38057ee
Compare
Contributor
Author
|
Issues with sliding sync seem to be resolved now |
RoomTimeline.tsx: - Add liveTimelineLinked guard to the initial-scroll useLayoutEffect. Without this, a render with stale data from the previous room (before the room-change reset propagates) fires scrollToIndex at the wrong index and marks hasInitialScrolledRef = true, blocking the correct scroll when the right data arrives. - Add pendingReadyRef to defer setIsReady(true) when the 80 ms timer fires but processedEvents is empty (e.g. the onLifecycle reset cleared the timeline within the window). A recovery useLayoutEffect watches for processedEvents.length becoming non-empty and performs the final scroll + setIsReady when the flag is set. Previously setIsReady(true) was called unconditionally from the timer, revealing the room at position 0 when events had not yet arrived. - Reset pendingReadyRef on room change so dangling recovery state from a previous room cannot fire in the new room context. useTimelineSync.ts: - After await paginateEventTimeline, re-read from timelineRef.current instead of using the pre-await lTimelines capture. A sliding sync reset (resetLiveTimeline) during pagination replaces lTimelines[0] with a new EventTimeline object; the stale reference causes getLinkedTimelines to traverse the wrong chain, making countAfter/countBefore comparisons wrong and triggering incorrect pagination decisions. - Add room-change useEffect that resets the timeline state to the new room's initial linked timelines when room.roomId changes. Without this, the component retains stale data from the previous room, keeping liveTimelineLinked false until a TimelineReset event fires -- which may never happen for revisited rooms where the sliding sync response does not include initial:true.
Just-Insane
changed the title
Just-Insane
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes regressions in the room timeline experience after the navigation/timeline-reset refactor, focusing on correct initial scrolling, safer pagination across sliding-sync resets, and reducing duplicate network activity.
Changes:
- Reset/guard timeline state to prevent stale-room data from breaking initial scroll-to-bottom.
- Harden pagination against sliding-sync timeline resets and pagination races.
- Deduplicate URL preview requests and remove redundant sliding-sync spidering startup work.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/client/slidingSync.ts | Resets live timelines for active rooms on initial/limited sliding-sync room updates before room data processing. |
| src/client/initMatrix.ts | Removes automatic background spidering startup. |
| src/app/utils/timeline.ts | Replaces recursive linked-timeline collection with an iterative approach. |
| src/app/hooks/useSlidingSyncActiveRoom.ts | Subscribes immediately to the active room (removes delayed subscription). |
| src/app/hooks/timeline/useTimelineSync.ts | Adds room-change timeline reset + pagination robustness against stale timeline chains and races. |
| src/app/features/room/RoomTimeline.tsx | Adds guards/timers to avoid incorrect initial scroll and prevent repeated unread scroll attempts. |
| src/app/components/url-preview/UrlPreviewCard.tsx | Adds module-level in-flight request deduplication for URL previews. |
| .changeset/fix-bug-fixes.md | Adds a patch changeset entry describing the fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Clear initialScrollTimerRef when room changes to prevent stale timer from firing in a new room after navigation (RoomTimeline.tsx) - Scope URL preview cache by MatrixClient to prevent cross-account deduplication when multiple clients/sessions exist (UrlPreviewCard.tsx) - Delete cache entries after promise settles (both success/rejection) to prevent unbounded cache growth over long sessions (UrlPreviewCard.tsx) - Fix pagination token direction check: use appropriate timeline end and token direction based on pagination direction (forward/backward) instead of always checking first timeline's backward token (useTimelineSync.ts)
Contributor
Author
|
Addressed the outstanding Copilot review items on
Latest PR checks are all green: format, lint, typecheck, knip, tests, and build. |
Contributor
Author
|
@dozro Can you check this again? It should be good now. |
7w1
approved these changes
Mar 28, 2026
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 28, 2026
> [!IMPORTANT] > Merging this PR will create a new release. ## Features * Add ability to click on usernames in member and state events to view user info ([#536](#536) by @thundertheidiot) * Add black theme ([#437](#437) by @Elec3137) * added a limited compatibility with `pk;member` commands ([#550](#550) by @dozro) * Add /location sharing command, and a /sharemylocation command. ([#509](#509) by @nushea) * added option to use shorthands to send a message with a Persona, for example `✨:test` ([#550](#550) by @dozro) * Add quick reply keybinds by using <kbd>ctrl</kbd>+<kbd>up</kbd> / <kbd>ctrl</kbd>+<kbd>down</kbd> you can now cycle through the message you are replying to with keybinds ([#524](#524) by @CodeF53) * Adds a `/html` command to send HTML messages ([#560](#560) by @Vespe-r) * Add room abbreviations with hover tooltips: moderators define term/definition pairs in room settings; matching terms are highlighted in messages. ([#514](#514) by @Just-Insane) * Add support for timestamps, playlists and youtube music links for the youtube embeds ([#534](#534) by @thundertheidiot) * Add settings sync across devices via Matrix account data, with JSON export/import ([#515](#515) by @Just-Insane) ## Fixes * Add detailed error messages to forwarding failures. ([#532](#532) by @7w1) * Cap unread badge numbers at `1k+`, and something extra :) ([#484](#484) by @hazre) * Fix scroll-to-bottom after room navigation, timeline pagination reliability, and URL preview deduplication. ([#529](#529) by @Just-Insane) * Fixes the most recent pmp message in encrypted rooms not consistently rendering the pmp and not grouping with previous pmps. ([#526](#526) by @7w1) * fixed sending sticker and attachments while having a persona selected ([#525](#525) by @dozro) * Fix push notifications missing sender/room avatar and showing stale display names when using event_id_only push format. ([#551](#551) by @Just-Insane) * Sanitize formatted reply previews before rendering to prevent unsafe HTML from being parsed in reply snippets. ([#569](#569) by @Just-Insane) * Fix broken link to Sliding Sync known issues — now points to #39 instead of the old repository. ([#519](#519) by @Just-Insane) * Fix service worker authenticated media requests returning 401 errors after SW restart or when session data is missing/stale. ([#516](#516) by @Just-Insane) * rephrased the command describtion for `/usepmp` and made `/usepmp reset` actually reset the room association of the pmp ([#550](#550) by @dozro) * Fix confusing ui with `Client Side Embeds in Encrypted Rooms` setting ([#535](#535) by @thundertheidiot) * fix forwarding metadata by removing the `null` value ([#540](#540) by @dozro) * fix forwarding issue for users on synapse homeservers, by removing the relation ([#558](#558) by @dozro) * fixed the syntax issues regarding `/addpmp` and `usepmp` (note that the syntax for `/usepmp` has changed) ([#550](#550) by @dozro) * fix the display of jumbo emojis on messages sent with a persona ([#530](#530) by @dozro) * Fix sidebar notification badge positioning so unread and unverified counts align consistently. ([#484](#484) by @hazre) * Use the browser's native compact number formatting for room and member counts. ([#484](#484) by @hazre) * fix(sentry): scrub percent-encoded Matrix IDs and opaque base64url tokens from Sentry URLs ([#531](#531) by @Just-Insane) ## Notes * new/changed bios will now also be saved in the format MSC4440 expects ([#559](#559) by @dozro) * moved the setting for filtering pronouns by language from experimental to the appearance setting ([#521](#521) by @dozro)
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
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.
Description
Fixes several timeline bugs introduced after the timeline-reset-on-navigation refactor:
Fixes #
Type of change
Checklist:
AI disclosure:
RoomTimeline.tsx: Added aliveTimelineLinkedguard so the initial scroll doesn't fire with stale data from the previous room, and apendingReadyRefthat deferssetIsReady(true)whenprocessedEventsis still empty (preventing the room opening at position 0).useTimelineSync.ts: AuseEffectresets the timeline state on room change so revisited rooms aren't stuck withliveTimelineLinked = false. ReadslinkedTimelinesfresh after eachawaitin pagination so a sliding sync reset mid-paginate doesn't cause count comparisons on a detached chain. Acontinuingflag prevents two concurrent pagination calls from racing on the lock. URL preview dedup uses a module-levelMapto share in-flight promises for the same URL. Removed a redundantstartSpideringsubscription call in the sync setup path.