useCustomHookWithRefs
Avoiding A Common Pitfall
Question: What’s wrong with this code?
import {useIsMounted} from './useIsMounted';
function Button({callback}) {
const isMounted = useIsMounted();
const onClick = () => {
if (isMounted) {
callback();
}
}
return <button onClick={onClick}>Hallo</button>;
}
Answer: if (isMounted)
will always return true
because isMounted
is a ref
, not a boolean, and so callback
will always be called when the button is clicked, even when it shouldn’t be. There’s no way of knowing that that’s wrong without looking at the useIsMounted
definition to see what it returns. The correct form is:
import {useIsMounted} from './useIsMounted';
function Button({callback}) {
const isMounted = useIsMounted();
const onClick = () => {
if (isMounted.current) {
callback();
}
}
return <button onClick={onClick}>Hallo</button>;
}
This is a common pitfall that combines naming difficulties with JS typecasting difficulties. Even TypeScript wouldn’t catch this bug, since it’s ok to call if(foo)
on any type for foo
. It’s frustrating, because custom hooks that use useState
work pretty much exactly how you’d expect, but custom hooks that return refs can cause confusion if a dev reading over the code doesn’t know what type of value the custom hook is returning. I believe this is what we call a “footgun”.
Right now, my current recommendation for fixing this is to name the hook useIsMountedRef
to make it abundantly clear that that’s happening, and encourage naming its return value isMountedRef
instead of isMounted
. This mostly solves the problem and is ok, but I don’t love it as a solution: it’s not very elegant to have to name a function with the type it returns. I’m curious if anyone else has found a better way. Let me know if you do!
Update: Other Suggestions!
My friend recommended creating a useCallbackIfMounted
custom hook that abstracts the problem away:
function useCallbackIfMounted(callback) {
const isMountedRef = useIsMounted();
return () => {
if (isMountedRef.current) {
callback();
}
}
}
function Button({callback}) {
const mountedCallback = useCallbackIfMounted(callback);
return <button onClick={mountedCallback}>Hallo</button>;
}
Using this would mean that fewer devs would have to figure out the ref hook bit. It doesn’t help for cases where you’re using a ref for something else, but is a nice solution to this specific problem.