Add eslint-plugin-react-hooks and fix lint errors

dfltr · dfltr · commit 317cdb38639d · 2025-01-07T12:05:28.000-08:00
diff --git a/package.json b/package.json
@@ -24,6 +24,7 @@
   "devDependencies": {
     "@changesets/cli": "^2.22.0",
     "eslint": "^7.32.0",
+    "eslint-plugin-react-hooks": "^5.1.0",
     "prettier": "^2.5.1",
     "rimraf": "^5.0.0",
     "turbo": "^1.10.14"
diff --git a/packages/react/.eslintrc.cjs b/packages/react/.eslintrc.cjs
@@ -1,6 +1,6 @@
 /* eslint-env node */
 module.exports = {
-  extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended', 'prettier'],
+  extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended', 'plugin:react-hooks/recommended', 'prettier'],
   ignorePatterns: ['**/dist/*'],
   parser: '@typescript-eslint/parser',
   plugins: ['@typescript-eslint', 'react-refresh'],
diff --git a/packages/react/src/components/Checklist/CarouselEmblaWrapper.tsx b/packages/react/src/components/Checklist/CarouselEmblaWrapper.tsx
@@ -69,7 +69,7 @@ export function CarouselEmblaWrapper({
       setHasNext(emblaApi.canScrollNext())
       setHasPrev(emblaApi.canScrollPrev())
     })
-  }, [emblaApi])
+  }, [emblaApi, hasNext, hasPrev])
 
   useEffect(() => {
     // check if hasCompletedInitialSort. If not then sort the steps from not completed not skipped to completed/skipped
@@ -91,7 +91,7 @@ export function CarouselEmblaWrapper({
         setStepOrder(steps.map((step) => step.id))
       }
     }
-  }, [])
+  }, [flow.steps, sort, stepOrder])
 
   const completedSteps = flow.getNumberOfCompletedSteps()
   const availableSteps = flow.getNumberOfAvailableSteps()
diff --git a/packages/react/src/components/ClientPortal/ClientPortal.tsx b/packages/react/src/components/ClientPortal/ClientPortal.tsx
@@ -6,5 +6,5 @@ export interface ClientPortalProps {
 }
 
 export function ClientPortal({ children, container = 'body' }: ClientPortalProps) {
-  return children != null ? useClientPortal(children, container) : null
+  return useClientPortal(children, container)
 }
diff --git a/packages/react/src/components/Form/FormStep.tsx b/packages/react/src/components/Form/FormStep.tsx
@@ -92,7 +92,7 @@ export function FormStep({
         }, {})
       )
     }
-  }, [fieldDatas])
+  }, [fieldDatas, formContext])
 
   useEffect(() => {
     if (
@@ -102,7 +102,7 @@ export function FormStep({
     ) {
       formContext.trigger()
     }
-  }, [step.id])
+  }, [fieldDatas, formContext, step.id])
 
   const [isSubmitting, setIsSubmitting] = useState(false)
 
