react-hook icon indicating copy to clipboard operation
react-hook copied to clipboard

Improvement to return type of `useMouse`

Open pc-erin opened this issue 3 years ago • 2 comments

I noticed that the return type of useMouse isn't exactly reflective of the data returned.

If we made it shaped more like this:

export type MousePosition = MousePositionOver | MousePositionOut

type MousePositionOver = {
  x: number
  y: number
  pageX: number
  pageY: number
  clientX: number
  clientY: number
  screenX: number
  screenY: number
  elementWidth: number
  elementHeight: number
  isOver: true
  isDown: boolean
  isTouch: boolean
}

type MousePositionOut = {
  x: undefined
  y: undefined
  pageX: undefined
  pageY: undefined
  clientX: undefined
  clientY: undefined
  screenX: undefined
  screenY: undefined
  elementWidth: undefined
  elementHeight: undefined
  isOver: false
  isDown: false
  isTouch: boolean
}

This preserve additional type information since, if you check that mouseData.isOver is true, the inside of that conditional will see all the values as numbers and not have to check each property individually.

Also, if we switch from null to undefined for the missing values, you can use default values during destructuring:

const { x = 0, y = 0, isOver } = useMouse(ref)

The change from null to undefined would be breaking, so if you prefer we could put off merging that until a planned major version bump, but the rest should be safe.

pc-erin avatar Sep 23 '22 23:09 pc-erin

This sounds good to me!

jaredLunde avatar Sep 24 '22 01:09 jaredLunde

Hey, so while trying to modify the hook, I noticed a few things that might be an issue.

First, on line 32, element.getBoundingClientRect is being called inside a reducer which breaks the semantics and guarantees of reducers.

Information should never be retrieved from non-deterministic sources inside a reducer. Their output should be a pure function of their input. Same for any instances of getting properties of window inside the reducer.

All of that information should be passed into the reducer via the action, not retrieved directly inside it. It will produce very strange results if anyone tries to use react time-travel debugging on that reducer, and memoization/caching will also fail to work correctly.

I can't add those more specific return types because the way the code is currently written, there is no guarantee that the pointer position (or any other information) will be present in the state when the pointer is over the element. I think this hook may need a significant amount of re-writing before we can make statements about it's return types with any certainty.

pc-erin avatar Oct 06 '22 17:10 pc-erin