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). diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 5c61d9dd4..ff6a6b02d 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,50 @@ 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; + // 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; + // eslint-disable-next-line react/no-unused-prop-types + isEditing: boolean; + // eslint-disable-next-line react/no-unused-prop-types + isReplying: boolean; + // eslint-disable-next-line react/no-unused-prop-types + isOpenThread: boolean; + // eslint-disable-next-line react/no-unused-prop-types + settingsEpoch: object; +} + +// 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) => ( { 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 // 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; @@ -317,7 +376,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). @@ -582,6 +647,52 @@ export function RoomTimeline({ utils: { htmlReactParserOptions, linkifyOpts, getMemberPowerTag, parseMemberEvent }, }); + // Render function ref — updated synchronously each render so TimelineItem + // always calls the latest version (which has the current focusItem, editId, + // etc. in its closure) without needing to be a prop dep. + const renderFnRef = useRef(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)); @@ -620,8 +731,33 @@ 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: 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. + } + + // 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') { @@ -635,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; @@ -713,13 +848,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, @@ -741,9 +883,13 @@ 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. + setAtBottom(true); setIsReady(true); - }, [processedEvents.length]); + }, [processedEvents.length, setAtBottom]); useEffect(() => { if (!onEditLastMessageRef) return; @@ -890,14 +1036,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.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 f1f799308..f3ed37864 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,20 @@ 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. + * + * 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; } export interface ProcessedEvent { @@ -55,8 +69,23 @@ export function useProcessedTimeline({ hideNickAvatarEvents, isReadOnly, hideMemberInReadOnly, + 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 + // 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 +201,37 @@ 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. + // 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 && + 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, }; } diff --git a/src/app/hooks/useNotificationJumper.ts b/src/app/hooks/useNotificationJumper.ts index 43c358317..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'; @@ -52,13 +52,38 @@ 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.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; // 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 +99,11 @@ export function NotificationJumper() { getSpaceRoomPath( getCanonicalAliasOrRoomId(mx, parentSpace), roomIdOrAlias, - pending.eventId + resolvedEventId ) ); } else { - navigate(getHomeRoomPath(roomIdOrAlias, pending.eventId)); + navigate(getHomeRoomPath(roomIdOrAlias, resolvedEventId)); } } setPending(null); @@ -117,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