@@ -136,7 +136,7 @@ export function FormStep({
 
   useEffect(() => {
     formContext.clearErrors()
-  }, [step])
+  }, [formContext, step])
 
   return (
     <>
diff --git a/packages/react/src/components/Form/fields/BaseField.tsx b/packages/react/src/components/Form/fields/BaseField.tsx
@@ -8,7 +8,7 @@ import * as styles from './BaseField.styles'
 export interface FieldProps {
   value: string
   onChange: (value: string) => void
-  [key: string]: any
+  [key: string]: unknown
 }
 
 interface BaseFieldProps extends FormFieldProps {
diff --git a/packages/react/src/components/Form/fields/CheckboxField.tsx b/packages/react/src/components/Form/fields/CheckboxField.tsx
@@ -11,7 +11,7 @@ import * as baseStyles from '@/components/Form/fields/BaseField.styles'
 export function CheckboxField(props: FormFieldProps) {
   const {
     field: { onChange, value },
-    fieldData: { options, label, id = [] },
+    fieldData: { label, id = [] },
   } = props
 
   return (
@@ -32,7 +32,7 @@ export function CheckboxField(props: FormFieldProps) {
           justifyContent="center"
           alignItems="center"
           display="flex"
-          // @ts-ignore
+          // @ts-expect-error :hover type isn't in style props yet
           backgroundColor:hover="neutral.900"
           part="field-checkbox"
           id={id as string}
diff --git a/packages/react/src/components/Form/fields/SelectMultipleField.tsx b/packages/react/src/components/Form/fields/SelectMultipleField.tsx
@@ -11,7 +11,7 @@ import * as baseStyles from '@/components/Form/fields/BaseField.styles'
 
 export function SelectMultipleField(props: FormFieldProps) {
   const {
-    field: { onChange, value },
+    field: { onChange },
     fieldData: { options = [] },
   } = props
 
diff --git a/packages/react/src/components/Form/index.tsx b/packages/react/src/components/Form/index.tsx
@@ -47,7 +47,7 @@ export interface FormFieldData extends ValidationRules {
   multiple?: boolean
   props?: Record<string, string | number | boolean | undefined>
   // allow any other custom properties
-  [key: string]: any
+  [key: string]: unknown
 }
 
 // TODO: Wire UseControllerReturn into this type
diff --git a/packages/react/src/components/Hint/index.tsx b/packages/react/src/components/Hint/index.tsx
@@ -103,7 +103,7 @@ export function Hint({
     } else if (!autoScroll) {
       setScrollComplete(true)
     }
-  }, [autoScroll, refs.reference.current])
+  }, [autoScroll, refs.reference, scrollComplete])
 
   if (refs.reference.current == null || !scrollComplete || !isVisible) {
     return null
diff --git a/packages/react/src/components/Hint/useFloatingHint.ts b/packages/react/src/components/Hint/useFloatingHint.ts
@@ -109,7 +109,7 @@ export function useFloatingHint({
     } else {
       console.debug(`[frigade] Hint: No anchor found for selector: ${anchor}`)
     }
-  }, [anchorElement])
+  }, [anchor, anchorElement, refs])
 
   // The flip() middleware might reverse the align prop
   const finalPlacement = computedPlacement.split('-')
diff --git a/packages/react/src/components/Provider/ImagePreloader.tsx b/packages/react/src/components/Provider/ImagePreloader.tsx
@@ -16,7 +16,7 @@ export function ImagePreloader() {
         })
       })
     })()
-  }, [])
+  }, [frigade])
 
   return null
 }
diff --git a/packages/react/src/components/Provider/Provider.tsx b/packages/react/src/components/Provider/Provider.tsx
@@ -163,15 +163,26 @@ export function Provider({
       __platformVersion: SDK_VERSION,
       __platformName: 'React',
     } as FrigadeConfig)
