Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Hooks] Abortable useEffect #137

Closed
sag1v opened this issue Jan 6, 2020 · 7 comments
Closed

[Hooks] Abortable useEffect #137

sag1v opened this issue Jan 6, 2020 · 7 comments

Comments

@sag1v
Copy link

sag1v commented Jan 6, 2020

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".

@sag1v sag1v changed the title Abortable useEffect [Hooks] Abortable useEffect Jan 6, 2020
@courtyenn
Copy link

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.

@sag1v
Copy link
Author

sag1v commented Apr 7, 2020

@courtneynguyen

Shouldn't status and effect 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.

@atticoos
Copy link

atticoos commented Apr 7, 2020

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

@ghasemikasra39
Copy link

nice idea

@Flufd
Copy link

Flufd commented Aug 4, 2020

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.

@gaearon
Copy link
Member

gaearon commented Aug 24, 2021

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.

@sag1v
Copy link
Author

sag1v commented Aug 25, 2021

RFC is now opened in #202. We should continue further discussion in there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants