[Hooks] Abortable useEffect
sag1v opened this issue · 7 comments
When ever we use effect with a state update inside an async callback (or after an await
) we probably want to check that we are not trying to update an unmounted component or worse, we are in a race condition and updating the state with the wrong data.
With hooks, this is fairly simple to achieve using a boolean and a closure:
useEffect(() => {
let current = true;
asyncOp(arg).then(res => {
current && setState(res);
});
// cleanup
return () => current = false
}, [arg]);
But doing that in each effect, every time we are dealing with a state update in an async operation can become tedious.
I played around and created an "abortable" effect, this effect is just a wrapper over useEffect
and its providing a status
indicator so the consumer can check if the effect was "cleaned":
function useAbortableEffect(effect, dependencies) {
const status = {}; // mutable status object
useEffect(() => {
status.aborted = false;
// pass the mutuable object to the effect callback
// store the returned value for cleanup
const cleanUpFn = effect(status);
return () => {
// mutate the object to signal the consumer this effect is cleaning up
status.aborted = true;
if (typeof cleanUpFn === "function") {
// run the cleanup function
cleanUpFn();
}
};
// not the best way to pass dependencies, but what other options do we have
}, [...dependencies]);
}
And the usage:
useAbortableEffect(status => {
asyncOp(arg).then(res => {
!status.aborted && setState(res);
});
}, [arg]);
I wish that useEffect
would provide this information out of the box instead of us writing it in "user land".
Shouldn't status
and effect
be included in the dependencies array? Also, since dependencies
is not being used inside the side effect itself, it shouldn't be included.
Shouldn't
status
andeffect
be included in the dependencies array?
Well status
is mutable so i'm not sure how effective it will be to include it as dependency, i think its the same as a ref
.
As for effect
, again this won't make much sense as it will be a new function reference on each cycle because most if not all the time we will pass effect
as an inline function.
Also, since
dependencies
is not being used inside the side effect itself, it shouldn't be included.
We are basically wrapping useEffect
here so we should keep the same API, if we won't use dependencies
then the consumers won't be able to sync it with dependencies like they can with the builtin useEffect
.
You probably wouldn't want status
changing to rerun the effect, since the job will run a second time. This is a ref
, where the callback that becomes executed can understand if it has since become aborted
nice idea
I wrote this small library that does basically this a while ago. https://github.com/Flufd/use-current-effect
The implementation is almost exactly the same but the hook passes a boolean returning function to check the clean-up state instead of { status: boolean }
I do think it would work nicely in React by default though.
Hi, thanks for your suggestion. RFCs should be submitted as pull requests, not issues. I will close this issue but feel free to resubmit in the PR format.