From 59246b68167acdf4284572511fde61f5a160d0fb Mon Sep 17 00:00:00 2001 From: Silviu Alexandru Avram Date: Fri, 26 Jul 2024 10:08:00 +0300 Subject: [PATCH] fix(useMouseAndTouchTrackers): dependency array (#1612) * fix(useMouseAndTouchTrackers): dependency array * remove console logs * some improvements * pass references --- .../__snapshots__/utils.test.js.snap | 101 ++++++++++++++++++ src/hooks/__tests__/utils.test.js | 78 ++++++-------- src/hooks/useCombobox/index.js | 5 +- src/hooks/useSelect/index.js | 6 +- src/hooks/utils.js | 15 ++- 5 files changed, 151 insertions(+), 54 deletions(-) create mode 100644 src/hooks/__tests__/__snapshots__/utils.test.js.snap diff --git a/src/hooks/__tests__/__snapshots__/utils.test.js.snap b/src/hooks/__tests__/__snapshots__/utils.test.js.snap new file mode 100644 index 00000000..1f1fbc1e --- /dev/null +++ b/src/hooks/__tests__/__snapshots__/utils.test.js.snap @@ -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], + ], +] +`; diff --git a/src/hooks/__tests__/utils.test.js b/src/hooks/__tests__/utils.test.js index 1cd40c40..1f49224f 100644 --- a/src/hooks/__tests__/utils.test.js +++ b/src/hooks/__tests__/utils.test.js @@ -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() }) @@ -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({ diff --git a/src/hooks/useCombobox/index.js b/src/hooks/useCombobox/index.js index 4e8f0ee7..d9e357c8 100644 --- a/src/hooks/useCombobox/index.js +++ b/src/hooks/useCombobox/index.js @@ -98,7 +98,6 @@ function useCombobox(userProps = {}) { }) const mouseAndTouchTrackers = useMouseAndTouchTracker( environment, - [toggleButtonRef, menuRef, inputRef], useCallback( function handleBlur() { if (latest.current.state.isOpen) { @@ -110,6 +109,10 @@ function useCombobox(userProps = {}) { }, [dispatch, latest], ), + useMemo( + () => [menuRef, toggleButtonRef, inputRef], + [menuRef.current, toggleButtonRef.current, inputRef.current], + ), ) const setGetterPropCallInfo = useGetterPropsCalledChecker( 'getInputProps', diff --git a/src/hooks/useSelect/index.js b/src/hooks/useSelect/index.js index f36f840b..5f22eeb6 100644 --- a/src/hooks/useSelect/index.js +++ b/src/hooks/useSelect/index.js @@ -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. @@ -120,7 +121,6 @@ function useSelect(userProps = {}) { const mouseAndTouchTrackers = useMouseAndTouchTracker( environment, - [toggleButtonRef, menuRef], useCallback( function handleBlur() { if (latest.current.state.isOpen) { @@ -131,6 +131,10 @@ function useSelect(userProps = {}) { }, [dispatch, latest], ), + useMemo( + () => [menuRef, toggleButtonRef], + [menuRef.current, toggleButtonRef.current], + ), ) const setGetterPropCallInfo = useGetterPropsCalledChecker( 'getMenuProps', diff --git a/src/hooks/utils.js b/src/hooks/utils.js index af6ba5b0..defceeb4 100644 --- a/src/hooks/utils.js +++ b/src/hooks/utils.js @@ -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} 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, @@ -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. @@ -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 }