Skip to content

Commit

Permalink
fix(useMouseAndTouchTrackers): dependency array (#1612)
Browse files Browse the repository at this point in the history
* fix(useMouseAndTouchTrackers): dependency array

* remove console logs

* some improvements

* pass references
  • Loading branch information
silviuaavram authored Jul 26, 2024
1 parent ee2a828 commit 59246b6
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 54 deletions.
101 changes: 101 additions & 0 deletions src/hooks/__tests__/__snapshots__/utils.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`utils useMouseAndTouchTracker adds and removes listeners to environment: element change rerender adding events 1`] = `
[
[
mousedown,
[Function],
],
[
mouseup,
[Function],
],
[
touchstart,
[Function],
],
[
touchmove,
[Function],
],
[
touchend,
[Function],
],
]
`;

exports[`utils useMouseAndTouchTracker adds and removes listeners to environment: element change rerender remove events 1`] = `
[
[
mousedown,
[Function],
],
[
mouseup,
[Function],
],
[
touchstart,
[Function],
],
[
touchmove,
[Function],
],
[
touchend,
[Function],
],
]
`;

exports[`utils useMouseAndTouchTracker adds and removes listeners to environment: initial adding events 1`] = `
[
[
mousedown,
[Function],
],
[
mouseup,
[Function],
],
[
touchstart,
[Function],
],
[
touchmove,
[Function],
],
[
touchend,
[Function],
],
]
`;

