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.
This commit is contained in:
@@ -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<string>();
|
||||
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 {
|
||||
|
||||
Reference in New Issue
Block a user