Memoize hook output (#149)

* chore(lint): add eslint-plugin-react-hooks

* refactor(performance): memoize useSwipeable output

Author: FaberVitale

.eslintrc

@@ -33,10 +33,9 @@
     "react/jsx-no-undef": 2,
     "react/jsx-wrap-multilines": 2,
     "react/no-string-refs": 0,
-    "react/prop-types": [1, {"ignore": ["className", "children", "style"]}]
+    "react/prop-types": [1, { "ignore": ["className", "children", "style"] }],
+    "react-hooks/rules-of-hooks": 2,
+    "react-hooks/exhaustive-deps": 1
   },
-  "plugins": [
-    "import",
-    "react"
-  ]
+  "plugins": ["import", "react", "react-hooks"]
 }

package-lock.json

@@ -87,6 +87,7 @@
     "eslint-plugin-import": "^2.14.0",
     "eslint-plugin-prettier": "^3.0.0",
     "eslint-plugin-react": "^7.11.1",
+    "eslint-plugin-react-hooks": "^1.6.0",
     "gh-pages": "^1.0.0",
     "jest": "^23.6.0",
     "prettier": "1.15.3",

package.json

@@ -1,6 +1,7 @@
 /* global document, jest, expect, beforeAll, afterAll */
 import React from 'react'
 import Enzyme from 'enzyme'
+import PropTypes from 'prop-types'
 import Adapter from 'enzyme-adapter-react-16'
 import { Swipeable, useSwipeable, LEFT, RIGHT, UP, DOWN } from '../index'
 import { createTouchEventObject as cte, createMouseEventObject as cme } from './helpers/events'
@@ -55,14 +56,22 @@ function mockListenersSetup(el) {
  */
 function SwipeableUsingHook(props) {
   const eventHandlers = useSwipeable(props)
+  const Elem = props.nodeName
   // Use innerRef prop to access the mounted div for testing.
   const ref = el => (props.innerRef && props.innerRef(el), eventHandlers.ref(el)) // eslint-disable-line
   return (
-    <div {...eventHandlers} ref={ref}>
+    <Elem {...eventHandlers} ref={ref}>
       {props.children}
-    </div>
+    </Elem>
   )
 }
+SwipeableUsingHook.propTypes = {
+  nodeName: PropTypes.string
+}
+
+SwipeableUsingHook.defaultProps = {
+  nodeName: 'div'
+}
 
 function setupGetMountedComponent(TYPE, mockListeners = mockListenersSetup) {
   return props => {
@@ -387,7 +396,7 @@ function setupGetMountedComponent(TYPE, mockListeners = mockListenersSetup) {
       })
     })
 