exports[`utils useMouseAndTouchTracker adds and removes listeners to environment: unmount remove events 1`] = `
[
[
mousedown,
[Function],
],
[
mouseup,
[Function],
],
[
touchstart,
[Function],
],
[
touchmove,
[Function],
],
[
touchend,
[Function],
],
]
`;
78 changes: 34 additions & 44 deletions src/hooks/__tests__/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe('utils', () => {
describe('useMouseAndTouchTracker', () => {
test('renders without error', () => {
expect(() => {
renderHook(() => useMouseAndTouchTracker(undefined, [], jest.fn()))
renderHook(() => useMouseAndTouchTracker(undefined, jest.fn(), []))
}).not.toThrowError()
})

Expand All @@ -93,63 +93,53 @@ describe('utils', () => {
addEventListener: jest.fn(),
removeEventListener: jest.fn(),
}
const refs = []
const elements = [{}, {}]
const handleBlur = jest.fn()

const {unmount, rerender, result} = renderHook(() =>
useMouseAndTouchTracker(environment, refs, handleBlur),
const initialProps = {environment, handleBlur, elements}

const {unmount, rerender, result} = renderHook(
props =>
useMouseAndTouchTracker(
props.environment,
props.handleBlur,
props.elements,
),
{initialProps},
)

expect(environment.addEventListener).toHaveBeenCalledTimes(5)
expect(environment.addEventListener).toHaveBeenCalledWith(
'mousedown',
expect.any(Function),
)
expect(environment.addEventListener).toHaveBeenCalledWith(
'mouseup',
expect.any(Function),
)
expect(environment.addEventListener).toHaveBeenCalledWith(
'touchstart',
expect.any(Function),
)
expect(environment.addEventListener).toHaveBeenCalledWith(
'touchmove',
expect.any(Function),
)
expect(environment.addEventListener).toHaveBeenCalledWith(
'touchend',
expect.any(Function),
)
expect(environment.removeEventListener).not.toHaveBeenCalled()
expect(environment.addEventListener.mock.calls).toMatchSnapshot(
'initial adding events',
)

rerender()
environment.addEventListener.mockReset()
environment.removeEventListener.mockReset()
rerender(initialProps)

expect(environment.removeEventListener).not.toHaveBeenCalled()
expect(environment.addEventListener).not.toHaveBeenCalled()

unmount()
rerender({...initialProps, elements: [...elements]})

expect(environment.addEventListener).toHaveBeenCalledTimes(5)
expect(environment.removeEventListener).toHaveBeenCalledTimes(5)
expect(environment.removeEventListener).toHaveBeenCalledWith(
'mousedown',
expect.any(Function),
expect(environment.addEventListener.mock.calls).toMatchSnapshot(
'element change rerender adding events',
)
expect(environment.removeEventListener).toHaveBeenCalledWith(
'mouseup',
expect.any(Function),
expect(environment.removeEventListener.mock.calls).toMatchSnapshot(
'element change rerender remove events',
)
expect(environment.removeEventListener).toHaveBeenCalledWith(
'touchstart',
expect.any(Function),
)
expect(environment.removeEventListener).toHaveBeenCalledWith(
'touchmove',
expect.any(Function),
)
expect(environment.removeEventListener).toHaveBeenCalledWith(
'touchend',
expect.any(Function),

environment.addEventListener.mockReset()
environment.removeEventListener.mockReset()

unmount()

expect(environment.addEventListener).not.toHaveBeenCalled()
expect(environment.removeEventListener).toHaveBeenCalledTimes(5)
expect(environment.removeEventListener.mock.calls).toMatchSnapshot(
'unmount remove events',
)

expect(result.current).toEqual({
Expand Down
5 changes: 4 additions & 1 deletion src/hooks/useCombobox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ function useCombobox(userProps = {}) {
})
const mouseAndTouchTrackers = useMouseAndTouchTracker(
environment,
[toggleButtonRef, menuRef, inputRef],
useCallback(
function handleBlur() {
if (latest.current.state.isOpen) {
Expand All @@ -110,6 +109,10 @@ function useCombobox(userProps = {}) {
},
[dispatch, latest],
),
useMemo(
() => [menuRef, toggleButtonRef, inputRef],
[menuRef.current, toggleButtonRef.current, inputRef.current],
),
)
const setGetterPropCallInfo = useGetterPropsCalledChecker(
'getInputProps',
Expand Down
6 changes: 5 additions & 1 deletion src/hooks/useSelect/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ function useSelect(userProps = {}) {
const toggleButtonRef = useRef(null)
const menuRef = useRef(null)
const itemRefs = useRef({})

// used to keep the inputValue clearTimeout object between renders.
const clearTimeoutRef = useRef(null)
// prevent id re-generation between renders.
Expand Down Expand Up @@ -120,7 +121,6 @@ function useSelect(userProps = {}) {

const mouseAndTouchTrackers = useMouseAndTouchTracker(
environment,
[toggleButtonRef, menuRef],
useCallback(
function handleBlur() {
if (latest.current.state.isOpen) {
Expand All @@ -131,6 +131,10 @@ function useSelect(userProps = {}) {
},
[dispatch, latest],
),
useMemo(
() => [menuRef, toggleButtonRef],
[menuRef.current, toggleButtonRef.current],
),
)
const setGetterPropCallInfo = useGetterPropsCalledChecker(
'getMenuProps',
Expand Down
15 changes: 7 additions & 8 deletions src/hooks/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,15 +359,15 @@ function getHighlightedIndexOnOpen(props, state, offset) {
/**
* Tracks mouse and touch events, such as mouseDown, touchMove and touchEnd.
*
* @param {Object} environment The environment to add the event listeners to, for instance window.
* @param {Array<HTMLElement>} downshiftElementRefs The refs for the element that should not trigger a blur action from mouseDown or touchEnd.
* @param {Function} handleBlur The function that is called if mouseDown or touchEnd occured outside the downshiftElements.
* @returns {Object} The mouse and touch events information, if any of are happening.
* @param {Window} environment The environment to add the event listeners to, for instance window.
* @param {() => void} handleBlur The function that is called if mouseDown or touchEnd occured outside the downshiftElements.
* @param {Array<{current: HTMLElement}>} downshiftElementsRefs The refs for the elements that should not trigger a blur action from mouseDown or touchEnd.
* @returns {{isMouseDown: boolean, isTouchMove: boolean, isTouchEnd: boolean}} The mouse and touch events information, if any of are happening.
*/
function useMouseAndTouchTracker(
environment,
downshiftElementRefs,
handleBlur,
downshiftElementsRefs,
) {
const mouseAndTouchTrackersRef = useRef({
isMouseDown: false,
Expand All @@ -380,7 +380,7 @@ function useMouseAndTouchTracker(
return noop
}

const downshiftElements = downshiftElementRefs.map(ref => ref.current)
const downshiftElements = downshiftElementsRefs.map(ref => ref.current)

function onMouseDown() {
mouseAndTouchTrackersRef.current.isTouchEnd = false // reset this one.
Expand Down Expand Up @@ -431,8 +431,7 @@ function useMouseAndTouchTracker(
environment.removeEventListener('touchmove', onTouchMove)
environment.removeEventListener('touchend', onTouchEnd)
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- refs don't change
}, [environment, handleBlur])
}, [downshiftElementsRefs, environment, handleBlur])

return mouseAndTouchTrackersRef.current
}
Expand Down

0 comments on commit 59246b6

Please sign in to comment.