fix(useInterval): clear a manually resumed interval on unmount - #212
Merged
childrentime merged 1 commit intoJul 30, 2026
Merged
Conversation
With `controls: true` the effect returns before it registers a cleanup, so an interval started through resume() keeps firing after the component unmounts - the hook docs already state the interval is cleared on unmount. Register the cleanup in its own mount-scoped effect so it runs in both modes without changing what happens when `delay` updates. resume() also overwrote timer.current without clearing the previous timer, so calling it twice left an interval that neither pause() nor the unmount cleanup could reach. Clear before starting a new one.
Owner
|
Thanks @ostapondo! Solid fix — the docs promised cleanup on unmount and the |
10 tasks
childrentime
added a commit
that referenced
this pull request
Jul 31, 2026
Fixes - useOrientation: lockOrientation/unlockOrientation had inverted isBrowser guards — they early-returned in the browser and only ran during SSR, making both no-ops where they mattered. Closes #215. - useInterval: with controls: true, an interval started through resume() kept firing after unmount, and calling resume() twice leaked a timer neither pause() nor unmount could reach. Closes #212. - useMicrophone: level stayed frozen at the last reading after stop(); it now resets to 0. Closes #213. - useElementByPoint: multiple mode re-rendered on every rAF frame because elementsFromPoint allocates a fresh array; the hit list is now compared element-by-element and kept stable when unchanged. Closes #214. Co-Authored-By: Claude Fable 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
2 participants
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.
Description
useIntervalnever clears its timer on unmount whencontrols: trueis used.The effect that owns the cleanup bails out before it can register one:
So an interval started through
resume()keeps firing after the component isgone. The hook's docs promise the opposite:
To reproduce: mount with
{ controls: true }, callresume(), unmount, thenadvance the timers. The callback keeps running — 10 more calls over 500ms at a
50ms delay in the test I added.
I put the unmount cleanup in its own mount-scoped effect rather than returning
cleanfrom the existing one. That keeps the currentdelay-change behaviouruntouched: in manual mode, updating
delaystill doesn't stop a running timer,which felt like a separate decision from fixing the leak.
While I was in there —
resume()overwrotetimer.currentwithout clearing theprevious timer, so calling it twice orphaned the first interval and neither
pause()nor the unmount cleanup could reach it afterwards. It now clears first.Type of Change
Checklist
No doc change: the existing page already describes the fixed behaviour.
Two tests added to
useInterval/index.spec.ts— one for the unmount leak, onefor the double-
resume()orphan. Both fail onmainand pass here; the other 9in that file are untouched.