Add hooks linter

.eslintrc.json

@@ -2,7 +2,8 @@
   "extends": [
     "plugin:github/es6",
     "plugin:github/react",
-    "plugin:jsx-a11y/recommended"
+    "plugin:jsx-a11y/recommended",
+    "plugin:react-hooks/recommended"
   ],
   "rules": {
     "import/no-namespace": 0,
@@ -11,6 +12,7 @@
       "ignoreRestSiblings": true
     }],
     "eslint-comments/no-use": 0,
+    "react-hooks/exhaustive-deps": "error",
     "jsx-a11y/label-has-for": [
       2,
       {

package.json

@@ -41,8 +41,8 @@
     "@styled-system/prop-types": "5.1.2",
     "@styled-system/props": "5.1.4",
     "@styled-system/theme-get": "5.1.2",
-    "@types/styled-components": "^4.4.0",
     "@testing-library/react": "9.4.0",
+    "@types/styled-components": "^4.4.0",
     "@types/styled-system": "5.1.2",
     "babel-plugin-macros": "2.6.1",
     "babel-polyfill": "6.26.0",
@@ -69,6 +69,7 @@
     "eslint-plugin-github": "3.4.0",
     "eslint-plugin-jest": "22.20.0",
     "eslint-plugin-jsx-a11y": "6.2.3",
+    "eslint-plugin-react-hooks": "3.0.0",
     "jest": "25.0.0",
     "jest-styled-components": "6.3.3",
     "jscodeshift": "0.6.4",

src/Pagination/Pagination.js

@@ -93,7 +93,7 @@ function usePaginationPages({
         </Page>
       )
     })
-  }, [model, hrefBuilder, pageChange])
+  }, [model, hrefBuilder, pageChange, theme])
 
   return children
 }

src/SelectMenu/SelectMenuTab.js

@@ -63,7 +63,7 @@ const SelectMenuTab = ({tabName, index, className, onClick, ...rest}) => {
     if (!menuContext.selectedTab && index === 0) {
       menuContext.setSelectedTab(tabName)
     }
-  }, [])
+  }, [index, menuContext, tabName])
 
   const isSelected = menuContext.selectedTab === tabName
 

src/SelectMenu/hooks/useKeyboardNav.js

