fix(useMousePressed): attach the touch and drag listeners to a ref target - #225
Merged
childrentime merged 1 commit intoSep 10, 2026
Merged
Conversation
…rget
The target was resolved during render, where a ref handed to a child is
still null, and the effect depended on that resolved element so it never
ran again to pick it up. With the usual `useRef(null)` pattern the
documented `touch` and `drag` listeners were never attached at all, while
`mousedown` worked because it goes through `useEventListener`, which
resolves inside its own effect.
Resolved in the effect now, keyed on `useStableTarget`, as
`useEventListener` does.
`onPressed` was also curried, so `onPressed('mouse')` built one function
for the add and another for the remove, and `dragstart` and `touchstart`
were never detached. One stable handler each.
Adds the spec this hook did not have. Two of its six cases fail on main.
Co-authored-by: Roshan Ramani <rawsun007@users.noreply.github.com>
childrentime
approved these changes
Sep 10, 2026
childrentime
left a comment
Owner
There was a problem hiding this comment.
Verified locally: the spec alone against main fails 2/6 (attach to ref target, detach on unmount); with the fix 6/6 pass, eslint and tsc clean. The useStableTarget + resolve-in-effect shape matches useEventListener, and capturing element in the cleanup closure is the right call since React nulls a callback ref before passive cleanup runs.
Two non-blocking notes for a follow-up, not this PR:
- With no target (
useMousePressed(), which is what the docs demo uses),mousedownfalls back towindowbut touch/drag still attach nothing. PassingdefaultWindowthroughuseStableTargetwould align them, but that is a behaviour change worth its own PR. - The docs demo does not actually use a ref, so it doesn't exercise the path this fixes.
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.
Description
follow up to #224, which mentioned this as a separate change.
useMousePressedresolves its target during render (const element = getTargetElement(target)), and a ref handed to a child is still null then. the effect depends on that resolved element, so it never runs again to pick it up. with the usualuseRef(null)pattern the documentedtouchanddraglisteners are never attached at all.mousedownworks, because it goes throughuseEventListener, which resolves inside its own effect and says so in a comment there.resolved inside the effect now and keyed on
useStableTarget, the same shapeuseEventListeneruses.second defect in the same block:
onPressedwas curried, soonPressed('mouse')built one function for the add and a different one for the remove.removeEventListenermatches on the callback reference, sodragstartandtouchstartwere never detached. one stable handler each fixes it, andonReleasedwas already stable so the other four came off fine.the hook had no spec, so this adds one. two of the six cases fail on main.
worth flagging how i measured it, because my first attempt was wrong: spying on
HTMLElement.prototype.addEventListeneralso catches react 19's delegated listeners on the container, about 140 of them, which made the broken case look like it passed. the spec spies on the one element instead, via a callback ref so the spy is in place before the effects run.verification:
pnpm test415 pass across 70 suites, typecheck and lint clean. reverting the whole file fails 2 of 6; restoring only the render-time resolution fails 3; restoring only the curried removal fails 1.Type of Change
Checklist
the docs already describe the behaviour this restores, so there was nothing to change.
disclosure: written by Claude Opus 5 running in Claude Code, on my machine and under my direction. i have not read the diff line by line myself yet.