This is a non-exhaustive list of the coding patterns the WorkWave RouteManager's front-end team follows. The patterns are based on years of experience writing, debugging, and refactoring front-end applications with React and TypeScript but evolves constantly. Most of the possible improvements and the code smells are detected during the code reviews and the pair programming sessions.
(please note: I do not work for WorkWave anymore, these patterns will not be updated)
(last update: 2022, March)
- Avoid Component's lifecycle-like hooks
- Always create co-located hooks
- Never omit useEffect's dependency array
- Don't memoize primitives when not necessary
- Specify when something isn't memoized
- Don't memoize always-spread collections
- Always use useCallback
- If callbacks are just pure functions, move them outside of the component/hook
- Always move hooks-based code in custom hooks
- Leave comment for counter-intuitive dependencies
- Update all the refs with a single useEffect
- Check if the component is still mounted in async callbacks
- Check if the hook is still active in async effects
- Don't use multiple states if not necessary
- Don't over-use refs
- The cases for useReducer
- Return useState-like array if possible
- Never return useState-like arrays if the consumer doesn't destructure them
- Return objects with descriptive key names
- Reduce the reactions
Avoid Component's lifecycle-like hooks
Using hooks like useMount, useDidUpdate, useUnmount leads to mistakes and prevent thinking the hooks way. More: they prevent ESLint from prompting you about some slight but huge errors with the dependencies array.
// ❌ don't
useMount(() => console.log('Component mounted!'))
// ✅ do
useEffect(() => console.log('Effect triggered!'), [])
Always create co-located hooks
React Hooks were built with co-location in mind, avoid creating generic-purpose hooks located away from their consumers.
// ❌ don't
import { useMount } from '@/hooks/useMount'
function Dialog(props:Props) {
useMount(() => {
autoSubmit()
})
/* ... rest of the code... */
})
// ✅ do
import { useAutoSubmit } from './useAutoSubmit'
function Dialog(props:Props) {
useAutoSubmit()
/* ... rest of the code... */
})
Never omit useEffect's dependency array
Effects without a dependency array trigger at every render, usually something unintended
// ❌ don't
useEffect(() => /* ... rest of the code... */)
// ✅ do
useEffect(() => /* ... rest of the code... */, [])
Don't memoize primitives when not necessary
Memoization is helpful to
- avoid component re-renders
- avoid expensive computations
the former doesn't happen in case the memoized value is a primitive.
// ❌ don't
const isViewer = useMemo(() => {
return role === 'viewer'
}, [role])
// ✅ do
const isViewer = role === 'viewer'
Specify when something isn't memoized
We memoize almost everything, please leave a comment when something isn't memoized on purpose.
// ❌ don't
function useActions() {
return {
foo: () => {},
bar: () => {},
}
})
// ✅ do
function useActions() {
// not memoized because always consumed spread
return {
foo: () => {},
bar: () => {},
}
})
Don't memoize always-spread collections
When the return value of a hook isn't a "unit" but just a way to return multiple, unrelated, items, and the consumers always spread them, don't memoize them.
function MyComponent() {
const { foo, bar } = useActions()
return <>
<button onClick={foo} />
<button onClick={bar} />
</>
})
// ❌ don't
function useActions() {
return useMemo(() => {
return {
foo: () => {},
bar: () => {},
}
}, [])
})
// ✅ do
function useActions() {
// not memoized because always consumed spread
return {
foo: () => {},
bar: () => {},
}
})
Always use useCallback
Never creates callbacks on the fly. It's not about performances (usually) but about avoiding triggering sub-components effects.
// ❌ don't
function Footer() {
const foo = () => {
/* ... rest of the code... */
}
return <>
<AboutUsButton onClick={foo} />
<PrivacyButton onClick={() => {
/* ... rest of the code... */
}} />
</>
})
// ✅ do
function Footer() {
const foo = useCallback(() => {
/* ... rest of the code... */
}, [])
const bar = useCallback(() => {
/* ... rest of the code... */
}, [])
return <>
<AboutUsButton onClick={foo} />
<PrivacyButton onClick={bar} />
</>
})
If callbacks are just pure functions, move them outside of the component/hook
Hooks makes code harder to read, don't use them if not necessary.
// ❌ don't
import { action } from './actions'
function Footer() {
const onClick = useCallback(() => action(), [])
return <Icon onClick={onClick} />
})
// ✅ do
import { action } from './actions'
const onClick = () => action()
function Footer() {
return <Icon onClick={onClick} />
})
Always move hooks-based code in custom hooks
Built-in hooks should be hidden behind custom hooks which name explain their meaning.
// ❌ don't
function Dialog(props:Props) {
const { submit, programmatic, close } = props
useEffect(() => {
if(programmatic) {
submit()
close()
}
}, [submit, programmatic, close])
return <Buttons onSubmit={submit} onClose={close} />
})
// ✅ do
function useAutoSubmit(programmatic: boolean, submit: () => void, close: () => void) {
useEffect(() => {
if(programmatic) {
submit()
close()
}
}, [submit, programmatic, close])
}
function Dialog(props:Props) {
const { programmatic, submit, close } = props
useAutoSubmit(programmatic, submit, close)
return <Buttons onSubmit={submit} onClose={close} />
})
Then, move useAutoSubmit to a dedicated file.
// ❌ don't
function useAutoSubmit(programmatic: boolean, submit: () => void, close: () => void) {
useEffect(() => {
if(programmatic) {
submit()
close()
}
}, [submit, programmatic, close])
}
function Dialog(props:Props) {
const { programmatic, submit, close } = props
useAutoSubmit(programmatic, submit, close)
return <Buttons onSubmit={submit} onClose={close} />
})
// ✅ do
import { useAutoSubmit } from './hooks/useAutoSubmit'
function Dialog(props:Props) {
const { programmatic, submit, close } = props
useAutoSubmit(programmatic, submit, close)
return <Buttons onSubmit={submit} onClose={close} />
})
Leave comment for counter-intuitive dependencies
Non-primitive variables trigger the reader's attention when they are part of the dependency arrays. When this is made on purpose, please leave a comment.
// ❌ don't
const { coordinates } = props
useEffect(() => {
scrollbar.scrollTo(coordinates.x, coordinates.y)
}, [coordinates])
// ✅ do
const { coordinates } = props
// The effect must depend directly on coordinates' x and y, otherwise the scrollbar can't be
// scrolled twice at the same position. The So, the consumer must memoize the `coordinates`
// object, otherwise the scrollbar scrolls at every render.
useEffect(() => {
scrollbar.scrollTo(coordinates.x, coordinates.y)
}, [coordinates])
Update all the refs with a single useEffect
If components/hooks have a lot of related refs, store them in a single ref and update them through a single useEffect. More: refs-related useEffects can be dependencies-free.
// ❌ don't
const bookTitleRef = useRef(bookTitle)
useEffect(() => void (bookTitleRef.current = bookTitle))
const bookAuthorRef = useRef(bar)
useEffect(() => void (bookAuthorRef.current = bookAuthor))
const bookPublisherRef = useRef(bookPublisher)
useEffect(() => void (bookPublisherRef.current = bookPublisher))
// ✅ do
const bookRef = useRef({ bookTitle, bookAuthor, bookPublisher })
useEffect(() => void (refs.current = { bookTitle, bookAuthor, bookPublisher }))
Check if the component is still mounted in async callbacks
A component can be unmounted before an asynchronous execution completes, resulting in unexpected behaviors and errors. The useIsUnmounted hooks is made on purpose.
// ❌ don't
export function Component() {
const [deleted, setDeleted] = useState(false)
const onClick = useCallback(async () => {
await deleteItemRequest()
setDeleted(true)
}, [])
return <>
<button onClick={onClick}>Delete</button>
{deleted && <>Deleted</>}
</>
}
// ✅ do
export function Component() {
const [deleted, setDeleted] = useState(false)
const isUnmounted = useIsUnmounted()
const onClick = useCallback(async () => {
await deleteItemRequest()
if (isUnmounted()) return
setDeleted(true)
}, [isUnmounted])
return <>
<button onClick={onClick}>Delete</button>
{deleted && <>Deleted</>}
</>
}
/*
// useIsUnmounted implementation
export function useIsUnmounted() {
const rIsUnmounted = useRef<'mounting' | 'mounted' | 'unmounted'>('mounting')
useLayoutEffect(() => {
rIsUnmounted.current = 'mounted'
return () => void (rIsUnmounted.current = 'unmounted')
}, [])
return useCallback(() => rIsUnmounted.current !== 'mounted', [])
}
*/
Check if the hook is still active in async effects
An hooks can be cleaned up before an asynchronous execution completes, resulting in unexpected behaviors and errors.
// ❌ don't
export function useFetchItems() {
const [items, setItems] = useState()
useEffect(() => {
const execute = async () => {
const response = await fetchItems()
setItems(response)
}
execute()
}, [])
return items
}
// ✅ do
export function useFetchItems() {
const [items, setItems] = useState()
useEffect(() => {
let effectCleaned = false
const execute = async () => {
const response = await fetchItems()
if (effectCleaned) return
setItems(response)
}
execute()
return () => {
effectCleaned = true
}
}, [])
return items
}
Don't use multiple states if not necessary
If multiple states are related to the same entity or you mostly set them all at once, prefer a single state.
// ❌ don't
export function useOrder(order: Order) {
const [name, setName] = useState('')
const [depot, setDepot] = useState('')
const [eligibility, setEligibility] = useState('any')
useEffect(() => {
setName(order.name)
setDepot(order.depot)
setEligibility(order.eligibility)
}, [order.name, order.depot, order.eligibility])
return {
name,
depot,
eligibility,
}
}
// ✅ do
export function useOrder(order: Order) {
const [localOrder, setLocalOrder] = useState({})
useEffect(() => {
setLocalOrder({
name: order.name,
depot: order.depot,
eligibility: order.eligibility,
})
}, [order.name, order.depot, order.eligibility])
return localOrder
}
Don't over-use refs
Don't put everything in a ref to optimize re-renders. A few re-renders are totally acceptable if they don't occur frequently and if avoiding them compromises the readability.
// ❌ don't
export function useActions(
/* ... rest of the code... */
) {
const { validateOnBlur } = validations
const api = useRef({
allowUnlistedValues,
setInputValue,
setTimeValue,
validations,
timeFormat,
inputValue,
timeValue,
setOpen,
options,
})
useEffect(() => {
api.current.allowUnlistedValues = allowUnlistedValues
api.current.setInputValue = setInputValue
api.current.setTimeValue = setTimeValue
api.current.validations = validations
api.current.timeFormat = timeFormat
api.current.inputValue = inputValue
api.current.timeValue = timeValue
api.current.setOpen = setOpen
api.current.options = options
}, [
allowUnlistedValues,
setInputValue,
setTimeValue,
validations,
timeFormat,
inputValue,
timeValue,
setOpen,
options,
])
const onBlur = useCallback(() => {
const {
allowUnlistedValues,
setInputValue,
setTimeValue,
inputValue,
timeFormat,
timeValue,
options,
setOpen,
} = api.current
/* ... rest of the code... */
}, [validateOnBlur])
/* ... rest of the code... */
}
// ✅ do
export function useActions(
/* ... rest of the code... */
) {
const { validateOnBlur } = validations
const api = useRef({
inputValue,
timeValue,
})
useEffect(() => {
api.current.inputValue = inputValue
api.current.timeValue = timeValue
}, [
inputValue,
timeValue,
])
const onBlur = useCallback(() => {
/* ... rest of the code... */
}, [
allowUnlistedValues,
validateOnBlur,
setInputValue,
setTimeValue,
timeFormat,
options,
setOpen,
])
/* ... rest of the code... */
}
more, you can even get the useEffect shorter by using void and removing the dependency array.
// ✅ do
export function useActions(
/* ... rest of the code... */
) {
const { validateOnBlur } = validations
const api = useRef({ inputValue, timeValue })
useEffect(() => void (api.current = { inputValue, timeValue }))
const onBlur = useCallback(() => {
/* ... rest of the code... */
}, [
allowUnlistedValues,
validateOnBlur,
setInputValue,
setTimeValue,
timeFormat,
options,
setOpen,
])
/* ... rest of the code... */
}
The cases for useReducer
We exploit useReducer in the following main cases:
- managing multiples connected or co-related values: if we have a
useStatehosting non-atomic data or have many (aka more than 2-3) smalluseStatefor different values, we should opt foruseReducer - avoiding the consumer to know how the state updates and centralizing the updates too (à la Redux)
- lightening a
useEffectwhich purpose is to update an object-based state but the update logic is not trivial - having a super-shortened state setter (ex. togglers and re-renderers)
Return useState-like arrays if possible
Even if a variable isn't stored in a React state, return an array to recall the useState signature.
// ❌ don't
export function useSessionStorageState() {
/* ... rest of the code... */
return { state, setState }
}
// ✅ do
export function useSessionStorageState() {
/* ... rest of the code... */
return [state, setState]
}
Never return useState-like arrays if the consumer doesn't destructure them
If the consumer will not destructure the returned data, never use useState-like arrays.
// ❌ don't
function useText() {
return ['Undo', 'Redo']
}
export function useConsumer() {
const texts = useTexts()
console.log(texts[0])
console.log(texts[1])
}
// ✅ do
function useText() {
return {
undo: 'Undo',
redo: 'Redo',
}
}
export function useSessionStorageState() {
const texts = useTexts()
console.log(texts.undo)
console.log(texts.redo)
}
Return objects with descriptive key names
When hooks return objects, use descriptive names to avoid the consumer renaming them on destructuring.
// ❌ don't
export function useSessionStorageState() {
/* ... rest of the code... */
return { state, set, reset, delete }
}
// ✅ do
export function useSessionStorageState() {
/* ... rest of the code... */
return {
sessionStorageState,
setSessionStorageState,
resetSessionStorageState,
deleteSessionStorageState,
}
}
Reduce the reactions
Where possible, prefer performing a series of actions in the caller instead of performing them as a reaction to a state change. Reactions are hard to follow and lead to a lot of unwanted effects and conditional behaviors.
// ❌ don't
function useOnTerritoryClick() {
return useCallback(async (territoryId: string) => {
await dispatch(saveTerritoryToOpen(territoryId))
await dispatch(loadTerritories())
}, [])
}
function useOpenTerritory() {
const territories = useSelector(getTerritories)
const territoryToOpen = useSelector(getTerritoryToOpen)
const territoryToOpenRef = useRef({ territoryToOpen })
useEffect(() => {
territoryToOpenRef.current = territoryToOpen
}, [territoryToOpen])
useEffect(() => {
dispatch(openTerritory(territoryToOpenRef.current))
}, [territories])
}
// ✅ do
function useOnTerritoryClick() {
return useCallback(async (territoryId: string) => {
await dispatch(loadTerritories())
await dispatch(openTerritory(territoryId))
}, [])
}
Top comments (0)