@@ -1,82 +1,89 @@
-import {useEffect} from 'react'
+import {useEffect, useCallback} from 'react'
 
 // adapted from details-menu web component https://github.com/github/details-menu-element
 function useKeyboardNav(details, open, setOpen) {
-  const handleKeyDown = event => {
-    const closeDetails = () => {
-      setOpen(false)
-      const summary = details.current.querySelector('summary')
-      if (summary) summary.focus()
-    }
-    const openDetails = () => {
-      setOpen(true)
-    }
-    const focusItem = next => {
-      const options = Array.from(
-        details.current.querySelectorAll('[role^="menuitem"]:not([hidden]):not([disabled]):not([aria-disabled="true"])')
-      )
-      const selected = document.activeElement
-      const index = options.indexOf(selected)
-      const found = next ? options[index + 1] : options[index - 1]
-      const def = next ? options[0] : options[options.length - 1]
-      return found || def
-    }
+  const handleKeyDown = useCallback(
+    event => {
+      const closeDetails = () => {
+        setOpen(false)
+        const summary = details.current.querySelector('summary')
+        if (summary) summary.focus()
+      }
+      const openDetails = () => {
+        setOpen(true)
+      }
+      const focusItem = next => {
+        const options = Array.from(
+          details.current.querySelectorAll(
+            '[role^="menuitem"]:not([hidden]):not([disabled]):not([aria-disabled="true"])'
+          )
+        )
+        const selected = document.activeElement
+        const index = options.indexOf(selected)
+        const found = next ? options[index + 1] : options[index - 1]
+        const def = next ? options[0] : options[options.length - 1]
+        return found || def
+      }
 
-    const isMenuItem = el => {
-      const role = el.getAttribute('role')
-      return role === 'menuitem' || role === 'menuitemcheckbox' || role === 'menuitemradio'
-    }
-    if (!(event instanceof KeyboardEvent)) return
-    const isSummaryFocused = event.target instanceof Element && event.target.tagName === 'SUMMARY'
-    switch (event.key) {
-      case 'Escape':
-        if (open) {
-          closeDetails(details)
-          event.preventDefault()
-          event.stopPropagation()
-        }
-        break
-      case 'ArrowDown':
-        {
-          if (isSummaryFocused && !open) {
-            openDetails(details)
+      const isMenuItem = el => {
+        const role = el.getAttribute('role')
+        return role === 'menuitem' || role === 'menuitemcheckbox' || role === 'menuitemradio'
+      }
+      if (!(event instanceof KeyboardEvent)) return
+      const isSummaryFocused = event.target instanceof Element && event.target.tagName === 'SUMMARY'
+      switch (event.key) {
+        case 'Escape':
+          if (open) {
+            closeDetails(details)
+            event.preventDefault()
+            event.stopPropagation()
           }
-          const target = focusItem(true)
-          if (target) target.focus()
-          event.preventDefault()
-        }
-        break
-      case 'ArrowUp':
-        {
-          if (isSummaryFocused && !open) {
-            openDetails()
+          break
+        case 'ArrowDown':
+          {
+            if (isSummaryFocused && !open) {
+              openDetails(details)
+            }
+            const target = focusItem(true)
+            if (target) target.focus()
+            event.preventDefault()
           }
-          const target = focusItem(false)
-          if (target) target.focus()
-          event.preventDefault()
-        }
-        break
-      case ' ':
-      case 'Enter':
-        {
-          const selected = document.activeElement
-          if (selected && isMenuItem(selected) && selected.closest('details') === details) {
+          break
+        case 'ArrowUp':
+          {
+            if (isSummaryFocused && !open) {
+              openDetails()
+            }
+            const target = focusItem(false)
+            if (target) target.focus()
             event.preventDefault()
-            event.stopPropagation()
-            selected.click()
           }
-        }
-        break
-    }
-  }
+          break
+        case ' ':
+        case 'Enter':
+          {
+            const selected = document.activeElement
+            if (selected && isMenuItem(selected) && selected.closest('details') === details) {
+              event.preventDefault()
+              event.stopPropagation()
+              selected.click()
+            }
+          }
+          break
+      }
+    },
+    [details, open, setOpen]
+  )
+
   useEffect(() => {
-    if (!details.current) return
+    const current = details.current
+    if (!current) return
 
-    details.current.addEventListener('keydown', handleKeyDown)
+    current.addEventListener('keydown', handleKeyDown)
     return () => {
-      details.current.removeEventListener('keydown', handleKeyDown)
+      current.removeEventListener('keydown', handleKeyDown)
     }
-  }, [details.current])
+  }, [details, handleKeyDown])
 }
 
 export default useKeyboardNav

yarn.lock

@@ -4837,6 +4837,11 @@ eslint-plugin-prettier@>=2.6.0:
   dependencies:
     prettier-linter-helpers "^1.0.0"
 
+eslint-plugin-react-hooks@3.0.0:
+  version "3.0.0"
+  resolved "https://registry.yarnpkg.com/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-3.0.0.tgz#9e80c71846eb68dd29c3b21d832728aa66e5bd35"
+  integrity sha512-EjxTHxjLKIBWFgDJdhKKzLh5q+vjTFrqNZX36uIxWS4OfyXe5DawqPj3U5qeJ1ngLwatjzQnmR0Lz0J0YH3kxw==
+
 eslint-plugin-react@>=7.7.0:
   version "7.17.0"
   resolved "https://registry.yarnpkg.com/eslint-plugin-react/-/eslint-plugin-react-7.17.0.tgz#a31b3e134b76046abe3cd278e7482bd35a1d12d7"