Repository navigation
test(List): the update proofs read an exit while it runs - #10973
Conversation
Four List update proofs read a row's exit, or counted layouts, over the runner's round trips. On a loaded runner a round trip can outlast the exit, or take in a layout of other page work. - The removal proof arms a hold on the list that takes the exit from its first frame, judging its clip. Its inert, aria-hidden and accessibility reads then find the row, and the release lets it go. - The two swiped-exit proofs press the second change in the first frame that shows the exit, not 30 ms later. They read the rows once React commits it, in the same task. - The swiped-open layout proof counts layouts on the page's clock, from the click through two frames, not LayoutCount over round trips.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| List | 18.6 kB (0) · 790 B CSS (0) | — | ✓ · ✓ · — |
| ListItem | 70.7 kB (0) · 11.4 kB CSS (0) | — | ✓ · ✓ · ✓ |
Package size: measured; main has no measurement of the merge base to compare yet.
API changes: none in this pull request
Committed API report behind this build: cli 29 rows, core 36 rows; run pnpm api:report.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. The pull request is a draft from a fork, so once it is marked ready for review a maintainer needs to add the ci:run label for CI to run.
The four proofs now read what they judge on the page's own timing. The whole spec passes in Chromium and WebKit at this head, and a replay of the removal proof with a late runner fails on main and passes here.
- Optional hardening: the removal proof's hold covers only the list (
#storybook-root ul), so the glides of the folder-count text and the three buttons after it are still read one round trip after the click. Scoping the hold to the animated container (#storybook-root > div) holds them too (see the inline note).
[Automated review]
The removal proof held only the list, so the glides of the text and the buttons after it still ended on their own before a late read. The hold now takes the story's animated container, and checks it took the text's glide as well as the exit.
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. The pull request is a draft from a fork, so a maintainer needs to add the ci:run label, and CI then runs once it is marked ready for review.
The removal proof's hold now takes the text and buttons after the list and checks that it took the text's glide, which settles the earlier note; the four proofs read what they judge on the page's own timing.
- Optional: the swiped-open layout proof stamps its end in the second animation-frame callback, before that frame lays out, so it counts the click and one frame rather than "the next two frames" (see the inline note).
[Automated review]
The swiped-open layout proof stamped its end in the second frame's callback, before that frame laid out. A layout a swiped row adds in the second frame fell outside the count. It now ends on a third frame, once the second has laid out.
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. The pull request is a draft from a fork, so once it is marked ready for review a maintainer needs to add the ci:run label for CI to run.
The swiped-open layout proof now ends its window on a third animation frame, so a layout in the second frame after the click is counted; that settles the earlier note. The four changed proofs read what they judge on the page's own timing, and the whole spec passes in Chromium on this head.
[Automated review]
cixzhang
added
the
ci:run
The hold on the whole animated container also took the pressed button's own press animation. In WebKit that can start on the pointerdown, before the click removes the row, and it met the hold's count first. The hold takes the list only again. The glides after the list wait for a hold that waits for the judged motion.
|
/approve-spec c5abb41 |
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. The pull request is still a draft, so its CI run starts once it is marked ready for review; it already carries ci:run.
The removal, swiped-exit and swiped-open layout proofs now read what they judge on the page's own timing, and the whole spec passes in Chromium on this head. One optional point from the earlier thread still applies: with the hold back on the list, the glides of the text and buttons after the list are read one round trip after the click, so a read delayed past their 300 ms can still fail the removal proof.
[Automated review]
Maintainer impact
Four List update proofs fail intermittently in Chromium under load, with unnamed assertion errors. Each read a row's exit, or counted layouts, across the runner's round trips. On a loaded runner a round trip can outlast the exit, or take in a layout of other page work.
Intended invariant
A proof reads a transient state while it runs (README: curve sampling). It counts only the work of the change it judges (
traceRendering).Change
inert,aria-hiddenand accessibility reads then find the row, and the release lets it go. The text's glide after the list is not held, so a read delayed past it can miss it. Holding it waits for a hold that starts on the judged motion.traceRendering, from the click until the second frame has laid out. It no longer readsLayoutCountacross round trips.Evidence
main. Its row was gone before the reads.mainwith each failure's exact message, 3 of 3. The fix passes them 3 of 3.mainfailed 2 of 12 ("with a row swiped open: 1; with none: 2"). The fix passed 12 of 12.Scope
Testing
On
maina1fda63, in CI's container profile (4 CPUs), one worker:mainand with the change. WebKit, 2 runs: 18 of 18 (the Chromium-only proofs skip).main4398069: the removal proof 10 of 10 in each engine, and 12 of 12 under load. The whole spec: Chromium 19 of 19, WebKit 9 of 9.