From 620512cc2b1d231e1d4121e70208dd7e267f5336 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Thu, 9 Apr 2026 20:05:50 -0400 Subject: [PATCH 01/10] perf: memoize VList timeline items to prevent mass re-renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - useTimelineSync: add mutationVersion counter, incremented only on mutations (reactions, edits, local-echo, thread updates) via a new triggerMutation() callback. Live event arrivals do NOT bump it — the eventsLength change already signals those. - useProcessedTimeline: add stableRefsCache (useRef) that reuses the same ProcessedEvent object across renders when mutationVersion is unchanged and structural fields (collapsed, dividers, eventSender) are identical. New mutationVersion bypasses the cache so fresh objects reach React on actual content mutations. - RoomTimeline: define TimelineItem as React.memo outside the component function so the type is stable. Render via renderFnRef (synchronously updated each cycle) to avoid stale closures without adding to deps. Per-item boolean props (isHighlighted, isEditing, isReplying, isOpenThread) and a settingsEpoch object let memo skip re-renders on unchanged items while still re-rendering the one item that changed. vListIndices deps changed from timelineSync.timeline (always a new object from spread) to timelineSync.timeline.linkedTimelines + timelineSync.mutationVersion. Expected gains: Scrolling: 0 item re-renders (was: all visible items) New message: 1 item re-renders (was: all) focusItem/editId change: 1-2 items (was: all) Reactions/edits/mutations: all items (same as before, content changed) Settings change: all items via settingsEpoch (same as before) --- src/app/features/room/RoomTimeline.tsx | 120 ++++++++++++++++-- .../hooks/timeline/useProcessedTimeline.ts | 43 ++++++- src/app/hooks/timeline/useTimelineSync.ts | 29 +++-- 3 files changed, 167 insertions(+), 25 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 5c61d9dd4..95e45f147 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -1,6 +1,7 @@ import { Fragment, ReactNode, + memo, useCallback, useEffect, useLayoutEffect, @@ -78,6 +79,49 @@ import { ProcessedEvent, useProcessedTimeline } from '$hooks/timeline/useProcess import { useTimelineEventRenderer } from '$hooks/timeline/useTimelineEventRenderer'; import * as css from './RoomTimeline.css'; +/** Render function type passed to the memoized TimelineItem via a ref. */ +type TimelineRenderFn = (eventData: ProcessedEvent) => ReactNode; + +/** + * Renders one timeline item. Defined outside RoomTimeline so React never + * recreates the component type, and wrapped in `memo` so it skips re-renders + * when neither the event data nor any per-item volatile state changed. + * + * The actual rendering is delegated to `renderRef.current` (always the latest + * version of `renderMatrixEvent`, set synchronously during each render cycle) + * so stale-closure issues are avoided. + * + * Props not used in the function body (`isHighlighted`, `isEditing`, etc.) are + * intentionally included: React.memo's default shallow-equality comparator + * inspects ALL props, so changing one of them for a specific item causes only + * that item to re-render (e.g. only the message being edited re-renders when + * editId changes). + */ +interface TimelineItemProps { + data: ProcessedEvent; + renderRef: React.MutableRefObject; + /** Changed when this specific item becomes highlighted / un-highlighted. */ + isHighlighted: boolean; + /** Changed when this specific item enters / exits edit mode. */ + isEditing: boolean; + /** Changed when this specific item is the active reply target. */ + isReplying: boolean; + /** Changed when this specific item's thread drawer is open. */ + isOpenThread: boolean; + /** + * Opaque object whose identity changes when any global render-affecting + * setting changes (layout, spacing, nicknames, permissions…). Forces all + * visible items to re-render when settings change. + */ + settingsEpoch: object; +} + +const TimelineItem = memo(function TimelineItem({ data, renderRef }: TimelineItemProps) { + // isHighlighted, isEditing, isReplying, isOpenThread, settingsEpoch are not + // used here directly — their sole purpose is to guide React.memo comparisons. + return <>{renderRef.current?.(data)}; +}); + const TimelineFloat = as<'div', css.TimelineFloatVariants>( ({ position, className, ...props }, ref) => ( (null); + renderFnRef.current = (eventData: ProcessedEvent) => + renderMatrixEvent( + eventData.mEvent.getType(), + typeof eventData.mEvent.getStateKey() === 'string', + eventData.id, + eventData.mEvent, + eventData.itemIndex, + eventData.timelineSet, + eventData.collapsed + ); + + // Object whose identity changes when any global render-affecting setting + // changes. TimelineItem memo sees the new reference and re-renders all items. + const settingsEpoch = useMemo( + () => ({}), + // Any setting that changes how ALL items are rendered should be listed here. + // eslint-disable-next-line react-hooks/exhaustive-deps + [ + messageLayout, + messageSpacing, + hideReads, + showDeveloperTools, + hour24Clock, + dateFormatString, + mediaAutoLoad, + showBundledPreview, + showUrlPreview, + showClientUrlPreview, + autoplayStickers, + hideMemberInReadOnly, + isReadOnly, + hideMembershipEvents, + hideNickAvatarEvents, + showHiddenEvents, + reducedMotion, + nicknames, + imagePackRooms, + htmlReactParserOptions, + linkifyOpts, + ] + ); + const tryAutoMarkAsRead = useCallback(() => { if (!readUptoEventIdRef.current) { requestAnimationFrame(() => markAsRead(mx, room.roomId, hideReads)); @@ -713,13 +803,20 @@ export function RoomTimeline({ : timelineSync.eventsLength; const vListIndices = useMemo( () => Array.from({ length: vListItemCount }, (_, i) => i), + // timelineSync.timeline.linkedTimelines: recompute when the timeline structure + // changes (pagination, room switch). timelineSync.mutationVersion: recompute + // when event content mutates (reactions, edits) without changing the count. + // Using the linkedTimelines reference (not the timeline wrapper object) means + // a setTimeline spread for a live event arrival does NOT recompute this — the + // eventsLength / vListItemCount change already covers that case. // eslint-disable-next-line react-hooks/exhaustive-deps - [vListItemCount, timelineSync.timeline] + [vListItemCount, timelineSync.timeline.linkedTimelines, timelineSync.mutationVersion] ); const processedEvents = useProcessedTimeline({ items: vListIndices, linkedTimelines: timelineSync.timeline.linkedTimelines, + mutationVersion: timelineSync.mutationVersion, ignoredUsersSet, showHiddenEvents, showTombstoneEvents, @@ -890,14 +987,19 @@ export function RoomTimeline({ return ; } - const renderedEvent = renderMatrixEvent( - eventData.mEvent.getType(), - typeof eventData.mEvent.getStateKey() === 'string', - eventData.id, - eventData.mEvent, - eventData.itemIndex, - eventData.timelineSet, - eventData.collapsed + const renderedEvent = ( + ); const dividers = ( diff --git a/src/app/hooks/timeline/useProcessedTimeline.ts b/src/app/hooks/timeline/useProcessedTimeline.ts index f1f799308..4b24b83ae 100644 --- a/src/app/hooks/timeline/useProcessedTimeline.ts +++ b/src/app/hooks/timeline/useProcessedTimeline.ts @@ -1,4 +1,4 @@ -import { useMemo } from 'react'; +import { useMemo, useRef } from 'react'; import { MatrixEvent, EventTimelineSet, EventTimeline } from '$types/matrix-sdk'; import { getTimelineAndBaseIndex, @@ -20,6 +20,15 @@ export interface UseProcessedTimelineOptions { hideNickAvatarEvents: boolean; isReadOnly: boolean; hideMemberInReadOnly: boolean; + /** + * Increment this whenever existing event content mutates (reactions, edits, + * thread updates, local-echo). When it changes, `useProcessedTimeline` + * creates fresh `ProcessedEvent` objects so downstream `React.memo` item + * components re-render to reflect updated content. When unchanged (e.g. a + * new event was appended), existing objects are reused by identity, letting + * memo bail out for unchanged items. + */ + mutationVersion: number; } export interface ProcessedEvent { @@ -55,8 +64,23 @@ export function useProcessedTimeline({ hideNickAvatarEvents, isReadOnly, hideMemberInReadOnly, + mutationVersion, }: UseProcessedTimelineOptions): ProcessedEvent[] { + // Stable-ref cache: reuse the same ProcessedEvent object for an event when + // nothing structural changed. This lets React.memo on item components bail + // out for the majority of items when only a new message was appended. + const stableRefsCache = useRef>(new Map()); + const prevMutationVersionRef = useRef(-1); + return useMemo(() => { + // When mutationVersion changes, existing event content has mutated (reaction + // added, message edited, local-echo updated, thread reply). Create fresh + // objects so memo item components re-render. When version is unchanged (only + // items count changed), reuse cached refs for structurally-identical events. + const isMutation = mutationVersion !== prevMutationVersionRef.current; + prevMutationVersionRef.current = mutationVersion; + const prevCache = isMutation ? null : stableRefsCache.current; + let prevEvent: MatrixEvent | undefined; let isPrevRendered = false; let newDivider = false; @@ -172,18 +196,33 @@ export function useProcessedTimeline({ willRenderDayDivider, }; + // Reuse the previous ProcessedEvent object if all structural fields match, + // so that React.memo on timeline item components can bail out cheaply. + const prev = prevCache?.get(mEventId); + const stable = + prev && + prev.collapsed === collapsed && + prev.willRenderNewDivider === willRenderNewDivider && + prev.willRenderDayDivider === willRenderDayDivider && + prev.eventSender === eventSender + ? prev + : processed; + prevEvent = mEvent; isPrevRendered = true; if (willRenderNewDivider) newDivider = false; if (willRenderDayDivider) dayDivider = false; - acc.push(processed); + acc.push(stable); return acc; }, []); + // Update the stable-ref cache for the next render. + stableRefsCache.current = new Map(result.map((e) => [e.id, e])); return result; }, [ items, linkedTimelines, + mutationVersion, ignoredUsersSet, showHiddenEvents, showTombstoneEvents, diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index 51c85dda8..ca52b5442 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -376,6 +376,16 @@ export function useTimelineSync({ eventId ? getEmptyTimeline() : { linkedTimelines: getInitialTimeline(room).linkedTimelines } ); + // Incremented whenever existing event content mutates (reactions, edits, thread + // updates, local-echo status) but NOT on live-event arrivals (those are signalled + // by eventsLength increasing). Consumers use this to decide whether to + // re-create ProcessedEvent objects for stable-ref memoization. + const [mutationVersion, setMutationVersion] = useState(0); + const triggerMutation = useCallback(() => { + setTimeline((ct) => ({ ...ct })); + setMutationVersion((v) => v + 1); + }, []); + const [focusItem, setFocusItem] = useState< | { index: number; @@ -512,14 +522,14 @@ export function useTimelineSync({ eventRoom: Room | undefined ) => { if (eventRoom?.roomId !== room.roomId) return; - setTimeline((ct) => ({ ...ct })); + triggerMutation(); }; room.on(RoomEvent.LocalEchoUpdated, handleLocalEchoUpdated); return () => { room.removeListener(RoomEvent.LocalEchoUpdated, handleLocalEchoUpdated); }; - }, [room, setTimeline]); + }, [room, triggerMutation]); useLiveTimelineRefresh( room, @@ -533,19 +543,9 @@ export function useTimelineSync({ }, [room, isAtBottomRef, scrollToBottom]) ); - useRelationUpdate( - room, - useCallback(() => { - setTimeline((ct) => ({ ...ct })); - }, []) - ); + useRelationUpdate(room, triggerMutation); - useThreadUpdate( - room, - useCallback(() => { - setTimeline((ct) => ({ ...ct })); - }, []) - ); + useThreadUpdate(room, triggerMutation); useEffect(() => { const resetAutoScrollPending = resetAutoScrollPendingRef.current; @@ -605,5 +605,6 @@ export function useTimelineSync({ loadEventTimeline, focusItem, setFocusItem, + mutationVersion, }; } From 2d48f6021ac158e716141486e04add85721f055b Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Thu, 9 Apr 2026 20:39:12 -0400 Subject: [PATCH 02/10] fix(timeline): make mutationVersion optional in UseProcessedTimelineOptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ThreadDrawer calls useProcessedTimeline without an equivalent mutation counter (it doesn't use useTimelineSync). Making the field optional with a default of 0 means: - ThreadDrawer gets stable-ref caching for free on subsequent renders (isMutation=false after first render), which is correct — it doesn't wrap items in React.memo TimelineItem. - RoomTimeline continues to pass the real mutationVersion so its TimelineItem memo components are refreshed when content mutates. - pnpm typecheck / pnpm build no longer fail with TS2345. --- src/app/hooks/timeline/useProcessedTimeline.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/app/hooks/timeline/useProcessedTimeline.ts b/src/app/hooks/timeline/useProcessedTimeline.ts index 4b24b83ae..ac38d6c80 100644 --- a/src/app/hooks/timeline/useProcessedTimeline.ts +++ b/src/app/hooks/timeline/useProcessedTimeline.ts @@ -27,8 +27,13 @@ export interface UseProcessedTimelineOptions { * components re-render to reflect updated content. When unchanged (e.g. a * new event was appended), existing objects are reused by identity, letting * memo bail out for unchanged items. + * + * Optional — defaults to 0 (stable refs always applied after first render). + * Call sites that do NOT use `React.memo` item components (e.g. `ThreadDrawer`) + * can omit this; the SDK mutates `mEvent` in place so rendered content stays + * correct regardless of object identity. */ - mutationVersion: number; + mutationVersion?: number; } export interface ProcessedEvent { @@ -64,7 +69,7 @@ export function useProcessedTimeline({ hideNickAvatarEvents, isReadOnly, hideMemberInReadOnly, - mutationVersion, + mutationVersion = 0, }: UseProcessedTimelineOptions): ProcessedEvent[] { // Stable-ref cache: reuse the same ProcessedEvent object for an event when // nothing structural changed. This lets React.memo on item components bail From 90c5ff1035bbd421b3368ff56779ac1004719205 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Thu, 9 Apr 2026 22:51:21 -0400 Subject: [PATCH 03/10] fix(timeline): satisfy lint rules in TimelineItem memo component --- src/app/features/room/RoomTimeline.tsx | 27 +++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 95e45f147..60c219596 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -100,27 +100,28 @@ type TimelineRenderFn = (eventData: ProcessedEvent) => ReactNode; interface TimelineItemProps { data: ProcessedEvent; renderRef: React.MutableRefObject; - /** Changed when this specific item becomes highlighted / un-highlighted. */ + // The props below are not read in the component body — they exist solely so + // React.memo's shallow-equality comparator sees them and re-renders only the + // affected item when they change. + // eslint-disable-next-line react/no-unused-prop-types isHighlighted: boolean; - /** Changed when this specific item enters / exits edit mode. */ + // eslint-disable-next-line react/no-unused-prop-types isEditing: boolean; - /** Changed when this specific item is the active reply target. */ + // eslint-disable-next-line react/no-unused-prop-types isReplying: boolean; - /** Changed when this specific item's thread drawer is open. */ + // eslint-disable-next-line react/no-unused-prop-types isOpenThread: boolean; - /** - * Opaque object whose identity changes when any global render-affecting - * setting changes (layout, spacing, nicknames, permissions…). Forces all - * visible items to re-render when settings change. - */ + // eslint-disable-next-line react/no-unused-prop-types settingsEpoch: object; } -const TimelineItem = memo(function TimelineItem({ data, renderRef }: TimelineItemProps) { - // isHighlighted, isEditing, isReplying, isOpenThread, settingsEpoch are not - // used here directly — their sole purpose is to guide React.memo comparisons. +// Declared outside memo() so the callback receives a reference, not an inline +// function expression (satisfies prefer-arrow-callback). +function TimelineItemInner({ data, renderRef }: TimelineItemProps) { return <>{renderRef.current?.(data)}; -}); +} +const TimelineItem = memo(TimelineItemInner); +TimelineItem.displayName = 'TimelineItem'; const TimelineFloat = as<'div', css.TimelineFloatVariants>( ({ position, className, ...props }, ref) => ( From e3972bb567aa3798e0dca6b234789b7970b02a6b Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Thu, 9 Apr 2026 22:51:34 -0400 Subject: [PATCH 04/10] chore: add changeset for perf-timeline-item-memo --- .changeset/perf-timeline-item-memo.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/perf-timeline-item-memo.md diff --git a/.changeset/perf-timeline-item-memo.md b/.changeset/perf-timeline-item-memo.md new file mode 100644 index 000000000..1471e3d0a --- /dev/null +++ b/.changeset/perf-timeline-item-memo.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Memoize individual VList timeline items to prevent mass re-renders when unrelated state changes (e.g. typing indicators, read receipts, or new messages while not at the bottom). From 443d55cad224f187e012ed3dd009daf1c13dc1ac Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Thu, 9 Apr 2026 23:19:45 -0400 Subject: [PATCH 05/10] fix(timeline): preempt atBottom to prevent Jump to Latest flashing at bottom --- src/app/features/room/RoomTimeline.tsx | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 60c219596..9f5c5e912 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -353,6 +353,9 @@ export function RoomTimeline({ // was empty when the timer fired (e.g. the onLifecycle reset cleared the // timeline within the 80 ms window), defer setIsReady until the recovery // effect below fires once events repopulate. + // scrollToIndex is async; pre-empt atBottom so the "Jump to Latest" + // button doesn't flash for one render cycle before onScroll confirms. + setAtBottom(true); setIsReady(true); } else { pendingReadyRef.current = true; @@ -362,7 +365,13 @@ export function RoomTimeline({ } // No cleanup return — the timer must survive eventsLength fluctuations. // It is cancelled on unmount by the dedicated effect below. - }, [timelineSync.eventsLength, timelineSync.liveTimelineLinked, eventId, room.roomId]); + }, [ + timelineSync.eventsLength, + timelineSync.liveTimelineLinked, + eventId, + room.roomId, + setAtBottom, + ]); // Cancel the initial-scroll timer on unmount (the useLayoutEffect above // intentionally does not cancel it when deps change). @@ -840,8 +849,11 @@ export function RoomTimeline({ if (processedEvents.length === 0) return; pendingReadyRef.current = false; vListRef.current?.scrollToIndex(processedEvents.length - 1, { align: 'end' }); + // scrollToIndex is async; pre-empt atBottom so the "Jump to Latest" button + // doesn't flash for one render cycle before onScroll confirms the position. + setAtBottom(true); setIsReady(true); - }, [processedEvents.length]); + }, [processedEvents.length, setAtBottom]); useEffect(() => { if (!onEditLastMessageRef) return; From 9710aa01d5b4c969102f6f02ccea04b6ec96ac48 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Fri, 10 Apr 2026 08:58:01 -0400 Subject: [PATCH 06/10] fix(timeline): suppress intermediate VList scroll events after programmatic scroll-to-bottom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After setIsReady(true) commits, virtua can fire onScroll events with isNowAtBottom=false during its height-correction pass (particularly on first visit when item heights above the viewport haven't been rendered yet). These intermediate events were driving atBottomState to false while isReady=true, flashing the 'Jump to Latest' button. Add programmaticScrollToBottomRef: set it before each scrollToIndex bottom-scroll, suppress the first intermediate false event (clearing the guard immediately), so the next event — the corrected position or a real user scroll — is processed normally. --- src/app/features/room/RoomTimeline.tsx | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 9f5c5e912..d3df812d3 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -265,6 +265,12 @@ export function RoomTimeline({ // A recovery useLayoutEffect watches for processedEvents becoming non-empty // and performs the final scroll + setIsReady when this flag is set. const pendingReadyRef = useRef(false); + // Set to true before each programmatic scroll-to-bottom so intermediate + // onScroll events from virtua's height-correction pass cannot drive + // atBottomState to false (flashing the "Jump to Latest" button). + // Cleared when VList confirms isNowAtBottom, or on the first intermediate + // event so subsequent user-initiated scrolls are tracked normally. + const programmaticScrollToBottomRef = useRef(false); const currentRoomIdRef = useRef(room.roomId); const [isReady, setIsReady] = useState(false); @@ -274,6 +280,7 @@ export function RoomTimeline({ mountScrollWindowRef.current = Date.now() + 3000; currentRoomIdRef.current = room.roomId; pendingReadyRef.current = false; + programmaticScrollToBottomRef.current = false; if (initialScrollTimerRef.current !== undefined) { clearTimeout(initialScrollTimerRef.current); initialScrollTimerRef.current = undefined; @@ -348,6 +355,7 @@ export function RoomTimeline({ initialScrollTimerRef.current = setTimeout(() => { initialScrollTimerRef.current = undefined; if (processedEventsRef.current.length > 0) { + programmaticScrollToBottomRef.current = true; vListRef.current?.scrollToIndex(processedEventsRef.current.length - 1, { align: 'end' }); // Only mark ready once we've successfully scrolled. If processedEvents // was empty when the timer fired (e.g. the onLifecycle reset cleared the @@ -720,8 +728,20 @@ export function RoomTimeline({ const distanceFromBottom = v.scrollSize - offset - v.viewportSize; const isNowAtBottom = distanceFromBottom < 100; + // Clear the programmatic-scroll guard whenever VList confirms we are at the + // bottom, regardless of whether atBottomRef needs updating. + if (isNowAtBottom) programmaticScrollToBottomRef.current = false; if (isNowAtBottom !== atBottomRef.current) { - setAtBottom(isNowAtBottom); + if (isNowAtBottom || !programmaticScrollToBottomRef.current) { + setAtBottom(isNowAtBottom); + } else { + // VList fired an intermediate "not at bottom" event while settling after + // a programmatic scroll-to-bottom (e.g. height-correction pass). Suppress + // the false negative and clear the guard so the next event — either a + // VList correction to the true bottom, or a genuine user scroll — is + // processed normally. + programmaticScrollToBottomRef.current = false; + } } if (offset < 500 && canPaginateBackRef.current && backwardStatusRef.current === 'idle') { @@ -848,6 +868,7 @@ export function RoomTimeline({ if (!pendingReadyRef.current) return; if (processedEvents.length === 0) return; pendingReadyRef.current = false; + programmaticScrollToBottomRef.current = true; vListRef.current?.scrollToIndex(processedEvents.length - 1, { align: 'end' }); // scrollToIndex is async; pre-empt atBottom so the "Jump to Latest" button // doesn't flash for one render cycle before onScroll confirms the position. From bdaee47d7506cc66f376b3638b342cc5630ceb66 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Fri, 10 Apr 2026 18:10:50 -0400 Subject: [PATCH 07/10] fix(timeline): set programmatic guard in scrollToBottom; keep guard active through all intermediate VList events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously programmaticScrollToBottomRef was only set at a few specific call-sites and cleared after the first suppressed intermediate event. VList fires several height-correction scroll events after scrollTo(); the second one (after the clear) would call setAtBottom(false) and flash "Jump to Latest". - Move programmaticScrollToBottomRef.current = true into scrollToBottom() itself so all callers (live message arrival, timeline refresh, auto-scroll effect) are automatically guarded without missing a call-site. - Remove the guard clear in the else branch; the guard now stays active until VList explicitly confirms isNowAtBottom = true. fix(notifications): skip loadEventTimeline when event is already in live timeline When a notification tap opens a room, NotificationJumper was always navigating with the eventId URL path which triggered loadEventTimeline → roomInitialSync. If sliding sync had already delivered the event to the live timeline this produced a sparse historical slice that (a) looked like a brand-new chat and (b) left the room empty when the user navigated away and returned without the eventId. Check whether the event is in the live timeline before navigating; if it is present, open the room at the live bottom instead. Historical events still use the eventId path. --- src/app/features/room/RoomTimeline.tsx | 16 +++++++++------- src/app/hooks/useNotificationJumper.ts | 25 +++++++++++++++++++++---- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index d3df812d3..238237ab3 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -295,6 +295,9 @@ export function RoomTimeline({ if (!vListRef.current) return; const lastIndex = processedEventsRef.current.length - 1; if (lastIndex < 0) return; + // Guard against VList's intermediate height-correction scroll events that + // would otherwise call setAtBottom(false) before the scroll settles. + programmaticScrollToBottomRef.current = true; vListRef.current.scrollTo(vListRef.current.scrollSize); }, []); @@ -734,14 +737,13 @@ export function RoomTimeline({ if (isNowAtBottom !== atBottomRef.current) { if (isNowAtBottom || !programmaticScrollToBottomRef.current) { setAtBottom(isNowAtBottom); - } else { - // VList fired an intermediate "not at bottom" event while settling after - // a programmatic scroll-to-bottom (e.g. height-correction pass). Suppress - // the false negative and clear the guard so the next event — either a - // VList correction to the true bottom, or a genuine user scroll — is - // processed normally. - programmaticScrollToBottomRef.current = false; } + // else: programmatic guard active — suppress the false-negative and keep + // the guard set. VList can fire several intermediate "not at bottom" + // events while it corrects item heights after a scrollTo(); clearing the + // guard on the first one would let the second cause a spurious + // setAtBottom(false) and flash the "Jump to Latest" button. The guard + // is cleared above (unconditionally) when isNowAtBottom becomes true. } if (offset < 500 && canPaginateBackRef.current && backwardStatusRef.current === 'idle') { diff --git a/src/app/hooks/useNotificationJumper.ts b/src/app/hooks/useNotificationJumper.ts index 43c358317..1eabd1cf4 100644 --- a/src/app/hooks/useNotificationJumper.ts +++ b/src/app/hooks/useNotificationJumper.ts @@ -52,13 +52,30 @@ export function NotificationJumper() { const isJoined = room?.getMyMembership() === 'join'; if (isSyncing && isJoined) { - log.log('jumping to:', pending.roomId, pending.eventId); + // If the notification event is already in the room's live timeline (i.e. + // sliding sync has already delivered it), open the room at the live bottom + // rather than using the eventId URL path. The eventId path triggers + // loadEventTimeline → roomInitialSync which loads a historical slice that + // (a) may look like a brand-new chat if the event is the only one in the + // slice, and (b) makes the room appear empty when the user navigates away + // and returns without the eventId, because the sliding-sync live timeline + // hasn't been populated yet. Omitting the eventId for events already in + // the live timeline lets the room open normally at the bottom where the + // new message is visible. Historical events (not in live timeline) still + // use the eventId so loadEventTimeline can jump to the right context. + const liveEvents = room?.getUnfilteredTimelineSet?.()?.getLiveTimeline?.()?.getEvents?.(); + const eventInLive = + pending.eventId && liveEvents + ? liveEvents.some((e) => e.getId() === pending.eventId) + : false; + const resolvedEventId = eventInLive ? undefined : pending.eventId; + log.log('jumping to:', pending.roomId, resolvedEventId, { eventInLive }); jumpingRef.current = true; // Navigate directly to home or direct path — bypasses space routing which // on mobile shows the space-nav panel first instead of the room timeline. const roomIdOrAlias = getCanonicalAliasOrRoomId(mx, pending.roomId); if (mDirects.has(pending.roomId)) { - navigate(getDirectRoomPath(roomIdOrAlias, pending.eventId)); + navigate(getDirectRoomPath(roomIdOrAlias, resolvedEventId)); } else { // If the room lives inside a space, route through the space path so // SpaceRouteRoomProvider can resolve it — HomeRouteRoomProvider only @@ -74,11 +91,11 @@ export function NotificationJumper() { getSpaceRoomPath( getCanonicalAliasOrRoomId(mx, parentSpace), roomIdOrAlias, - pending.eventId + resolvedEventId ) ); } else { - navigate(getHomeRoomPath(roomIdOrAlias, pending.eventId)); + navigate(getHomeRoomPath(roomIdOrAlias, resolvedEventId)); } } setPending(null); From 6bc44a177892dae0fb81bc8d2598a19ca0a49a73 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sat, 11 Apr 2026 11:13:25 -0400 Subject: [PATCH 08/10] fix(notifications): defer notification jump until live timeline is non-empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the app wakes from a killed state and the user taps a notification, performJump fires during the initial sync window before the room's live timeline has been populated by sliding sync. Previously, eventInLive was always false in this case, so we fell back to loadEventTimeline → roomInitialSync which loaded a sparse historical slice. On subsequent visits to the room without the eventId the room appeared empty because the live timeline was never populated for the initial roomInitialSync result. Two changes: 1. Guard: if the live timeline is completely empty, return early and wait rather than navigating — the RoomEvent.Timeline listener below retries once events start arriving. 2. Listen on RoomEvent.Timeline for the target room so performJump re-runs as soon as the first event arrives in the room, at which point the notification event is almost certainly also present so we can navigate without eventId (avoiding loadEventTimeline entirely). --- src/app/hooks/useNotificationJumper.ts | 30 ++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/app/hooks/useNotificationJumper.ts b/src/app/hooks/useNotificationJumper.ts index 1eabd1cf4..dd47a3f24 100644 --- a/src/app/hooks/useNotificationJumper.ts +++ b/src/app/hooks/useNotificationJumper.ts @@ -1,7 +1,7 @@ import { useCallback, useEffect, useRef } from 'react'; import { useAtom, useAtomValue } from 'jotai'; import { useNavigate } from 'react-router-dom'; -import { SyncState, ClientEvent } from '$types/matrix-sdk'; +import { SyncState, ClientEvent, RoomEvent } from '$types/matrix-sdk'; import { activeSessionIdAtom, pendingNotificationAtom } from '../state/sessions'; import { mDirectAtom } from '../state/mDirectList'; import { useSyncState } from './useSyncState'; @@ -63,11 +63,19 @@ export function NotificationJumper() { // the live timeline lets the room open normally at the bottom where the // new message is visible. Historical events (not in live timeline) still // use the eventId so loadEventTimeline can jump to the right context. - const liveEvents = room?.getUnfilteredTimelineSet?.()?.getLiveTimeline?.()?.getEvents?.(); - const eventInLive = - pending.eventId && liveEvents - ? liveEvents.some((e) => e.getId() === pending.eventId) - : false; + const liveEvents = + room?.getUnfilteredTimelineSet?.()?.getLiveTimeline?.()?.getEvents?.() ?? []; + const eventInLive = pending.eventId + ? liveEvents.some((e) => e.getId() === pending.eventId) + : false; + // If the live timeline is empty the room hasn't been populated by sliding + // sync yet. Defer navigation and let the RoomEvent.Timeline listener below + // retry once events arrive — by then the notification event will almost + // certainly be in the live timeline and we can skip loadEventTimeline. + if (!eventInLive && liveEvents.length === 0) { + log.log('live timeline empty, deferring jump...', { roomId: pending.roomId }); + return; + } const resolvedEventId = eventInLive ? undefined : pending.eventId; log.log('jumping to:', pending.roomId, resolvedEventId, { eventInLive }); jumpingRef.current = true; @@ -134,11 +142,21 @@ export function NotificationJumper() { if (!pending) return undefined; const onRoom = () => performJumpRef.current(); + // Re-check once events arrive in the target room — this fires shortly after + // the initial sync populates the live timeline, letting us verify whether + // the notification event is already there before falling back to + // loadEventTimeline (which creates a sparse historical slice that may make + // the room appear empty on subsequent visits without the eventId). + const onTimeline = (_evt: unknown, eventRoom: { roomId?: string } | undefined) => { + if (eventRoom?.roomId === pending.roomId) performJumpRef.current(); + }; mx.on(ClientEvent.Room, onRoom); + mx.on(RoomEvent.Timeline, onTimeline as Parameters[1]); performJumpRef.current(); return () => { mx.removeListener(ClientEvent.Room, onRoom); + mx.removeListener(RoomEvent.Timeline, onTimeline as Parameters[1]); }; }, [pending, mx]); // performJump intentionally omitted — use ref above From 168edbd544b1d92ca759893bc59c98b96102008e Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sat, 11 Apr 2026 11:15:18 -0400 Subject: [PATCH 09/10] fix(timeline): skip scroll-cache save when viewing a historical eventId slice When the user taps a push notification and the app loads a historical sparse timeline via loadEventTimeline (eventId URL path), the VList item heights for those few events were being written to the room's scroll cache. On the next visit to the room (live timeline, many more events), the RoomTimeline mount read the stale cache and passed its heights to the new VList instance. The height mismatch between the sparse and live timelines caused incorrect scroll-position restoration, making the room appear to show stale or mispositioned messages. Guard the roomScrollCache.save call with !eventId so historical views never overwrite the live-timeline cache. The next live visit will either use the pre-existing (untouched) live cache or fall back to the first-visit 80 ms measurement path. --- src/app/features/room/RoomTimeline.tsx | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 238237ab3..ff6a6b02d 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -746,6 +746,20 @@ export function RoomTimeline({ // is cleared above (unconditionally) when isNowAtBottom becomes true. } + // Keep the scroll cache fresh so the next visit to this room can restore + // position (and skip the 80 ms measurement wait) immediately on mount. + // Skip when viewing a historical slice via eventId: those item heights are + // for a sparse subset of events and would corrupt the cache for the next + // live-timeline visit, producing stale VList measurements and making the + // room appear to be at the wrong position (or visually empty) on re-entry. + if (!eventId) { + roomScrollCache.save(room.roomId, { + cache: v.cache, + scrollOffset: offset, + atBottom: isNowAtBottom, + }); + } + if (offset < 500 && canPaginateBackRef.current && backwardStatusRef.current === 'idle') { timelineSyncRef.current.handleTimelinePagination(true); } @@ -757,11 +771,10 @@ export function RoomTimeline({ timelineSyncRef.current.handleTimelinePagination(false); } }, - [setAtBottom] + [setAtBottom, room.roomId, eventId] ); const showLoadingPlaceholders = - timelineSync.eventsLength === 0 && (timelineSync.canPaginateBack || timelineSync.backwardStatus === 'loading'); let backPaginationJSX: ReactNode | undefined; From a5a99dc5612adc8dd945c160a062c8eaa9cc49ee Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sat, 11 Apr 2026 12:34:34 -0400 Subject: [PATCH 10/10] test(timeline): add useProcessedTimeline unit tests + fix stale itemIndex in stable-ref check --- .../timeline/useProcessedTimeline.test.ts | 313 ++++++++++++++++++ .../hooks/timeline/useProcessedTimeline.ts | 4 + 2 files changed, 317 insertions(+) create mode 100644 src/app/hooks/timeline/useProcessedTimeline.test.ts diff --git a/src/app/hooks/timeline/useProcessedTimeline.test.ts b/src/app/hooks/timeline/useProcessedTimeline.test.ts new file mode 100644 index 000000000..01cd1a213 --- /dev/null +++ b/src/app/hooks/timeline/useProcessedTimeline.test.ts @@ -0,0 +1,313 @@ +import { renderHook } from '@testing-library/react'; +import { describe, it, expect } from 'vitest'; +import type { EventTimeline, EventTimelineSet, MatrixEvent } from '$types/matrix-sdk'; +import { useProcessedTimeline } from './useProcessedTimeline'; + +// --------------------------------------------------------------------------- +// Minimal fakes +// --------------------------------------------------------------------------- + +function makeEvent( + id: string, + opts: { + sender?: string; + type?: string; + ts?: number; + content?: Record; + } = {} +): MatrixEvent { + const { + sender = '@alice:test', + type = 'm.room.message', + ts = 1_000, + content = { body: 'hello' }, + } = opts; + return { + getId: () => id, + getSender: () => sender, + isRedacted: () => false, + getTs: () => ts, + getType: () => type, + threadRootId: undefined, + getContent: () => content, + getRelation: () => null, + isRedaction: () => false, + } as unknown as MatrixEvent; +} + +const fakeTimelineSet = {} as EventTimelineSet; + +function makeTimeline(events: MatrixEvent[]): EventTimeline { + return { + getEvents: () => events, + getTimelineSet: () => fakeTimelineSet, + } as unknown as EventTimeline; +} + +/** Default options — keeps tests concise; individual tests override what they need. */ +const defaults = { + ignoredUsersSet: new Set(), + showHiddenEvents: false, + showTombstoneEvents: false, + mxUserId: '@alice:test', + readUptoEventId: undefined, + hideMembershipEvents: false, + hideNickAvatarEvents: false, + isReadOnly: false, + hideMemberInReadOnly: false, +} as const; + +// --------------------------------------------------------------------------- +// Helpers to derive `items` from a linked-timeline list +// index 0 = first event in first timeline, etc. +// --------------------------------------------------------------------------- +function makeItems(count: number, startIndex = 0): number[] { + return Array.from({ length: count }, (_, i) => startIndex + i); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('useProcessedTimeline', () => { + it('returns an empty array when there are no events', () => { + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: [], + linkedTimelines: [makeTimeline([])], + }) + ); + expect(result.current).toHaveLength(0); + }); + + it('returns one ProcessedEvent per visible event', () => { + const events = [makeEvent('$e1'), makeEvent('$e2'), makeEvent('$e3')]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(3), + linkedTimelines: [timeline], + }) + ); + + expect(result.current).toHaveLength(3); + expect(result.current[0].id).toBe('$e1'); + expect(result.current[2].id).toBe('$e3'); + }); + + it('filters out poll response and end events', () => { + const events = [ + makeEvent('$start', { type: 'm.poll.start' }), + makeEvent('$resp', { type: 'm.poll.response' }), + makeEvent('$end', { type: 'm.poll.end' }), + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(3), + linkedTimelines: [timeline], + showHiddenEvents: true, // ensure other filter doesn't swallow these + }) + ); + + // Only the poll start renders; response and end are always filtered. + const ids = result.current.map((e) => e.id); + expect(ids).not.toContain('$resp'); + expect(ids).not.toContain('$end'); + }); + + it('collapses consecutive messages from the same sender within 2 minutes', () => { + const events = [ + makeEvent('$e1', { ts: 1_000 }), + makeEvent('$e2', { ts: 60_000 }), // same sender, ~1 min later + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(2), + linkedTimelines: [timeline], + }) + ); + + expect(result.current[1].collapsed).toBe(true); + }); + + it('does NOT collapse messages from the same sender more than 2 minutes apart', () => { + const events = [ + makeEvent('$e1', { ts: 1_000 }), + makeEvent('$e2', { ts: 3 * 60_000 }), // 3 min later + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(2), + linkedTimelines: [timeline], + }) + ); + + expect(result.current[1].collapsed).toBe(false); + }); + + // ------------------------------------------------------------------------- + // Stable-ref optimisation + // ------------------------------------------------------------------------- + + it('reuses the same ProcessedEvent reference when nothing changed (stable-ref)', () => { + const events = [makeEvent('$e1'), makeEvent('$e2')]; + const timeline = makeTimeline(events); + const items = makeItems(2); + + const { result, rerender } = renderHook( + ({ ver }) => + useProcessedTimeline({ + ...defaults, + items, + linkedTimelines: [timeline], + mutationVersion: ver, + }), + { initialProps: { ver: 0 } } + ); + + const firstRender = result.current; + + // Re-render with the same mutationVersion — refs should be reused + rerender({ ver: 0 }); + + expect(result.current[0]).toBe(firstRender[0]); + expect(result.current[1]).toBe(firstRender[1]); + }); + + it('creates fresh ProcessedEvent objects when mutationVersion increments', () => { + const events = [makeEvent('$e1'), makeEvent('$e2')]; + const timeline = makeTimeline(events); + const items = makeItems(2); + + const { result, rerender } = renderHook( + ({ ver }) => + useProcessedTimeline({ + ...defaults, + items, + linkedTimelines: [timeline], + mutationVersion: ver, + }), + { initialProps: { ver: 0 } } + ); + + const firstRender = result.current; + + // Bump mutation version — stale refs must not be reused + rerender({ ver: 1 }); + + expect(result.current[0]).not.toBe(firstRender[0]); + expect(result.current[1]).not.toBe(firstRender[1]); + }); + + it('creates fresh ProcessedEvent objects when itemIndex shifts after back-pagination', () => { + // Initial: one event at index 0 + const existingEvent = makeEvent('$existing'); + const timelineV1 = makeTimeline([existingEvent]); + + const { result, rerender } = renderHook( + ({ linkedTimelines, items }: { linkedTimelines: EventTimeline[]; items: number[] }) => + useProcessedTimeline({ + ...defaults, + items, + linkedTimelines, + mutationVersion: 0, // unchanged — only the itemIndex changes + }), + { + initialProps: { + linkedTimelines: [timelineV1], + items: [0], + }, + } + ); + + const firstRef = result.current[0]; + expect(firstRef.id).toBe('$existing'); + expect(firstRef.itemIndex).toBe(0); + + // Back-pagination prepends a new event at the front — existing event now at index 1 + const newEvent = makeEvent('$new'); + const timelineV2 = makeTimeline([newEvent, existingEvent]); + + rerender({ linkedTimelines: [timelineV2], items: [0, 1] }); + + const existingProcessed = result.current.find((e) => e.id === '$existing')!; + // itemIndex must be 1 (updated), NOT 0 (stale from previous render) + expect(existingProcessed.itemIndex).toBe(1); + // And it must be a new object, not the stale cached ref + expect(existingProcessed).not.toBe(firstRef); + }); + + it('filters events from ignored users', () => { + const events = [ + makeEvent('$e1', { sender: '@alice:test' }), + makeEvent('$e2', { sender: '@ignored:test' }), + makeEvent('$e3', { sender: '@alice:test' }), + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(3), + linkedTimelines: [timeline], + ignoredUsersSet: new Set(['@ignored:test']), + }) + ); + + const ids = result.current.map((e) => e.id); + expect(ids).not.toContain('$e2'); + expect(ids).toContain('$e1'); + expect(ids).toContain('$e3'); + }); + + it('places willRenderNewDivider on the event immediately after readUptoEventId', () => { + const events = [ + makeEvent('$read', { sender: '@bob:test', ts: 1_000 }), + makeEvent('$new', { sender: '@bob:test', ts: 2_000 }), + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(2), + linkedTimelines: [timeline], + mxUserId: '@alice:test', // different from sender so divider renders + readUptoEventId: '$read', + }) + ); + + expect(result.current[1].willRenderNewDivider).toBe(true); + }); + + it('places willRenderDayDivider between events on different calendar days', () => { + const DAY = 86_400_000; + const events = [ + makeEvent('$e1', { ts: 1_000 }), + makeEvent('$e2', { ts: 1_000 + DAY + 1 }), // next day + ]; + const timeline = makeTimeline(events); + + const { result } = renderHook(() => + useProcessedTimeline({ + ...defaults, + items: makeItems(2), + linkedTimelines: [timeline], + }) + ); + + expect(result.current[1].willRenderDayDivider).toBe(true); + }); +}); diff --git a/src/app/hooks/timeline/useProcessedTimeline.ts b/src/app/hooks/timeline/useProcessedTimeline.ts index ac38d6c80..f3ed37864 100644 --- a/src/app/hooks/timeline/useProcessedTimeline.ts +++ b/src/app/hooks/timeline/useProcessedTimeline.ts @@ -203,9 +203,13 @@ export function useProcessedTimeline({ // Reuse the previous ProcessedEvent object if all structural fields match, // so that React.memo on timeline item components can bail out cheaply. + // itemIndex must also be equal: after back-pagination the same eventId + // shifts to a higher VList index, so a stale itemIndex would break + // getRawIndexToProcessedIndex and focus-highlight comparisons. const prev = prevCache?.get(mEventId); const stable = prev && + prev.itemIndex === processed.itemIndex && prev.collapsed === collapsed && prev.willRenderNewDivider === willRenderNewDivider && prev.willRenderDayDivider === willRenderDayDivider &&