fix use_async_effect cleanup not executed when dependencies change (#1328)

Author: Archmonger

docs/source/about/changelog.rst

@@ -25,7 +25,6 @@ Unreleased
 - :pull:`1308` - Event data now supports accessing properties via dot notation (ex. ``event.target.value``).
 - :pull:`1308` - Added ``reactpy.types.Event`` to provide type hints for the standard ``data`` function argument (for example ``def on_click(event: Event): ...``).
 - :pull:`1113` - Added ``asgi`` and ``jinja`` installation extras (for example ``pip install reactpy[asgi, jinja]``).
-- :pull:`1267` - Added ``shutdown_timeout`` parameter to the ``reactpy.use_async_effect`` hook.
 - :pull:`1113` - Added ``reactpy.executors.asgi.ReactPy`` that can be used to run ReactPy in standalone mode via ASGI.
 - :pull:`1269` - Added ``reactpy.executors.asgi.ReactPyCsr`` that can be used to run ReactPy in standalone mode via ASGI, but rendered entirely client-sided.
 - :pull:`1113` - Added ``reactpy.executors.asgi.ReactPyMiddleware`` that can be used to utilize ReactPy within any ASGI compatible framework.

pyproject.toml

@@ -210,11 +210,11 @@ build_client = ['hatch run "src/build_scripts/build_js_client.py" {args}']
 build_app = ['hatch run "src/build_scripts/build_js_app.py" {args}']
 publish_event_to_object = [
   'hatch run javascript:build_event_to_object',
-  'cd "src/js/packages/event-to-object" && bun publish --access public',
+  'cd "src/js/packages/event-to-object" && bunx npm publish --provenance --access public',
 ]
 publish_client = [
   'hatch run javascript:build_client',
-  'cd "src/js/packages/@reactpy/client" && bun publish --access public',
+  'cd "src/js/packages/@reactpy/client" && bunx npm publish --provenance --access public',
 ]
 
 #########################

src/reactpy/core/hooks.py

@@ -182,22 +182,19 @@ async def effect(stop: asyncio.Event) -> None:
 def use_async_effect(
     function: None = None,
     dependencies: Sequence[Any] | ellipsis | None = ...,
-    shutdown_timeout: float = 0.1,
 ) -> Callable[[_EffectApplyFunc], None]: ...
 
 
 @overload
 def use_async_effect(
     function: _AsyncEffectFunc,
     dependencies: Sequence[Any] | ellipsis | None = ...,
-    shutdown_timeout: float = 0.1,
 ) -> None: ...
 
 
 def use_async_effect(
     function: _AsyncEffectFunc | None = None,
     dependencies: Sequence[Any] | ellipsis | None = ...,
-    shutdown_timeout: float = 0.1,
 ) -> Callable[[_AsyncEffectFunc], None] | None:
     """
     A hook that manages an asynchronous side effect in a React-like component.
@@ -214,9 +211,6 @@ def use_async_effect(
             of any value in the given sequence changes (i.e. their :func:`id` is
             different). By default these are inferred based on local variables that are
             referenced by the given function.
-        shutdown_timeout:
-            The amount of time (in seconds) to wait for the effect to complete before
-            forcing a shutdown.
 
     Returns:
         If not function is provided, a decorator. Otherwise ``None``.
@@ -232,26 +226,37 @@ async def effect(stop: asyncio.Event) -> None:
             # always clean up the previous effect's resources
             run_effect_cleanup(cleanup_func)
 
-            # Execute the effect in a background task
+            # Execute the effect and store the clean-up function.
+            # We run this in a task so it can be cancelled if the stop signal
+            # is set before the effect completes.
             task = asyncio.create_task(func())
 
-            # Wait until we get the signal to stop this effect
-            await stop.wait()
+            # Wait for either the effect to complete or the stop signal
+            stop_task = asyncio.create_task(stop.wait())
+            done, _ = await asyncio.wait(
+                [task, stop_task],
+                return_when=asyncio.FIRST_COMPLETED,
+            )
 
-            # If renders are queued back-to-back, the effect might not have
-            # completed. So, we give the task a small amount of time to finish.
-            # If it manages to finish, we can obtain a clean-up function.
-            results, _ = await asyncio.wait([task], timeout=shutdown_timeout)
-            if results:
-                cleanup_func.current = results.pop().result()
+            # If the effect completed first, store the cleanup function
+            if task in done:
+                cleanup_func.current = task.result()
+                # Cancel the stop task since we don't need it anymore
+                stop_task.cancel()
+                with contextlib.suppress(asyncio.CancelledError):
+                    await stop_task
+                # Now wait for the stop signal to run cleanup
+                await stop.wait()
+            else:
+                # Stop signal came first - cancel the effect task
+                task.cancel()
+                with contextlib.suppress(asyncio.CancelledError):
+                    await task
 
             # Run the clean-up function when the effect is stopped,
             # if it hasn't been run already by a new effect
             run_effect_cleanup(cleanup_func)
 
-            # Cancel the task if it's still running
-            task.cancel()
-
         return memoize(lambda: hook.add_effect(effect))
 
     # Handle decorator usage

tests/test_core/test_hooks.py

@@ -476,6 +476,49 @@ def cleanup():
         assert cleanup_trigger_count.current == 1
 
 
+async def test_memoized_async_effect_cleanup_only_triggered_before_new_effect():
+    """Test that use_async_effect cleanup is triggered when dependencies change.
+
+    This is the async version of test_memoized_effect_cleanup_only_triggered_before_new_effect.
+    Regression test for https://github.com/reactive-python/reactpy/issues/1327
+    """
+    component_hook = HookCatcher()
+    set_state_callback = reactpy.Ref(None)
+    cleanup_trigger_count = reactpy.Ref(0)
+
+    first_value = 1
+    second_value = 2
+
+    @reactpy.component
+    @component_hook.capture
+    def ComponentWithEffect():
+        state, set_state_callback.current = reactpy.hooks.use_state(first_value)
+
+        @reactpy.hooks.use_async_effect(dependencies=[state])
+        async def effect():
+            def cleanup():
+                cleanup_trigger_count.current += 1
+
+            return cleanup
+
+        return reactpy.html.div()
+
+    async with Layout(ComponentWithEffect()) as layout:
+        await layout.render()
+
+        assert cleanup_trigger_count.current == 0
+
+        component_hook.latest.schedule_render()
+        await layout.render()
+
+        assert cleanup_trigger_count.current == 0
+
+        set_state_callback.current(second_value)
+        await layout.render()
+
+        assert cleanup_trigger_count.current == 1
+
+
 async def test_use_async_effect():
     effect_ran = asyncio.Event()