bug: jumping to an old thread reply or the thread parent silently fails in threads with 300+ replies

オープン
  • 何か問題がありますか？
#7,792 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
1〜2日
初心者へのやさしさ
30/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
react-native, typescript
領域
frontend, mobile

調査の方向性

Start in app/views/RoomView/List/hooks/useScroll.ts, where onReObserve polls fetchMessages() up to MAX_JUMP_GROWTH_RETRIES and growsWindowTowardTarget decides when it runs, then read useMessages for the window count and app/lib/methods/loadThreadMessages.ts for loadThreadMessagesUntil. Done means a jump to the earliest reply or the thread parent in a 500+ reply thread scrolls to and highlights the target, while existing jumps keep working. The work depends on the unmerged #7773, so check its state before starting.

索引モデルが issue の本文から書いたものです。

説明

🐛 bug type: bug
Describe the Bug

In a thread with roughly 300 or more replies, jumping to an old reply (quote link, search result, deep link) or to the thread parent does nothing. The loading overlay closes after about 5 seconds, the list does not move, and no error is shown.

This was found while reviewing #7773, which loads threads 50 replies at a time on servers >= 8.8.0. It is listed under "Known limits" in that PR and was raised in this review comment, where the reviewer suggested handling it as a follow-up.

Why it happens

A jump inside a thread has two stages:

  1. Storing the target. loadThreadMessagesUntil (app/lib/methods/loadThreadMessages.ts) pages the thread until the target is stored in thread_messages. This works at any depth.
  2. Showing it. The list only renders what its Message Window holds, so after the target is stored the window still has to grow until it contains the target. useScroll (app/views/RoomView/List/hooks/useScroll.ts) does this by polling in onReObserve: each time the target is not in the list it calls fetchMessages(), which grows the window by QUERY_SIZE (50), and waits for the next emit.

That polling is capped by MAX_JUMP_GROWTH_RETRIES = 5, and each growth step re-arms JUMP_SAFETY_TIMEOUT (5 s). The window therefore reaches about 300 rows (50 initial + 5 x 50). A target deeper than that never appears, the safety timeout fires, and abortJump resolves the jump without a scroll or an error.

The same distance problem exists one layer down: MAX_SCROLL_TO_INDEX_RETRIES was raised from 20 to 60 in #7773 so deep targets can be reached by scrollToIndex.

On develop this growth only applied to the main room's Anchored Window (jump.anchored). #7773 extended it to threads (|| !!tmid) so jumps inside a thread can reach replies outside the first window, and the same cap now limits them. #7773 also stops appending the thread parent below the newest 50 replies, so a jump to the parent now needs the window to reach the oldest reply. Per #7773's notes the limit applies on any server version.

Steps to Reproduce
  1. Use a server >= 8.8.0 and create a thread with about 500 replies.
  2. Open the thread in the app.
  3. Jump to one of the earliest replies (link to the reply from inside the thread, a search result, or a deep link). A link to the thread parent behaves the same way.
  4. All pages are requested (offset 0, 50, ... up to the page holding the target) and the window grows to about 300 rows.
Expected Behavior

The list scrolls to the target and highlights it.

Actual Behavior

After about 5 seconds the overlay closes with no scroll and no error.

Proposed approach

The target's ts is known once loadThreadMessagesUntil has stored it, so the window can be sized in one step instead of polled:

  • Add a growWindowTo(ts) to useMessages. It sets count.current to the number of rows with ts >= targetTs plus QUERY_SIZE, then re-subscribes once. raiseOrReleaseAnchor already does the same kind of sizing with its aboveCount query (fetchCount()).
  • That would remove growsWindowTowardTarget and the tmid parameter of useScroll, and may let the scroll-to-index retry cap go back to 20.

Things to check when picking this up:

  • The count must use the same filters as the window query (rid = tmid, buildVisibleSystemTypesClause(hideSystemMessages)), otherwise the window is sized too small or too large. Over-counting only adds harmless margin; under-counting reproduces the bug.
  • growsWindowTowardTarget is jump.anchored || !!tmid, so the main room's Anchored Window jumps use the same polling. Decide whether that path keeps it or moves to the same approach.
  • A window of several hundred rows has a render and re-emit cost. Worth measuring on a low-end Android device with a 500+ reply thread.
  • Related and separate: on a slow connection the 5 s highlight (HIGHLIGHT_TIMEOUT) can expire before a deep jump arrives. Also listed in #7773's known limits.
Acceptance
  • In a thread of 500+ replies, jumping to the earliest reply and to the thread parent scrolls to and highlights the target on servers >= 8.8.0 and on servers below it.
  • Jumps within the first 300 rows and Anchored Window jumps in the main room behave as before.
  • Unit tests cover the window sizing, and the termination test for the scroll-to-index retries still passes.
Rocket.Chat Server Version

= 8.8.0 (paged thread loading). The window-growth limit itself is server-version independent.

Rocket.Chat App Version

Needs #7773 (not yet merged at the time of writing).

主要言語
TypeScript
スター
2.4k
フォーク
1.5k
平均マージ
1日 15時間
マージ済み PR（30日）
29

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

RocketChat/Rocket.Chat.ReactNative のほかの issue

RocketChat/Rocket.Chat.ReactNative の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。