feat(desktop): re-land virtualized timeline to fix macOS beachball#1250
Open
wpfleger96 wants to merge 11 commits into
Open
feat(desktop): re-land virtualized timeline to fix macOS beachball#1250wpfleger96 wants to merge 11 commits into
wpfleger96 wants to merge 11 commits into
Conversation
wpfleger96
force-pushed
the
duncan/timeline-virtualization-reland
branch
from
be2d9e5 to
cbfc4e6
Compare
wpfleger96
force-pushed
the
duncan/timeline-virtualization-reland
branch
4 times, most recently
from
f4927af to
77a5801
Compare
Port the virtualized timeline subsystem onto main's day-group render structure, re-threading the read-marker work through the virtualized rows. main builds every row synchronously on first mount, so cold channel-switch cost climbs with channel depth; virtualization renders only the visible window, making cost independent of depth. Ports timelineItems/scrollConvergence (+ lib tests), useLoadOlderOnScroll, useConvergentScrollToMessage, and the virtualizer-index restore in useAnchoredScroll. main's unread-counter fix is preserved, confined to the unread-count increment block. The two perf-hoist props the reference branch passed into MessageRow are dropped: virtualization already bounds rendered rows to the visible window, so the hoist optimizes a cost the mechanism eliminates, and MessageRow stays untouched. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The timeline-virtualization acceptance gate is a same-harness delta (header-arm longest-longtask <= B + 15ms), but the instrument that produced the baseline was ad-hoc and never committed, so it evaporated between sessions. Commit it so the gate's own instrument survives. Measures main-thread longtasks during the first (cold) switch into the 600-message deep-history channel under 4x CPU throttle, windowed to the 300-row ceiling. Reports median-of-5 longest-longtask, run-to-run spread, and total-longtask-time-in-window. Instrument, not a gate: the only assertion confirms the switch exercised the mount under throttle. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ized timeline A sticky header inside the scroll container drifts ~49px at scrollTop 0: once older history prepends and the scroll restores, the header pins to its clamped offset, but at the top it had been sitting at its larger natural flow offset. The fix portals the header into a non-scrolling overlay container outside the scroll element (mirroring the unread-pill overlay), so it pins to a fixed offset regardless of scroll position and cannot move as content prepends above the anchor. The per-scroll re-render that resolves the topmost visible day stays localized to VirtualizedList rather than forcing MessageTimeline to re-render on every scroll. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The timeline-virtualization port regressed scroll-anchoring: it dropped
the non-virtualized list's implicit "full content height exists at pin
time" invariant. Rows mount estimate-sized and measure to real height on
scroll, so the raw `scrollTo(scrollHeight)` mount pin landed short and
`scrollHeight` grew as off-screen rows measured.
Drive the mount bottom-pin through `virtualizer.scrollToIndex(lastIndex,
{align:"end"})` so TanStack lands the true last row through its own
measurement. Arm the existing settle-guard on smooth `scrollToBottom`
too — an animated jump is not atomic, so a mid-animation `onScroll`
latched a stale mid-history anchor that the ResizeObserver then restored.
Teach the prepend-restore loop to re-aim at the bottom when the user
abandons to bottom mid-restore, and the all-gone fallback to keep a
windowed-out anchor (vs. only pinning on genuine deletion).
The teleport spec's `scrollHeight <= baseline+100` setup proxy assumed
the non-virtualized contract (scrollHeight changes only on DOM adds); a
virtualizer grows `getTotalSize()` from lazy measurement alone. Replace
it with the direct in-flight signal the suite already keys on. Seed loops
that omitted `createdAt` collided on one whole-second stamp and sorted by
random UUID, so the asserted last/target row landed at a random index
often outside the virtualized window — make them monotonic to match the
file's own channel-intro seed precedent. No product-property assertion
text changed.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…lization The prior helper asserted symmetric balance (gapAbove == gapBelow within 1px), valid when intro, divider, and first row shared one flow layout. The virtualization re-land moved the divider and first row into the translateY track while the intro stays a flex sibling, so the two gaps are no longer comparable quantities. The fix had collapsed the oracle to bare non-overlap (>= 0 on both gaps), which gutted the layout-regression guard the test exists to provide. Source measurement showed the intro -> divider gap is the layout-controlled 8px and rock-stable, while the divider -> message gap is ~0 by construction (virtualizer rows are back-to-back) plus MessageRow avatar/font render jitter, genuinely variable run-to-run. So band the stable gap (8 +/- 2) as the real guard and keep the variable one as a non-overlap reading-order check. Renamed to match what it now verifies. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The keyed scroll container remounts per channel, but the virtualizer and the anchored-scroll ResizeObserver are owned by the parent and persisted across switches, so on switch-back they kept pointing at the previous channel's detached nodes. The mount-pin then fired scrollToIndex against a stale virtualizer (scrollElement on a detached node), landing at the top instead of the bottom, and the late-measurement bottom-chase never ran — so the top-anchored channel intro painted when it should be windowed out. Match the JS objects' lifetimes to the scroll node's: key TimelineMessageList on channelId so the virtualizer remounts fresh, register it in a layout effect so the parent's same-commit mount-pin reads the fresh instance, and add channelId to the observer deps so it re-observes the live content node. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
force-pushed
the
duncan/timeline-virtualization-reland
branch
from
77a5801 to
bb941d4
Compare
wesbillman
reviewed
Jun 25, 2026
wesbillman
reviewed
Jun 25, 2026
wesbillman
approved these changes
Jun 26, 2026
Two bugs surfaced in review:
1. useConvergentScrollToMessage was called without onAbandoned. When
convergence fails (deleted target, bad deep-link, frame cap hit),
convergingTargetIdRef stays non-null indefinitely, permanently
disabling anchor restore (bottom-stick, resize, append) for that
channel view. Wire onAbandoned the same as onConverged so the route
param clears and anchor restore re-enables.
2. TimelineMessageList called onItems in a passive useEffect, which
fires after paint. useAnchoredScroll's mount bottom-pin runs in a
layout effect on the same commit — before the passive effect — so
timelineItems was still null, virtualizerOption was null, and
pinToBottomByIndex fell back to raw scrollHeight instead of
scrollToIndex(lastIndex, { align: 'end' }). Change to
useLayoutEffect so items are available when the layout effect runs.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…out layout regression The previous fix (useLayoutEffect for onItems) caused a synchronous re-render before paint, which broke the narrow-thread-view header geometry test. The root cause: calling setTimelineItems in a layout effect triggered a cascading synchronous flush that mutated the DOM before Playwright measured bounding boxes. The correct fix: compute timelineItems directly in MessageTimeline via useMemo, mirroring the same entries logic TimelineMessageList uses (mainEntries when the deferred snapshot is current, buildMainTimelineEntries otherwise). This makes virtualizerOption non-null from the first deferred commit that carries real messages — no state update, no extra render, no layout side effects. VirtualizedList already publishes the virtualizer instance in a layout effect (child before parent), so getVirtualizer() is live when the mount pin runs. Both conditions for the index-driven pin are now met on the same commit. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… flushSync Two CI failures on the virtualized timeline branch: 1. virtualization.spec.ts:293 — load-older prepend scroll height decreased. virtualizerOption.itemCount is derived from deferredMessages (stale during a fetch). useLoadOlderOnScroll captured previousCount = virt.itemCount (stale) and polled after?.itemCount (also stale) — grew never fired and the anchor restore never ran. Fix: read instance.options.count from the live virtualizer instance, which TanStack updates on every render. 2. messaging.spec.ts:811 — menuGap !== headerPaddingInlineEnd. df8d307 made virtualizerOption non-null on the first deferred commit, so the mount pin layout effect called virtualizer.scrollToIndex. TanStack react-virtual uses useFlushSync=true by default: scrollToIndex triggers a scroll event which calls flushSync(rerender), a synchronous React re-render inside the layout effect. This mutated the DOM before Playwright measured header layout. Fix: defer the scroll call to rAF so the current paint commit completes before the virtualizer drives its settle loop. The anchor and settlingRef are still set synchronously so scroll events during the settle window are suppressed correctly. mountPinRafIdRef cancels the pending rAF on channel switch. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
instance.options.count and virt.itemCount are both derived from deferredMessages (via VirtualizedList's items prop), so they are stale during a load-older fetch. The growth check (grew = after.count > previousCount) never fired, leaving the anchor restore dead and scroll height decreasing after prepend. Fix: thread liveMessageCount: messages.length through virtualizerOption (where messages is the live prop, not the deferred snapshot) and use it for previousCount and grew checks. itemCount (deferred) is still used for the lastIndex in the abandon path where we want the deferred count. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
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
2 participants
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.
Re-lands timeline virtualization on the desktop client, eliminating the macOS scroll beachball caused by rendering the entire message list into the DOM. Only the visible window is now mounted, and scroll anchoring is rebuilt to behave correctly under virtualization.
What changes
TimelineMessageListandVirtualizedListmount only the windowed rows instead of the full history.MessageTimelinelands at the true bottom on mount viascrollToIndex(lastIndex, { align: "end" })so the initial view is pinned correctly on the TanStack measurement path.useAnchoredScroll,useConvergentScrollToMessage,useLoadOlderOnScroll, andscrollConvergencepreserve the user's scroll position across history loads and deep-link/teleport navigation, with a settle guard so smooth-scroll animations can't latch a mid-animation anchor.ActiveDayHeaderprojects the current day divider as a drift-immune floating header over the virtualized list.cold-switch-longtask.perf.ts) measures channel-switch main-thread cost so the beachball budget is guarded going forward.scroll-history.spec.tsthat omittedcreatedAtcollided on a single whole-second stamp and sorted by random UUID — latent on the non-virtualized DOM, but decisive once only the windowed rows are mounted. Those loops now seed monotoniccreatedAt, matching the existing in-file precedent, with assertions unchanged. Thechannels.spec.tsintro/day-divider spacing oracle is re-expressed to band the layout-controlledgap-2(8px) spacing and assert non-overlap on the back-to-back row gap.Known follow-ups (non-blocking)
>=comparator in the anchor restore path is looser than its comment claims.Supersedes #1123 (abandoned hybrid attempt).