A safe useEffect for refs
dvoytenko opened this issue · 0 comments
The pattern:
const ref = useRef();
useEffect(() => {
ref.current.addEventListener(...);
return () => ref.current.removeEventListener(...);
}, [...])
return (
<div>
{props.mode === 1 ? <Mode1 ref={ref}/> : <Mode2 ref={ref}/>}
</div>
);
In this pattern, it's hard to react safely to changes of ref.current
, e.g. when a node mapped to it is deleted/changes. The "right" way to do this is to ensure that the deps
array contains the same condition that affects ref
. E.g. [props.mode]
in the example above. However, it's not always obvious and easy to miss.
Some solutions are below.
/1/ Ask nicely for deps
to be correct and hope for the best
Hopefully an "exhaustive deps" linter would not remove the extra dep.
/2/ Ban changing of Ref mapping.
I.e. disallow the example above. This could be hard with forwardRef
.
/3/ Use state function instead of ref:
const [node, setNode] = useState();
useEffect(() => {...}, [node])
return <...><Mode1 ref={setNode}></...>
The negative: it forces the second rerender each time the ref changes.
/4/ Use a funky xEffectWithRef
version.
It'd manage the ref
value internally and could look something like this:
function useEffectWithRef(ref, effect, deps) {
const unsubscribe = useRef(null);
const prev = useRef(null);
useEffect(() => {
return () => doUnsubscribe(prev, unsubscribe);
}, deps || []);
useEffect(() => {
const {current} = ref;
if (current !== prev.current) {
doUnsubscribe(prev, unsubscribe);
prev.current = current;
if (current) {
unsubscribe.current = effect(current);
}
}
});
}
The positive: it doesn't cause rerender.
A negative: one effect is executed each time, but it will almost always do nothing.