fix: cancel the pending animation frame on teardown - #47
Merged
Conversation
A leak audit across the whole library found one loose end, and it is not a leak. useRejectionFlash schedules two callbacks when a keystroke is refused: a requestAnimationFrame to turn the flag on, and a setTimeout to turn it off again. The timer was cleared on unmount and before rescheduling. The frame was neither. Verified rather than assumed: stubbing rAF and firing the pending callback after unmount shows it running and setting state on a component that is gone. React 18 makes that a silent no-op, which is why nothing had ever complained. The retention is a single frame, so it never accumulates — but leaving one of two scheduled callbacks uncancelled is the asymmetry that becomes a real leak as soon as somebody moves it to a longer queue. What the rest of the audit found: nothing. The only listener is the combobox's pointerdown, added and removed in the same effect and measured at 500 added and 500 removed over 500 cycles. The undo history is capped at HISTORY_LIMIT on all four of its mutation sites, and after 22,000 edits holds one snapshot. There is one piece of module-level state, a boolean. No observers, no intervals, and no promises anywhere in the shipped code. Under sustained editing the component is flat: 20,000 edits move the heap by 0.01 MB, where a plain controlled React input under the same load moves it by 4.53 MB. An earlier measurement suggesting otherwise was mine, not the library's — it compared against an uncontrolled input that never re-rendered, so it charged React's re-render cost to this component. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Full leak audit of the shipped code. No leaks found. One loose end, fixed
here, which is not a leak.
The loose end
useRejectionFlashschedules two callbacks on a refused keystroke — arequestAnimationFrameto turn the flag on, asetTimeoutto turn it off. Thetimer was cleared on unmount and before rescheduling. The frame was neither.
Verified rather than assumed — stubbing rAF and firing the pending callback
after unmount shows it running and setting state on a component that is gone.
React 18 makes that a silent no-op, which is why nothing ever complained.
Retention is one frame, so it never accumulates. But leaving one of two
scheduled callbacks uncancelled is the asymmetry that becomes a real leak the
moment someone moves it to a longer queue.
What the audit checked, and found clean
pointerdown, added and removed in the same effect. 500 added / 500 removed over 500 cyclesflagSupport)HISTORY_LIMITon all four mutation sites; after 22,000 edits holds one snapshotA correction to my own measurement
An earlier soak suggested +17.7 MB and looked like a real leak. It was my test,
not the library: I compared against an uncontrolled input that never
re-rendered, so React's re-render cost was being charged to this component.
With a fair control — both controlled, same re-render count:
The component is flat, and lighter under load than a plain controlled input.
Verification
1301 unit tests, coverage 100% × 4, bundle 3253 B / 3600 B.
🤖 Generated with Claude Code