test(react): drain stubbed animation frames in the remaining UI harnesses - #3890
Conversation
…sses
Eleven UI test files stub requestAnimationFrame with a real timer:
g.requestAnimationFrame = (cb) => setTimeout(() => cb(0), 0);
and nothing drains it. cancelAnimationFrame only helps a caller holding the id;
a frame still queued at teardown is a pending timer, which the Deno op sanitizer
reports as "a timer was started in this test, but never completed". React can
schedule a frame right up to unmount, so whether it has run is a matter of load.
Because the sanitizer reports at suite level, the resulting failure names a
suite rather than a step and lands on whatever branch happens to be pushing.
Track outstanding frames and clear them in the teardown that already restores
the globals and closes the DOM. The stub and its teardown were byte-identical
across every file, so the change is the same in each.
None of these was observed failing -- this is closing a latent hole, not a live
defect. navigation-menu is excluded because it is fixed on the pre-push-gate
branch; patching it here would only create a conflict.
Left as a follow-up rather than bundled here: each of these files hand-rolls the
same JSDOM globals harness, and extracting one shared helper would remove the
next copy of this bug before it is written. That is a refactor across twelve
files and deserves its own review rather than riding along with a mechanical fix.
|
Warning Review limit reached
Next review available in: 3 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
1 participant
Description
Eleven UI test files stub
requestAnimationFramewith a real timer and never drain it:cancelAnimationFrameonly helps a caller holding the id. A frame still queued at teardown is a pending timer, which the Deno op sanitizer reports as "a timer was started in this test, but never completed". React can schedule a frame right up to unmount, so whether it has run by teardown is a matter of load.Because the sanitizer reports at suite level, the resulting failure names a suite rather than a step and lands on whatever branch happens to be pushing — the misattribution pattern already recorded in #594 and #612.
Fix
Track outstanding frames and clear them in the teardown that already restores globals and closes the DOM. The stub and its teardown were byte-identical across every file, so the change is identical in each — a mechanical, exact-match patch rather than eleven judgement calls.
Scope and honesty
None of these was observed failing. This closes a latent hole; it is not a live defect. I am not claiming it fixes a specific flake.
navigation-menu.test.tsxis excluded — it is fixed on the pre-push-gate branch, and patching it here would only create a conflict.Deliberately not bundled: each of these files hand-rolls the same JSDOM globals harness. Extracting one shared helper would remove the next copy of this bug before it is written, and is the better long-term answer. But that is a refactor across twelve files with subtly different surroundings, and it deserves its own review rather than riding along with a mechanical fix. Worth doing next.
Evidence
src/react/under--parallel --trace-leaks, three consecutive runs:No production code changed.
Related Issue(s)
Tracked internally.
Type of Change
Checklist