matrix-org / matrix-org/matrix-js-sdk
Relation event insertion breaks during backwards pagination if parent is on another page (if `supportsThreads: true`)
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 2.2k
- Forks
- 704
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 40
Description
Assume that we have a `PARENT` event a `CHILD` event which is an `m.replace` of `PARENT`, sent a long time after `PARENT` was sent (so there are some pages between the two events).
If I have `supportsThreads: true`, and I'm backfilling the room via `paginateEventTimeline` (going in backwards direction), I hit `CHILD` first, going through `addEventsToTimeline` -> `eventShouldLiveIn`, leading me to...
https://github.com/matrix-org/matrix-js-sdk/blob/b515cdbdbbe21d481409c677af365a5315fb6390/src/models/room.ts#L2171
Since I didn't hit `PARENT` yet (and it is not in the page that is currently being processed, i.e. `events`), `parentEvent` is not found, causing the following case to fail (I think this case is the one that *should* be hit, although we don't have parent so we can't recur):
https://github.com/matrix-org/matrix-js-sdk/blob/b515cdbdbbe21d481409c677af365a5315fb6390/src/models/room.ts#L2174-L2177
All other branches fail, causing the function to return:
https://github.com/matrix-org/matrix-js-sdk/blob/b515cdbdbbe21d481409c677af365a5315fb6390/src/models/room.ts#L2198-L2204
This omits `CHILD` from the timeline (and I think shows `PARENT` instead?).
(I'm probably going to just disable `supportsThreads` to workaround this for myself.)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/models/room.ts at paginateEventTimeline, addEventsToTimeline, and eventShouldLiveIn, especially the relation handling around the linked lines. Reproduce backwards pagination with supportsThreads: true when a child m.replace event appears before its parent on a later page. Done means the child is retained in the timeline instead of being omitted when its parent is not in the current page.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100