mirror of
https://github.com/block/buzz.git
synced 2026-11-06 17:31:19 +00:00
bdd76baea5
## Summary
Re-entering a channel trims older pages from the head of the cached
timeline, and our Virtua patch compensates for each trim from the live
DOM `scrollTop`. When the reader was near the floor, the browser had
already clamped `scrollTop` to the shorter list, so the removal was
counted twice and the reader could be thrown to the top of history. That
is the `message-target-highlight.spec.ts` flake in Desktop Smoke E2E.
The patched scroller now remembers the last offset it observed or wrote,
and compensates from that offset when the live one was clamped to the
floor.
### fix(desktop): compensate head trims from the unclamped scroll offset
#### Problem
`message-target-highlight.spec.ts:79` ("message target flashes: ...")
flaked in Desktop Smoke E2E, always at :179: the linked row had left the
viewport 2.2 s after it flashed.
- main:
-
https://github.com/block/buzz/actions/runs/41568206043/job/125695139845
-
https://github.com/block/buzz/actions/runs/41669669478/job/126035103096
-
https://github.com/block/buzz/actions/runs/41669740241/job/126035480466
-
https://github.com/block/buzz/actions/runs/41694874145/job/126123865714
-
https://github.com/block/buzz/actions/runs/41728489172/job/126240193479
-
https://github.com/block/buzz/actions/runs/41737019256/job/126269365379
- branches:
-
https://github.com/block/buzz/actions/runs/41575774131/job/125719148353
-
https://github.com/block/buzz/actions/runs/41636043464/job/125918005733
-
https://github.com/block/buzz/actions/runs/41751914119/job/126319694104
-
https://github.com/block/buzz/actions/runs/41544910958/job/125618626645
-
https://github.com/block/buzz/actions/runs/41714110046/job/126190515793
All eleven runs predate #8742, and each failed in a case that picks
`mounted[2]`: the bottom-settle mode #8742 fixed by holding the target.
Run 41694874145 also failed once in "other channel, row near the
bottom", and that mode survives #8742: on current main it fails 40 of 60
local runs with tracing on, as CI's retries run.
#### Cause
Re-entering a channel refreshes its newest window, which drops the older
pages from the head of the cached timeline. #8660 passes that head trim
to Virtua as a `shift`, and Virtua compensates by the removed height in
a layout effect. Our Virtua patch (#2855) applies shift compensation to
the live DOM `scrollTop` so that corrections landing before the next
scroll event accumulate. But by then the browser has laid out the
shorter list and clamped `scrollTop` to the new floor whenever the
reader was within the removed height of it, so the removal is counted
twice. In a failing run the view sat 6px above the floor: trimming the
412px channel intro moved it 406px too far, and trimming 3054px of older
rows then wrote `scrollTop` -907, pinning it to the top.
`onStartReached` loaded older history and the target never came back. A
reader scrolled slightly above the bottom loses their place the same
way.
#### Fix
The patched Virtua scroller remembers the last offset it observed in a
scroll event or wrote itself. When a shift compensation finds the live
offset below that and pinned to the floor, the browser clamped it, so
the compensation starts from the remembered offset. Otherwise it keeps
#2855's live-offset base. The ESM and CJS builds change identically, and
the lockfile carries the new patch hash.
#### Validation
- New `virtuaShiftClampPatch.jsdom-test.mjs` drives the real patched
`VList` over a scroller that clamps like a browser. Head trims far from,
near and at the floor keep the reader's rows in place, a trim keeps a
scroll whose event has not fired yet, and a prepend then a trim before
any scroll event accumulate. Each part of the new condition fails a case
when removed: the previous patch line fails the near-floor, at-floor and
accumulation cases (3500 for 3900 and 4000, 3000 for 4000); upstream's
store-offset base fails the unreported-scroll and accumulation cases;
always using the remembered offset, or dropping the floor check, fails
the unreported-scroll case (2500 for 1900).
- "other channel, row near the bottom" with `--repeat-each=30
--workers=4 --trace=retain-on-failure`: 40/60 failed at :179 on current
main, 0/60 failed (60/60 passed) with the fix, and 25/30 failed again
with the previous patch reinstalled.
- All five cases, 20 traced runs each: 100/100 passed.
- scroll-history, virtualization, timeline-no-shift,
overscroll-boundary, dm-history-live-regression and
live-broadcast-reply-timeline, 3 runs each: 117/117 passed.
- Full smoke project: 1485 passed, 3 skipped, 2 failed.
`mentions.spec.ts:807` passed 5/5 on rerun. `messaging.spec.ts:2785`
(own-message avatar) fails the same way on a current-main build here
(3/3).
- `pnpm test`: 6933 unit and 176 jsdom tests passed.
**Rebased onto origin/main `64d12e297` (this branch):** Linux
workstation.
- `pnpm install --frozen-lockfile` installs virtua with the new patch
hash (`f4df71c39f6c...`).
- `pnpm typecheck` and `biome check --error-on-warnings` on the new
jsdom test are clean.
- Unit tests under `src/features/messages` and `src/features/channels`:
1990/1990. jsdom tests there: 12/12. Full desktop `pnpm test`: 6933 unit
and 176 jsdom tests passed.
- Playwright smoke after `pnpm build:e2e`, `--workers=4 --retries=0`:
messaging, send-channel-binding, scroll-history, virtualization,
timeline-no-shift, overscroll-boundary, live-broadcast-reply-timeline
and message-target-highlight, 137 passed and 1 failed. The failure is
`messaging.spec.ts:2785` (own-message avatar), which fails the same way
on main here, as noted above. `dm-history-live-regression.spec.ts` isn't
in the smoke project, so this run left it out.
- `message-target-highlight.spec.ts`, `--repeat-each=10`: 50/50 passed.
"other channel, row near the bottom", `--repeat-each=30`: 30/30 passed.
## Coordination
No open PR edits `patches/virtua@0.49.3.patch`. The overlaps are all in
`pnpm-lock.yaml`:
- #5547 ("fix(desktop): ship a bundle that parses on WebKit without
lookbehind (Intel/macOS 12)") and #7074 ("chore(deps): update react
monorepo (patch)") merge cleanly with main but conflict with this branch
on the `virtua@0.49.3` patch-hash lines. #5547 adds a patched dependency
next to the virtua entry, and #7074 changes the React peer versions in
the same lines. Whichever lands second resolves it by taking both sides'
package changes and re-running `pnpm install` so the lockfile records
`patch_hash=f4df71c39f6c...` for virtua.
- #7073, #7080, #7633, #8218 and #8643 also touch `pnpm-lock.yaml` and
merge cleanly with this branch. Older open PRs that touch it already
conflict with main, and none gains a new conflict from this branch.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: mtoohey <mtoohey@squareup.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>