-    it('Cleans up and re-attaches touch event listeners', () => {
+    it('Cleans up and re-attaches touch event listeners if trackTouch changes', () => {
       let spies
       const mockListeners = el => {
         // already spying
@@ -413,6 +422,32 @@ function setupGetMountedComponent(TYPE, mockListeners = mockListenersSetup) {
       expect(spies.addEventListener).toHaveBeenCalledTimes(6)
       expect(spies.removeEventListener).toHaveBeenCalledTimes(3)
     })
+
+    it('Cleans up and re-attaches touch event listeners if the DOM element changes', () => {
+      let spies
+      const mockListeners = el => {
+        // already spying
+        if (spies) return
+        spies = {}
+        spies.addEventListener = jest.spyOn(el, 'addEventListener')
+        spies.removeEventListener = jest.spyOn(el, 'removeEventListener')
+      }
+      const { wrapper } = setupGetMountedComponent(TYPE, mockListeners)({})
+
+      expect(spies.addEventListener).toHaveBeenCalledTimes(3)
+      expect(spies.removeEventListener).not.toHaveBeenCalled()
+
+      wrapper.setProps({ nodeName: 'p' })
+
+      expect(spies.addEventListener).toHaveBeenCalledTimes(3)
+      expect(spies.removeEventListener).toHaveBeenCalledTimes(3)
+      // VERIFY REMOVED HANDLERS ARE THE SAME ONES THAT WERE ADDED!
+      expect(spies.addEventListener.mock.calls.length).toBe(3)
+      spies.addEventListener.mock.calls.forEach((call, idx) => {
+        expect(spies.removeEventListener.mock.calls[idx][0]).toBe(call[0])
+        expect(spies.removeEventListener.mock.calls[idx][1]).toBe(call[1])
+      })
+    })
   })
 
   it(`${TYPE} handles updated prop swipe callbacks`, () => {
@@ -423,15 +458,22 @@ function setupGetMountedComponent(TYPE, mockListeners = mockListenersSetup) {
     }
     const onSwipedLeft = jest.fn()
 
+    function TestHookComponent({ next }) {
+      const handlers = useSwipeable({ onSwipedLeft: next })
+      // Use innerRef to access the mounted div for testing.
+      const ref = el => (innerRef(el), handlers.ref(el))
+      return <div {...handlers} ref={ref} />
+    }
+    TestHookComponent.propTypes = {
+      next: PropTypes.func.isRequired
+    }
+
     function TestComponent() {
       const [page, setPage] = React.useState(0)
       const next = () => (setPage(page + 1), onSwipedLeft(page + 1))
 
       if (TYPE === USESWIPEABLE) {
-        const handlers = useSwipeable({ onSwipedLeft: next })
-        // Use innerRef to access the mounted div for testing.
-        const ref = el => (innerRef(el), handlers.ref(el))
-        return <div {...handlers} ref={ref} />
+        return <TestHookComponent next={next} />
       }
       if (TYPE === SWIPEABLE) {
         // Use innerRef to access the mounted div for testing.

src/__tests__/index.spec.js

@@ -161,22 +161,6 @@ function getHandlers(set, handlerProps) {
     })
   }
 
-  // update state, and handlers
-  set((state, props) => {
-    let addState = {}
-    // clean up touch handlers if no longer tracking touches
-    if (!props.trackTouch && state.cleanUpTouch) {
-      state.cleanUpTouch()
-      addState.cleanUpTouch = null
-    } else if (props.trackTouch && !state.cleanUpTouch) {
-      // attach/re-attach touch handlers
-      if (state.el) {
-        addState.cleanUpTouch = attachTouch(state.el)
-      }
-    }
-    return { ...state, ...addState }
-  })
-
   // set ref callback to attach touch event listeners
   const output = { ref: onRef }
 
@@ -185,17 +169,46 @@ function getHandlers(set, handlerProps) {
     output.onMouseDown = onStart
   }
 
-  return output
+  return [output, attachTouch]
+}
+
+function updateTransientState(state, props, attachTouch) {
+  let addState = {}
+  // clean up touch handlers if no longer tracking touches
+  if (!props.trackTouch && state.cleanUpTouch) {
+    state.cleanUpTouch()
+    addState.cleanUpTouch = null
+  } else if (props.trackTouch && !state.cleanUpTouch) {
+    // attach/re-attach touch handlers
+    if (state.el) {
+      addState.cleanUpTouch = attachTouch(state.el)
+    }
+  }
+  return { ...state, ...addState }
 }
 
 export function useSwipeable(props) {
+  const { trackMouse } = props
   const transientState = React.useRef({ ...initialState, type: 'hook' })
   const transientProps = React.useRef()
   transientProps.current = { ...defaultProps, ...props }
-  return getHandlers(
-    cb => (transientState.current = cb(transientState.current, transientProps.current)),
-    { trackMouse: props.trackMouse }
+
+  const [handlers, attachTouch] = React.useMemo(
+    () =>
+      getHandlers(
+        cb => (transientState.current = cb(transientState.current, transientProps.current)),
+        { trackMouse }
+      ),
+    [trackMouse]
   )
+
+  transientState.current = updateTransientState(
+    transientState.current,
+    transientProps.current,
+    attachTouch
+  )
+
+  return handlers
 }
 
 export class Swipeable extends React.PureComponent {
@@ -228,7 +241,8 @@ export class Swipeable extends React.PureComponent {
 
   render() {
     const { className, style, nodeName = 'div', innerRef, children, trackMouse } = this.props
-    const handlers = getHandlers(this._set, { trackMouse })
+    const [handlers, attachTouch] = getHandlers(this._set, { trackMouse })
+    this.transientState = updateTransientState(this.transientState, this.props, attachTouch)
     const ref = innerRef ? el => (innerRef(el), handlers.ref(el)) : handlers.ref
     return React.createElement(nodeName, { ...handlers, className, style, ref }, children)
   }