From 0de8c0b5ac8249490b455cba91b01620b0284f08 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 18 Sep 2026 17:11:40 +0800 Subject: [PATCH] fix: message list shaking/flickering when scrolling to top or bottom topic.onData = notify (wireTopicCallbacks) fires once per individual message, not once per batch. Live-instrumented a real loadOlder() page fetch (24 older messages): scrollHeight/scrollTop mutated in ~14 separate small steps over ~400ms, each one racing loadOlder()'s own scroll-compensation and ConversationView's separate autoscroll-to- bottom effect before the page had actually finished arriving -- that rapid multi-step crawl is what reads as "shaking" rather than one clean jump. Coalesce notifications landing within the same animation frame into a single flush, so a burst of onData calls (initial load, loadOlder(), a fast run of incoming messages) triggers at most one re-render per frame instead of N. messageSnapshots invalidation stays synchronous (pure cache bookkeeping); only the listener callbacks that schedule React's re-render are batched. Live-verified with the same instrumentation: the same 24-message loadOlder() fetch now produces 2 dom-mutation events instead of 14, with scrollTop only moving once the batch has actually settled instead of crawling in lockstep with a growing scrollHeight. --- web/src/lib/tinode/topics.ts | 44 ++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/web/src/lib/tinode/topics.ts b/web/src/lib/tinode/topics.ts index 6a8895a7..f008d6ad 100644 --- a/web/src/lib/tinode/topics.ts +++ b/web/src/lib/tinode/topics.ts @@ -86,10 +86,50 @@ export function markConfirmedRead(topicName: string, seq: number) { if (seq > prev) confirmedReadSeq.set(topicName, seq); } +// Live-confirmed root cause of a real reported bug ("聊天消息框滚动到顶部或底部时UI会抖动闪烁"): +// `topic.onData = notify` (wireTopicCallbacks below) fires once per INDIVIDUAL message, not once per +// batch -- confirmed by instrumenting a real `loadOlder()` page fetch (24 older messages): the DOM +// mutated in ~14 separate small steps over ~400ms, each one growing `scrollHeight` a little and each +// one racing the OTHER open effect that reads it (loadOlder's own scroll-compensation, and +// ConversationView's separate "autoscroll to bottom" effect) -- neither gets a stable, final +// `scrollHeight` to work with until the whole page has actually finished arriving, so each of the ~14 +// intermediate re-renders nudges `scrollTop` again, which is what actually reads as "shaking" rather +// than one clean jump. This wasn't reproducible by reasoning about a single `getMessagesPage()` call +// in isolation -- it only showed up instrumenting `scrollTop` across a real fetch. +// Fix: coalesce any notifications that land within the same animation frame into one flush, so a +// burst of N `onData` calls (initial page load, `loadOlder()`, or a fast run of incoming messages) +// triggers at most one re-render per frame instead of N -- imperceptible as a delay, but it means +// `scrollHeight` is only ever read by a re-render once it reflects everything that arrived so far in +// that frame, not some in-between partial state. `messageSnapshots.delete` stays synchronous (it's +// pure cache invalidation, not a re-render) so a snapshot read between two notifications in the same +// frame still sees the latest data, only the LISTENER callbacks (which is what schedules React's own +// re-render) are what get batched. +const pendingTopicNotifications = new Set(); +let pendingAnyNotification = false; +let notifyFrameScheduled = false; + +function flushTopicNotifications() { + notifyFrameScheduled = false; + const topics = [...pendingTopicNotifications]; + pendingTopicNotifications.clear(); + const flushAny = pendingAnyNotification; + pendingAnyNotification = false; + for (const topicName of topics) { + topicListeners.get(topicName)?.forEach((cb) => cb()); + } + if (flushAny) { + anyTopicListeners.forEach((cb) => cb()); + } +} + function notifyTopic(topicName: string) { messageSnapshots.delete(topicName); - topicListeners.get(topicName)?.forEach((cb) => cb()); - anyTopicListeners.forEach((cb) => cb()); + pendingTopicNotifications.add(topicName); + pendingAnyNotification = true; + if (!notifyFrameScheduled) { + notifyFrameScheduled = true; + requestAnimationFrame(flushTopicNotifications); + } } export function subscribeTopicChanges(topicName: string, callback: () => void): () => void {