-  }, [props.userId, props.groupId, props.apiKey])
+  }, [
+    props.apiKey,
+    props.apiUrl,
+    props.userId,
+    props.groupId,
+    props.userProperties,
+    props.groupProperties,
+    props.generateGuestId,
+    props.syncOnWindowUpdates,
+    props.__readOnly,
+    props.__flowStateOverrides,
+  ])
 
   useEffect(() => {
     if (props.__flowStateOverrides) {
       const config = frigade.getConfig()
       config.__flowStateOverrides = props.__flowStateOverrides
       frigade.reload(config)
     }
-  }, [props.__flowStateOverrides])
+  }, [frigade, props.__flowStateOverrides])
 
   function batchRegistration() {
     const batchedFlowIds = [...registeredComponents.current.entries()].map(([flowId, options]) => [
@@ -236,7 +247,7 @@ export function Provider({
     return () => {
       frigade.destroy()
     }
-  }, [])
+  }, [frigade])
 
   return (
     <FrigadeContext.Provider
diff --git a/packages/react/src/components/Spotlight/index.tsx b/packages/react/src/components/Spotlight/index.tsx
@@ -105,7 +105,7 @@ export function Spotlight({ anchor, part, style = {}, ...props }: SpotlightProps
     } catch (invalidSelector) {
       /* no-op */
     }
-  }, [anchor])
+  }, [anchor, refs])
 
   return (
     <Overlay
diff --git a/packages/react/src/components/Tooltip/index.tsx b/packages/react/src/components/Tooltip/index.tsx
@@ -123,7 +123,7 @@ export function Tooltip({
     } else {
       console.debug(`[frigade] Tooltip: No anchor found for query: ${anchor}`)
     }
-  }, [anchor])
+  }, [anchor, anchorRef])
 
   useEffect(() => {
     function checkElementForAnchor(element: Element) {
@@ -193,7 +193,7 @@ export function Tooltip({
     observer.observe(document.querySelector('body'), { childList: true, subtree: true })
 
     return () => observer.disconnect()
-  }, [])
+  }, [anchor, anchorRef])
 
   useEffect(() => {
     const { scrollX, scrollY } = window
diff --git a/packages/react/src/hooks/useClientPortal.ts b/packages/react/src/hooks/useClientPortal.ts
@@ -13,7 +13,7 @@ export function useClientPortal(
     containerRef.current =
       typeof container === 'string' ? document.querySelector(container) : container
     setMounted(true)
-  }, [])
+  }, [container])
 
   return mounted ? createPortal(children, containerRef.current, key) : null
 }
diff --git a/packages/react/src/hooks/useCollection.ts b/packages/react/src/hooks/useCollection.ts
@@ -30,7 +30,7 @@ export function useCollection(collectionId?: string): {
 
   useEffect(() => {
     frigade?.registerCollection(collectionId)
-  }, [collectionId])
+  }, [collectionId, frigade])
 
   return {
     collection,
diff --git a/packages/react/src/hooks/useCollections.ts b/packages/react/src/hooks/useCollections.ts
@@ -1,4 +1,4 @@
-import { useCallback, useContext, useState } from 'react'
+import { useCallback, useContext, useRef, useState } from 'react'
 import { CollectionsList } from '@frigade/js'
 
 import { FrigadeContext } from '@/components/Provider'
@@ -8,34 +8,37 @@ export function useCollections() {
   const { frigade } = useContext(FrigadeContext)
   const [, setForceRender] = useState<boolean>(false)
 
-  let debounceTimeout: ReturnType<typeof setTimeout>
+  const debounceTimeout = useRef<ReturnType<typeof setTimeout>>()
 
-  const subscribe = useCallback((cb: () => void) => {
-    // TODO: Why is there a noticeable delay when this is commented out?
-    frigade?.getCollections().then(() => {
-      cb()
-    })
+  const subscribe = useCallback(
+    (cb: () => void) => {
+      // TODO: Why is there a noticeable delay when this is commented out?
+      frigade?.getCollections().then(() => {
+        cb()
+      })
 
-    const handler = () => {
-      clearTimeout(debounceTimeout)
+      const handler = () => {
+        clearTimeout(debounceTimeout.current)
 
-      /*
-       * NOTE: Since React doesn't re-render on deep object diffs,
-       * we need to gently prod it here by creating a state update.
-       */
-      debounceTimeout = setTimeout(() => {
-        setForceRender((forceRender) => !forceRender)
+        /*
+         * NOTE: Since React doesn't re-render on deep object diffs,
+         * we need to gently prod it here by creating a state update.
+         */
+        debounceTimeout.current = setTimeout(() => {
+          setForceRender((forceRender) => !forceRender)
 
-        cb()
-      }, 0)
-    }
+          cb()
+        }, 0)
+      }
 
-    frigade?.onStateChange(handler)
+      frigade?.onStateChange(handler)
 
-    return () => {
-      frigade?.removeStateChangeHandler(handler)
-    }
-  }, [])
+      return () => {
+        frigade?.removeStateChangeHandler(handler)
+      }
+    },
+    [frigade]
+  )
 
   const getSnapshot = () => {
     let result = undefined
diff --git a/packages/react/src/hooks/useFlow.ts b/packages/react/src/hooks/useFlow.ts
@@ -66,7 +66,7 @@ export function useFlow(
       ...variables,
       ...config?.variables,
     })
-  }, [config?.variables, flowId, variables])
+  }, [config?.variables, flow, flowId, variables])
 
   return {
     flow,
diff --git a/packages/react/src/hooks/useFlowHandlers.ts b/packages/react/src/hooks/useFlowHandlers.ts
@@ -42,7 +42,7 @@ export function useFlowHandlers(flow: Flow, { onComplete, onDismiss }: FlowHandl
     return () => {
       callHandler()
     }
-  }, [flow?.isCompleted])
+  }, [flow, onComplete])
 
   return {
     handleDismiss: useCallback<DismissHandler>(
diff --git a/packages/react/src/hooks/useModal.ts b/packages/react/src/hooks/useModal.ts
@@ -12,29 +12,29 @@ export function useCheckForModalCollision(flow: Flow, isModal = true) {
         setCurrentModal(flowId)
       }
     },
-    [isModal, flow?.isVisible]
+    [isModal, flow?.isVisible, setCurrentModal]
   )
 
   const releaseLock = useCallback(() => {
     if (flow != null && currentModal === flow?.id) {
       setCurrentModal(null)
     }
-  }, [currentModal, flow?.id])
+  }, [currentModal, flow, setCurrentModal])
 
   useEffect(() => {
     if (flow != null && currentModal === null && flow.isVisible) {
       claimLock(flow.id)
     }
 
     return releaseLock
-  }, [currentModal, flow?.id, flow?.isVisible])
+  }, [claimLock, currentModal, flow, releaseLock])
 
   // Edge case: The current modal may become non-modal while still mounted
   useEffect(() => {
     if (flow != null && (!isModal || !flow.isVisible)) {
       releaseLock()
     }
-  }, [flow?.isVisible, isModal])
+  }, [flow, isModal, releaseLock])
 
   // No flow? No problem.
   if (flow == null) {
diff --git a/packages/react/src/hooks/useStepHandlers.ts b/packages/react/src/hooks/useStepHandlers.ts
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -6,5 +6,5 @@ export interface ClientPortalProps {`
`6`	`6`	`}`
`7`	`7`
`8`	`8`	`export function ClientPortal({ children, container = 'body' }: ClientPortalProps) {`
`9`		`- return children != null ? useClientPortal(children, container) : null`
	`9`	`+ return useClientPortal(children, container)`
`10`	`10`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ import * as styles from './BaseField.styles'`
`8`	`8`	`export interface FieldProps {`
`9`	`9`	`value: string`
`10`	`10`	`onChange: (value: string) => void`
`11`		`- [key: string]: any`
	`11`	`+ [key: string]: unknown`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`interface BaseFieldProps extends FormFieldProps {`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ export interface FormFieldData extends ValidationRules {`
`47`	`47`	`multiple?: boolean`
`48`	`48`	`props?: Record<string, string \| number \| boolean \| undefined>`
`49`	`49`	`// allow any other custom properties`
`50`		`- [key: string]: any`
	`50`	`+ [key: string]: unknown`
`51`	`51`	`}`
`52`	`52`
`53`	`53`	`// TODO: Wire UseControllerReturn into this type`
Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ export function Hint({`
`103`	`103`	`} else if (!autoScroll) {`
`104`	`104`	`setScrollComplete(true)`
`105`	`105`	`}`
`106`		`- }, [autoScroll, refs.reference.current])`
	`106`	`+ }, [autoScroll, refs.reference, scrollComplete])`
`107`	`107`
`108`	`108`	`if (refs.reference.current == null \|\| !scrollComplete \|\| !isVisible) {`
`109`	`109`	`return null`