reactpy
reactpy copied to clipboard
reactive-python
Better Async Effect Cleanup
Current Situation
Presently, the "cleanup" behavior for async effects is that they are cancelled before the next effect takes place or the component is unmounted. While this behavior may be desirable in many cases, it does not give the user the ability to gracefully deal with cleanup logic in the way one can with a sync event.
For example, consider a long running async effect:
@use_effect
async def long_running_task():
while True:
await something()
await something_else()
The problem with this current behavior is that handling the cancellation is messy. You could do the following:
@use_effect
async def long_running_task():
try:
while True:
await something()
await something_else()
finally:
... # cleanup logic here
However, this may lead to some confusing behavior since it's not possible to know whether something() or something_else() will receive the cancellation. You might have to do a lot of extra work to figure out what state your program was left in after the cancellation.
Proposed Actions
Thankfully the above only impacts async effects. With a synchronous event, you would be able to handle async task cleanup using something like the following:
@use_effect
def start_long_running_task():
interupt = Event()
asyncio.create_task(long_running_task(interupt))
return cleanup.set
async def long_running_task(interrupt):
while not interupt.is_set():
await something()
await something_else()
... # cleanup logic here
Given this, our proposed solution to the problem is to allow async effects to accept a similar interupt asyncio.Event:
@use_effect
async def long_running_task(interrupt):
while not interupt.is_set():
await something()
await something_else()
... # cleanup logic here
@Archmonger I'm realizing that replicating the prior cancellation behavior is quite tricky to do with the interupt event - it would almost always be better to just write a sync event in that case. Perhaps we should preserve the original behavior where we cancel the effect if it does not not accept any arguments?
That was my original justification for the async_schedule parameter (name TBD). It simplifies awkward situations like that.
I think it's less ambiguous than just assuming cancellation on argless functions. With this, it might cover enough to where we don't need the interrupt parameter.
- cancel-oldest # The current behavior
- queue-all # Best for something that needs to run in the background every time, no matter what
- no-requeue # Best for long polling functions
The interrupt event is definitely necessary since that's what allows the user to implement cleanup logic after the event has been set. I'm also realizing that effects must run one after the other - that is, we must wait for the the prior effect to complete before kicking off the next one. Otherwise you could end up in a weird situation where the cleanup logic for the last effect might be running while the setup logic for the next one does.
Otherwise you could end up in a weird situation where the cleanup logic for the last effect might be running while the setup logic for the next one does.
I believe race conditions are expected with asyncio, and should be a problem for the user to solve.
I'm not a huge fan of an implict cancellation policy on argless functions...
Maybe we have a cancellable parameter in the use_effect call?
@use_effect(cancellable=True)
async def do_something(interrupt):
...
But then that kinda makes interrupt useless in this context anyways... Maybe we really do have to just support argless async effects. Either that or support async_schedule, which isn't ideal either.
I believe race conditions are expected with asyncio, and should be a problem for the user to solve.
The problem is that, with the proposed API, the user doesn't actually have enough control to manage this themselves. We would need to pass in a reference to the prior task so they could await it if they wanted:
@use_effect
def my_effect(prior: Task | None, interupt: Event):
if prior is not None:
await prior
# do stuff
await interupt.wait()
# clean up
This may actually be the interface that provides the necessary level of control. With a reference to the prior task you could cancel it if you wanted.
If we think we might need even more options in the future, we can consider a dataclass called something like EffectHandler containing both of these args.
Cancellation shielding may be the answer to this interface question. In short, the current behavior would be maintained. That is, async effects are cancelled before the next one is executed. Users can then leverage cancellation shielding to protect important tasks from cancellation if necessary.
With that said, we should, but currently don't, wait for the completion of old cancelled effects.
Fundamentally, work being done by an effect as a result of an old render should be stopped. The main issue for users is doing so gracefully since dealing with the unpredictable timing of cancellations is challenging. Thus, the remedy may simply be good documentation that explains this.
I'm in favor of deprecating async effects. It's fairly easy to manage an async task within a sync effect.
Any async design we create is going to be over complicated. And even worse, it will be a syntactical mismatch between ReactJS and ReactPy.
I think we wait for when/if ReactJS ever supports async effects to re-add support.