Skip to content

fix(useMousePressed): attach the touch and drag listeners to a ref target - #225

Merged
childrentime merged 1 commit into
childrentime:mainfrom
rawsun007:fix-mouse-pressed-ref-target
Sep 10, 2026
Merged

childrentime merged 1 commit into
childrentime:mainfrom
rawsun007:fix-mouse-pressed-ref-target

Conversation

@rawsun007

Copy link
Copy Markdown
Contributor

Description

follow up to #224, which mentioned this as a separate change.

useMousePressed resolves 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 usual useRef(null) pattern the documented touch and drag listeners are never attached at all. mousedown works, because it goes through useEventListener, which resolves inside its own effect and says so in a comment there.

resolved inside the effect now and keyed on useStableTarget, the same shape useEventListener uses.

second defect in the same block: onPressed was curried, so onPressed('mouse') built one function for the add and a different one for the remove. removeEventListener matches on the callback reference, so dragstart and touchstart were never detached. one stable handler each fixes it, and onReleased was 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.addEventListener also 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 test 415 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

  • Bug fix
  • New hook
  • Enhancement to existing hook
  • Documentation update
  • Other (please describe)

Checklist

  • I have read the Contributing Guide
  • My code follows the project's coding style
  • I have added tests for my changes
  • All existing tests pass
  • I have updated the documentation

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.

…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 childrentime left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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), mousedown falls back to window but touch/drag still attach nothing. Passing defaultWindow through useStableTarget would 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.

@childrentime
childrentime merged commit 7a92b92 into childrentime:main Sep 10, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants