Skip to content

Commit

Permalink
chore: resolve react-hooks/exhaustive-deps warnings (#1991)
Browse files Browse the repository at this point in the history
  • Loading branch information
Boris Yordanov authored Mar 30, 2021
1 parent fca261a commit bc0f242
Show file tree
Hide file tree
Showing 17 changed files with 112 additions and 112 deletions.
7 changes: 4 additions & 3 deletions cypress/component/Autocomplete.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState, useCallback } from 'react'
import React, { useCallback, useState } from 'react'
import { mount } from '@cypress/react'
import debounce from 'debounce'
import {
Expand Down Expand Up @@ -67,9 +67,10 @@ export const DynamicOptionsAutocompleteExample = () => {
const [options, setOptions] = useState<typeof OPTIONS>([])
const [loading, setLoading] = useState(false)

// eslint-disable-next-line react-hooks/exhaustive-deps
const handleChangeDebounced = useCallback(
debounce(async (inputValue: string) => {
const newOptions = await loadOptions(inputValue.trim().toLowerCase())
debounce(async (newValue: string) => {
const newOptions = await loadOptions(newValue.trim().toLowerCase())

setLoading(false)
setOptions(newOptions)
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"@svgr/cli": "^5.5.0",
"@testing-library/react": "^11.2.5",
"@testing-library/react-hooks": "^5.0.3",
"@toptal/browserslist-config": "^0.1.0-alpha.2",
"@toptal/browserslist-config": "^1.1.0",
"@toptal/davinci": "^1.0.63",
"@types/debounce": "^1.2.0",
"@types/esprima": "^4.0.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/picasso-forms/src/FieldWrapper/FieldWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ const FieldWrapper = <
clearValidators(name)
}
}
}, [])
}, [clearValidators, formConfig.validateOnSubmit, name])

const { meta, input } = useField<TInputValue>(name, {
validate: formConfig.validateOnSubmit ? undefined : validators,
Expand Down
75 changes: 34 additions & 41 deletions packages/picasso-forms/src/Form/Form.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React, { useCallback, useMemo, ReactNode, useRef } from 'react'
import React, { useMemo, ReactNode, useRef } from 'react'
import {
Form as FinalForm,
FormProps as FinalFormProps
} from 'react-final-form'
import { FormApi, SubmissionErrors, getIn, setIn } from 'final-form'
import { FormApi, SubmissionErrors, getIn, setIn, AnyObject } from 'final-form'
import { Form as PicassoForm, Container } from '@toptal/picasso'
import { useNotifications } from '@toptal/picasso/utils'

Expand Down Expand Up @@ -31,8 +31,6 @@ import {
createFormContext
} from './FormContext'

type AnyObject = Record<string, any>

export type Props<T = AnyObject> = FinalFormProps<T> & {
autoComplete?: HTMLFormElement['autocomplete']
successSubmitMessage?: ReactNode
Expand All @@ -42,10 +40,10 @@ export type Props<T = AnyObject> = FinalFormProps<T> & {

const getValidationErrors = (
validators: Validators,
formValues: Record<string, any>,
form: FormApi<Record<string, any>>
) => {
let errors: Record<string, any> | undefined
formValues: any,
form: FormApi<any>
): SubmissionErrors | void => {
let errors: SubmissionErrors

Object.entries(validators).forEach(([key, validator]) => {
const fieldValue = getIn(formValues, key)
Expand All @@ -57,15 +55,15 @@ const getValidationErrors = (

const error = validator(fieldValue, formValues, fieldMetaState)

if (error) {
errors = setIn(errors || {}, key, error)
if (errors && error) {
errors = setIn(errors, key, error)
}
})

return errors
}

export const Form = <T extends any = Record<string, any>>(props: Props<T>) => {
export const Form = <T extends any = AnyObject>(props: Props<T>) => {
const {
children,
autoComplete,
Expand Down Expand Up @@ -105,36 +103,31 @@ export const Form = <T extends any = Record<string, any>>(props: Props<T>) => {
showError(failedSubmitMessage, undefined, { persist: true })
}

const handleSubmit = useCallback(
async (values, form, callback) => {
const validationErrors = getValidationErrors(
validationObject.current.getValidators(),
values,
form
)

if (validationErrors) {
return validationErrors
}

const submissionErrors = await onSubmit(values, form, callback)

if (!submissionErrors) {
showSuccessNotification()
} else {
showErrorNotification(submissionErrors)
}

return submissionErrors
},
[
failedSubmitMessage,
onSubmit,
showError,
showSuccess,
successSubmitMessage
]
)
const handleSubmit = async (
values: T,
form: FormApi<T>,
callback?: (errors?: SubmissionErrors) => void
) => {
const validationErrors = getValidationErrors(
validationObject.current.getValidators(),
values,
form
)

if (validationErrors) {
return validationErrors
}

const submissionErrors = await onSubmit(values, form, callback)

if (!submissionErrors) {
showSuccessNotification()
} else {
showErrorNotification(submissionErrors)
}

return submissionErrors
}

return (
<FormContext.Provider value={validationObject}>
Expand Down
4 changes: 2 additions & 2 deletions packages/picasso-lab/src/TreeView/PointNode/PointNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const PointNode = forwardRef<any, Props>(
const xPosition = node.x - nodeWidth / 2

return `translate(${xPosition},${node.y})`
}, [node.x, node.y])
}, [node.x, node.y, nodeWidth])

useLayoutEffect(() => {
if (nodeRef.current) {
Expand All @@ -41,7 +41,7 @@ export const PointNode = forwardRef<any, Props>(
})
}
}
}, [nodeRef.current, dimensions])
}, [dimensions, node.ref])

return (
<g id={node.id} transform={transform} ref={ref}>
Expand Down
13 changes: 8 additions & 5 deletions packages/picasso-lab/src/TreeView/TreeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,20 @@ export const TreeView = (props: Props) => {
const classes = useStyles()
const rootRef = createRef<SVGSVGElement>()
const { nodes, links, selectedNode } = useTree({ data })

const center = useMemo<{ x: number; y: number } | undefined>(() => {
if (!selectedNode) {
return undefined
}
const { x: xPosition, y: yPosition, data } = selectedNode

const { x: xPosition, y: yPosition, data: nodeData } = selectedNode

return {
x: xPosition + (data.selectedOffset?.x || 0),
y: yPosition + (data.selectedOffset?.y || 0)
x: xPosition + (nodeData.selectedOffset?.x || 0),
y: yPosition + (nodeData.selectedOffset?.y || 0)
}
}, [selectedNode, selectedNode?.data])
}, [selectedNode])

const { handleZoom, zoom } = useZoom<SVGSVGElement>({
rootRef,
scaleExtent,
Expand All @@ -81,7 +84,7 @@ export const TreeView = (props: Props) => {
zoom
})
setInitialized(true)
}, [rootRef, initialized, zoom])
}, [rootRef, initialized, zoom, updateState])

return (
<div className={classes.root}>
Expand Down
5 changes: 3 additions & 2 deletions packages/picasso-lab/src/TreeView/useNodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export const useNodes = (
const [initialized, setInitializedState] = useState<boolean>(false)
const initialNodes = useMemo(() => {
return getDynamicNodes(rootNode.descendants())
}, [])
}, [rootNode])
const dynamicNodes = useMemo(() => {
const latestNodes = rootNode.descendants()

Expand All @@ -85,9 +85,10 @@ export const useNodes = (
})
}, [rootNode, initialNodes])

// we have to render nodes twice: first for the initial showing data, and the second one — with the correct positions.
const nodes = useMemo<DynamicPointNode[]>(() => {
return updateNodesYPosition(dynamicNodes)
// we have to render nodes twice: first for the initial showing data, and the second one — with the correct positions.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [dynamicNodes, initialized])

useLayoutEffect(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/picasso-lab/src/TreeView/useTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const useTree = ({
leaves,
nodeWidth: fullNodeWidth
})
}, [data])
}, [data, fullNodeWidth])

const nodes = useNodes(rootNode)

Expand Down
2 changes: 1 addition & 1 deletion packages/picasso-lab/src/TreeView/useZoom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export const useZoom = <ZoomRefElement extends ZoomedElementBaseType>({
.duration(750)
.call(zoom.translateTo, center.x, center.y)
}
}, [rootRef.current, zoom, initialized, center])
}, [zoom, initialized, center, rootRef, initialScale])

return {
zoom,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState, useCallback } from 'react'
import React, { useCallback, useState } from 'react'
import debounce from 'debounce'
import { Autocomplete, AutocompleteItem } from '@toptal/picasso'
import { isSubstring } from '@toptal/picasso/utils'
Expand Down Expand Up @@ -38,6 +38,7 @@ const Example = () => {
const [options, setOptions] = useState<AutocompleteItem[] | null>()
const [loading, setLoading] = useState(false)

// eslint-disable-next-line react-hooks/exhaustive-deps
const handleChangeDebounced = useCallback(
debounce(async (inputValue: string) => {
const newOptions = await loadOptions(inputValue.trim().toLowerCase())
Expand Down
12 changes: 5 additions & 7 deletions packages/picasso/src/Modal/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,7 @@ const generateKey = (() => {
})()

// eslint-disable-next-line react/display-name
export const Modal = forwardRef<HTMLElement, Props>(function Modal(
props,
ref
) {
export const Modal = forwardRef<HTMLElement, Props>(function Modal(props, ref) {
const {
children,
open,
Expand Down Expand Up @@ -175,23 +172,24 @@ export const Modal = forwardRef<HTMLElement, Props>(function Modal(
const resetBodyOverflow = () => {
document.body.style.overflow = bodyOverflow.current
}
const currentModal = modalId.current

if (open) {
// TODO: to be improved as part of https://toptal-core.atlassian.net/browse/FX-1069
bodyOverflow.current = document.body.style.overflow
document.body.style.overflow = 'hidden'

defaultManager.add(modalId.current)
defaultManager.add(currentModal)
} else {
resetBodyOverflow()

defaultManager.remove(modalId.current)
defaultManager.remove(currentModal)
}

return () => {
resetBodyOverflow()

defaultManager.remove(modalId.current)
defaultManager.remove(currentModal)
}
}, [open])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ const useEnterOrSpaceKeyDownHandler = <
},
[
canOpen,
open,
filteredOptions,
highlightedIndex,
closeOnEnter,
handleSelect
handleSelect,
open,
close
]
)

Expand Down
4 changes: 2 additions & 2 deletions packages/picasso/src/SidebarItem/SidebarItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const useStyles = makeStyles<Theme>(styles, {
})

export const SidebarItem: OverridableComponent<Props> = memo(
forwardRef<HTMLElement, Props>(function SidebarItem(props, ref) {
forwardRef<HTMLElement, Props>(function SidebarItem (props, ref) {
const {
as,
children,
Expand Down Expand Up @@ -91,7 +91,7 @@ export const SidebarItem: OverridableComponent<Props> = memo(
{menu}
</SubMenuContext.Provider>
),
[menu]
[index, menu]
)

const titleCase = useTitleCase(propsTitleCase)
Expand Down
4 changes: 3 additions & 1 deletion packages/picasso/src/utils/use-width-of.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@ export interface ReferenceObject {
const useWidthOf = <T extends ReferenceObject>(element: T | null) => {
const [menuWidth, setMenuWidth] = useState<string | undefined>()

const offsetParent = element?.offsetParent

useLayoutEffect(() => {
if (!element) {
return
}
const { width } = element.getBoundingClientRect()

setMenuWidth(`${width}px`)
}, [element, element?.offsetParent])
}, [element, offsetParent])

return menuWidth
}
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/src/Favicon/Favicon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const Favicon = ({ environment }: Props) => {
}

loadIcons()
}, [resolvedEnvironment])
}, [resolvedEnvironment, setIcons])

if (resolvedEnvironment === 'test') {
// do not load favicons in tests (e.g. in e2e)
Expand Down
1 change: 1 addition & 0 deletions packages/shared/src/Picasso/config/breakpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ export const useScreens = <T = unknown>() => {

return defaultValue
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[screenKey]
)
}
Expand Down
Loading

0 comments on commit bc0f242

Please sign in to comment.