Feature request: Consider supporting AbortSignal/AbortController for cleanup

[benjamingr]

Hey, I've recently worked on adding support for AbortController/AbortSignal in a bunch of Node.js APIs and some DOM ones and I think it would be really cool if React added support to the web platform's cancellation standard primitive.

Basically the ask is:

//before
useEffect(() => {
  const ac = new AbortController();
  (async () => {
    const data = await fetch('./api', { signal: ac.signal }).then(x => x.json());
    setData(data);
  })();
  return () => ac.abort();
});
//after
useEffect(async (signal) => {
  const data = await fetch('./api', { signal }).then(x => x.json());
  setData(data);
});

Or with events:

//before
useEffect(() => {
   const listener = (message) => setMessage(process(message));
   myApi.addEventListener("message", listener);
   return () => myApi.removeEventListener("message", listener);
});
//after
useEffect(async (signal) => {
  myApi.addEventListener("message", (message) => setMessage(process(message)), { signal });
});

I can bring in more examples. It's also possible to keep the API compatible with the existing API or create the AbortSignal lazily to minimize performance impact.

Apologies if I opened this in the incorrect place :)

[benjamingr]

(I am also happy to contribute code for this - though I seriously doubt that's the blocker)

[vkurchatkin]

It seems like something that can be trivially implemented as a separate package.

[benjamingr]

@vkurchatkin it's always possible to wrap all the React hooks in whatever wrapper we want I think? AbortSignal is the web-platform standard way to do cancellation - doing a "reverse wrapper" back to the standard way seems very backwards to me.

This is less about "this makes code easier for me personally to write" and more "this makes React behave like the web platform and Node in this regard".

[Ratstail91]

This is exactly what I need right now (what I actually need is some way to figure out why my component is rendering twice...)

[bengry]

This is exactly what I need right now (what I actually need is some way to figure out why my component is rendering twice...)

https://www.npmjs.com/package/@welldone-software/why-did-you-render

[lxsmnsyc]

AbortSignal is the web-platform standard way to do cancellation

The thing is though is that React also supports Node applications. The Abort Controller API is only supported for Node v15+, which could potentially break compatibility. I don't think the React team would add such a feature that would require them to shim it only for most of the users to not use it in the first place.

Cleanup functions is the simplest way they can offer.

And I would also agree that this kind of feature would be ideal as a userland hook rather than a built-in one.

[benjamingr]

The thing is though is that React also supports Node applications. The Abort Controller API is only supported for Node v15+,

We're (Node.js) happy to support (non-global) AbortController (as an import) if the React team believes that would make React adoption easier. The reason we added it in Node.js to begin with is having one standard way to do this sort of thing.

Cleanup functions is the simplest way they can offer.

Cleanup functions create non-standard long to write/read code that is very easy to get wrong in practice. Using standards instead of going against them has benefits (see the examples above). When React hooks were created AbortSignal wasn't a standard so not using it made sense.

This is similar for vNext versions of other libraries in userland (like RxJS) as well as core (we've covered most of the API surface with AbortSignal at this point.

And I would also agree that this kind of feature would be ideal as a userland hook rather than a built-in one.

I am... disinterested in userland hooks. The reason I work on things at the platform/library level is because I want these things to happen at the (drumroll...) platform/library level. The idea is to create less mental overhead and help users write correct code.

[benjamingr]

So I am fine with "we don't take feature requests from users" or "we don't have time to think about this now" or even "we're not sure" but some reply from maintainers would be nice.

Kind of frustrating it's easier to get feedback from browsers/platforms/language committees than from React. I don't want to ping individuals I know in the project (kind of rude IMO) and I want to follow the polite/kind process but I feel stuck.

[bvaughn]

We don't have time to think about this now.

The core team is very small (only 5 active engineers at the moment) and we're spread very thin.

[benjamingr]

@bvaughn ok thanks for the reply

[gaearon]

To give some extra context. I don't know the team's current consensus on this. We would probably need to have a few meetings just about this to reach some conclusion that's worthy of writing. Right now we're not able to prioritize this because we're in the process of wrapping up several years of existing work. So we didn't write a good response because we can't commit to diving dep into it at this moment.

We did consider this API during useEffect design. At that point, we made an explicit decision against it. There are a few reasons. useEffect by itself is an escape hatch. The goal is to eventually have a better high-level replacement for 90% cases. Your examples include data fetching. For data fetching, we are working on a completely separate set of primitives and APIs. There are some plans to consider abort signals there, though it's not set in stone. Because our approach relies more on caching, and if we're already fetching something, we might as well put it in the cache anyway. So abort signal is not necessarily the best strategy for us.

We know async effects are commonplace today, and React makes them a bit convoluted. That is intentional. We don't want to make that pattern too convenient because ultimately writing effects is hard. React has its own lifecycle that's separate from async code you spawn. So even with abort signals, it's too easy to introduce races. So we'd rather not make async effects more ergonomic and encourage people to write them. But we'd rather introduce ergonomic alternatives that don't suffer from races and also don't offer the same level of imperative control (so they wouldn't accept signals at the least at the user-facing API level).

It is not impossible that we'd revisit this, but that's what the consensus was last time and why we didn't do it. I'm sorry we were not responsive to this issue.

[gaearon]

Note: I meant we specifically decided against supporting async functions. Not necessarily against signal itself.

[benjamingr]

Thanks for the response.

We did consider this API during useEffect design. At that point, we made an explicit decision against it. There are a few reasons. useEffect by itself is an escape hatch. The goal is to eventually have a better high-level replacement for 90% cases. Your examples include data fetching.

That decision probably made sense - now (or soon) might be a good time to reconsider that decision since the web APIs themselves changed. Note that in the past AbortSignal was used only for fetch but right now it's supported in a lot more areas. See the other event example for one.

In Node.js all the APIs converged around cancellation with signals too. Data fetching isn't 90% of the problematic cases IMO - as more and more web APIs converge around the web cancellation primitive this will be easier and easier.

We know async effects are commonplace today, and React makes them a bit convoluted. That is intentional. We don't want to make that pattern too convenient because ultimately writing effects is hard.

I agree with that last statement wholeheartedly - writing effects is hard. Writing async code and writing side effects is hard - this is why it's so important we make the "correct" thing more obvious.

Right now in a large(ish) codebase I see a lot:

useEffect(async () => {
  // no cleanup
}, []);
useEffect(() => {
  (async () => {
    
  })(); 
}); // still no cleanup

I think having AbortSignal as an argument makes the fact cleanup should happen a lot more obvious (I'd hope) much like what promises did to APIs compared to callbacks.

[benjamingr]

As a second alternative - if react threw an error (at least in developer builds) if a function isn't returned from useEffect that would also be a nice change.

Though I think doing the standard web thing is probably(?) good

[gaearon]

if react threw an error (at least in developer builds) if a function isn't returned from useEffect that would also be a nice change.

Hmm. I thought we are throwing an error if what you return is neither undefined nor a function.

[benjamingr]

Yes, it's the undefined that's the problem for the cleanup case :)

I suspect it'll be even more so when concurrent mode becomes "the norm".

Though really - between "force everyone to return a function" and "take an AbortSignal argument like web APIs" I'd pick the latter as it makes for a much nicer API that "throws you to the pit of success" more often.

[gaearon]

What’s the idiomatic way to do cleanup for things that aren’t async, but just require disposal?

[gaearon]

I think conceptually it doesn’t feel like the right fit to me because (in .NET terms) effect cleanup is more like IDisposable than CancellationToken. But that’s just my hunch so I would need to see more comparisons to understand if there’s a real problem or I’m making it up.

[benjamingr]

@gaearon that's a good point, if you look carefully you'll see a bunch of the other APIs we have are more IDisposable than CancellationToken. The addEventListener example above is a good example.

Given the web converged on "disinterest semantics" for cancellation (cancellation is a request rather than an error) - the line between "cancellation" and "disposing a resource" is hair-thin (dispose a resource but cancel an action mostly).

However - for React in particular I think these sort of semantics make a ton of sense:

• Something like IDisposable typically means "you have to dispose it now when the user does .dispose. It typically comes with life-time management guarantees (like try using in the JS proposal case) which is the opposite of what frameworks would want (to control the lifecycle).
• Cancellation semantics like with AbortSignal mean "the producer upstream has expressed disinterest in this action" (data fetching, uploads, sockets listening, file actions, workers etc) - this gives React and the calling code a lot more flexibility.

---

_{Note: A logical next step is to loop in some more WHATWG involved folks.}

_{Note 2: I'm happy to loop in some of the .NET folks as well as some of the .NET folks who moved from C# to TS if that'd help but I'd like to keep the scope of the ask/discussion here small - aligning React with web platform APIs with regards to AbortSignal. Stuff like explicit resource management would be great to have but would not map well to React's use case where the framework needs to control the lifetime of resources rather than the calling code.}

[benjamingr]

What’s the idiomatic way to do cleanup for things that aren’t async, but just require disposal?

Here is one example above (I like it and go back to it because I know it well - but it's similar for other sync APIs going forward):

const ac = new AbortController();
const { signal } = ac;
const element = getElementSomeHow();
element.addEventListener('click', onClick, { signal });
element.addEventListener('mousedown', onMouseDown, { signal });
element.addEventListener('mouseup', onMouseUp, { signal });
// eventually we want to cleanup
ac.abort(); // removes all listeners

[jimmywarting]

I'm not a React developer, but i saw someone in my team using this kind of pattern

useEffect(() => {
  var fn = () => {...}
  addEventListener(name. fn)
  return () => removeEventListener(name, fn)
})

i immediately tough, oboy... it would have been nice if the syntax where just:

useEffect((signal) => {
  addEventListener(name. () => {}, { signal })
})

So i was going to suggest this to react as well unless @benjamingr haven't already done so.

[github-actions]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[github-actions]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!