From af162be9960751c2dda9be7e7b97d367597df111 Mon Sep 17 00:00:00 2001 From: Ryan Morshead Date: Sat, 9 Dec 2023 11:38:20 -0700 Subject: [PATCH 1/4] better async effects --- docs/source/reference/hooks-api.rst | 108 +++++++++--- .../reactpy/reactpy/core/_life_cycle_hook.py | 9 +- src/py/reactpy/reactpy/core/hooks.py | 156 ++++++++++++------ src/py/reactpy/tests/test_core/test_hooks.py | 119 ++++++++++--- 4 files changed, 297 insertions(+), 95 deletions(-) diff --git a/docs/source/reference/hooks-api.rst b/docs/source/reference/hooks-api.rst index ca8123e85..7e3d0d17d 100644 --- a/docs/source/reference/hooks-api.rst +++ b/docs/source/reference/hooks-api.rst @@ -89,7 +89,9 @@ Use Effect .. code-block:: - use_effect(did_render) + @use_effect + def did_render(): + ... # imperative or state mutating logic The ``use_effect`` hook accepts a function which may be imperative, or mutate state. The function will be called immediately after the layout has fully updated. @@ -111,23 +113,46 @@ Cleaning Up Effects ................... If the effect you wish to enact creates resources, you'll probably need to clean them -up. In such cases you may simply return a function that addresses this from the -``did_render`` function which created the resource. Consider the case of opening and -then closing a connection: +up. In this case you can write a generator function that will ``yield`` until your +cleanup logic needs to run. Similarly to a +`context manager `__, you'll also need to +use a ``try/finally`` block to ensure that the cleanup logic always runs: .. code-block:: + @use_effect def establish_connection(): - connection = open_connection() - return lambda: close_connection(connection) + conn = open_connection() + try: + # do something with the connection + yield + finally: + conn.close() - use_effect(establish_connection) +.. warning:: + + If you never ``yield`` control back to ReactPy, then the component will never + re-render and the effect will never be cleaned up. This is a common mistake when + using ``use_effect`` for the first time. -The clean-up function will be run before the component is unmounted or, before the next -effect is triggered when the component re-renders. You can +The clean-up logic inside the ``finally`` block will be run before the component is +unmounted or, before the next effect is triggered when the component re-renders. You can :ref:`conditionally fire events ` to avoid triggering them each time a component renders. +Alternatively to the generator style of cleanup, you can return a cleanup function from +your effect function. As with the generator style, the cleanup function will be run +before the component is unmounted or before the next effect is triggered when the +component re-renders: + +.. code-block:: + + @use_effect + def establish_connection(): + conn = open_connection() + # do something with the connection + return conn.close + Conditional Effects ................... @@ -141,40 +166,77 @@ example, imagine that we had an effect that connected to a ``url`` state variabl url, set_url = use_state("https://example.com") + @use_effect def establish_connection(): connection = open_connection(url) return lambda: close_connection(connection) - use_effect(establish_connection) - Here, a new connection will be established whenever a new ``url`` is set. +.. warning:: + + A component will be unable to render until all its outstanding effects have been + cleaned up. As such, it's best to keep cleanup logic as simple as possible and/or + to impose a time limit. + Async Effects ............. A behavior unique to ReactPy's implementation of ``use_effect`` is that it natively -supports ``async`` functions: +supports ``async`` effects. Async effect functions may either be an async function +or an async generator. If your effect doesn't need to do any cleanup, then you can +simply write an async function. .. code-block:: - async def non_blocking_effect(): - resource = await do_something_asynchronously() - return lambda: blocking_close(resource) + @use_effect + async def my_async_effect(): + await do_something() + +However, if you need to do any cleanup, then you'll need to write an async generator +instead. The generator, as in :ref:`sync effects ` should run the +effect logic in a ``try`` block, ``yield`` control back to ReactPy, and then run the +cleanup logic in a ``finally`` block: - use_effect(non_blocking_effect) +.. code-block:: + @use_effect + async def my_async_effect(): + try: + await effect_logic() + yield + finally: + await cleanup_logic() -There are **three important subtleties** to note about using asynchronous effects: +Unlike sync effects, when a component is re-rendered or unmounted the effect will be +cancelled if it is still running. This will typically happen for long-lived effects. +One example might be an effect that opens a connection and then responds to messages +for the lifetime of the connection: + +.. code-block:: -1. The cleanup function must be a normal synchronous function. + @use_effect + async def my_async_effect(): + conn = await open_connection() + try: + while True: + msg = await conn.recv() + await handle_message(msg) + finally: + await conn.close() -2. Asynchronous effects which do not complete before the next effect is created - following a re-render will be cancelled. This means an - :class:`~asyncio.CancelledError` will be raised somewhere in the body of the effect. +.. warning:: + + Because an effect can be cancelled at any time, it's possible that the cleanup logic + will run before all of the effect logic has finished. For example, in the code + above, we exclude ``conn = await open_connection()`` from the ``try`` block because + if the effect is cancelled before the connection is opened, then we don't need to + close it. + +.. note:: -3. An asynchronous effect may occur any time after the update which added this effect - and before the next effect following a subsequent update. + We don't need a yield statement here because the effect only ends when it's cancelled. Manual Effect Conditions diff --git a/src/py/reactpy/reactpy/core/_life_cycle_hook.py b/src/py/reactpy/reactpy/core/_life_cycle_hook.py index ea5e6d634..7812456e8 100644 --- a/src/py/reactpy/reactpy/core/_life_cycle_hook.py +++ b/src/py/reactpy/reactpy/core/_life_cycle_hook.py @@ -133,7 +133,7 @@ def __init__( self._current_state_index = 0 self._state: tuple[Any, ...] = () self._effect_funcs: list[EffectFunc] = [] - self._effect_tasks: list[Task[None]] = [] + self._effect_tasks: set[Task[None]] = set() self._effect_stops: list[Event] = [] self._render_access = Semaphore(1) # ensure only one render at a time @@ -212,7 +212,12 @@ async def affect_layout_did_render(self) -> None: """The layout completed a render""" stop = Event() self._effect_stops.append(stop) - self._effect_tasks.extend(create_task(e(stop)) for e in self._effect_funcs) + for effect_func in self._effect_funcs: + effect_task = create_task(effect_func(stop)) + # potential memory leak if the task doesn't remove itself when complete + effect_task.add_done_callback(lambda t: self._effect_tasks.remove(t)) + self._effect_tasks.add(effect_task) + self._effect_tasks.update() self._effect_funcs.clear() async def affect_component_will_unmount(self) -> None: diff --git a/src/py/reactpy/reactpy/core/hooks.py b/src/py/reactpy/reactpy/core/hooks.py index 4513dadef..87090a3e2 100644 --- a/src/py/reactpy/reactpy/core/hooks.py +++ b/src/py/reactpy/reactpy/core/hooks.py @@ -1,7 +1,14 @@ from __future__ import annotations -import asyncio -from collections.abc import Coroutine, Sequence +from asyncio import CancelledError, Event, Task, create_task +from collections.abc import ( + AsyncGenerator, + AsyncIterator, + Coroutine, + Generator, + Sequence, +) +from inspect import isasyncgenfunction, iscoroutinefunction, isgeneratorfunction from logging import getLogger from types import FunctionType from typing import ( @@ -14,8 +21,7 @@ cast, overload, ) - -from typing_extensions import TypeAlias +from warnings import warn from reactpy.config import REACTPY_DEBUG_MODE from reactpy.core._life_cycle_hook import current_hook @@ -93,34 +99,37 @@ def dispatch(new: _Type | Callable[[_Type], _Type]) -> None: self.dispatch = dispatch -_EffectCleanFunc: TypeAlias = "Callable[[], None]" -_SyncEffectFunc: TypeAlias = "Callable[[], _EffectCleanFunc | None]" -_AsyncEffectFunc: TypeAlias = ( - "Callable[[], Coroutine[None, None, _EffectCleanFunc | None]]" -) -_EffectApplyFunc: TypeAlias = "_SyncEffectFunc | _AsyncEffectFunc" +_SyncGeneratorEffect = Callable[[], Generator[None, None, None]] +_SyncFunctionEffect = Callable[[], Callable[[], None] | None] +_SyncEffect = _SyncGeneratorEffect | _SyncFunctionEffect + +_AsyncGeneratorEffect = Callable[[], AsyncGenerator[None, None]] +_AsyncFunctionEffect = Callable[[], Coroutine[None, None, Callable[[], None] | None]] +_AsyncEffect = _AsyncGeneratorEffect | _AsyncFunctionEffect + +_Effect = _SyncEffect | _AsyncEffect @overload def use_effect( function: None = None, dependencies: Sequence[Any] | ellipsis | None = ..., -) -> Callable[[_EffectApplyFunc], None]: +) -> Callable[[_Effect], None]: ... @overload def use_effect( - function: _EffectApplyFunc, + function: _Effect, dependencies: Sequence[Any] | ellipsis | None = ..., ) -> None: ... def use_effect( - function: _EffectApplyFunc | None = None, + function: _Effect | None = None, dependencies: Sequence[Any] | ellipsis | None = ..., -) -> Callable[[_EffectApplyFunc], None] | None: +) -> Callable[[_Effect], None] | None: """See the full :ref:`Use Effect` docs for details Parameters: @@ -130,48 +139,50 @@ def use_effect( Dependencies for the effect. The effect will only trigger if the identity 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. + referenced by the given function. If ``None``, then the effect runs on every + render. Returns: - If not function is provided, a decorator. Otherwise ``None``. + If no function is provided, a decorator. Otherwise ``None``. """ hook = current_hook() - dependencies = _try_to_infer_closure_values(function, dependencies) memoize = use_memo(dependencies=dependencies) - last_clean_callback: Ref[_EffectCleanFunc | None] = use_ref(None) - - def add_effect(function: _EffectApplyFunc) -> None: - if not asyncio.iscoroutinefunction(function): - sync_function = cast(_SyncEffectFunc, function) - else: - async_function = cast(_AsyncEffectFunc, function) - - def sync_function() -> _EffectCleanFunc | None: - task = asyncio.create_task(async_function()) - - def clean_future() -> None: - if not task.cancel(): - try: - clean = task.result() - except asyncio.CancelledError: - pass - else: - if clean is not None: - clean() - - return clean_future - - async def effect(stop: asyncio.Event) -> None: - if last_clean_callback.current is not None: - last_clean_callback.current() - last_clean_callback.current = None - clean = last_clean_callback.current = sync_function() + last_effect: Ref[tuple[Event, Task[None]] | None] = use_ref(None) + + def add_effect(function: _Effect) -> None: + effect_func = _cast_async_effect(function) + + async def run_effect(stop: Event) -> None: + # stop the last effect (if any) + if last_effect.current is not None: + last_stop, last_task = last_effect.current + last_stop.set() + try: + await last_task + except Exception: + logger.exception("Error in effect") + except CancelledError: + pass + # create the effect + effect_gen = effect_func() + # start running the effect + gen_coro = cast(Coroutine[None, None, None], effect_gen.asend(None)) + effect_task = create_task(gen_coro) + last_effect.current = stop, effect_task + # wait for re-render or unmount await stop.wait() - if clean is not None: - clean() - - return memoize(lambda: hook.add_effect(effect)) + # signal effect to stop (no-op if already complete) + effect_task.cancel() + # wait for effect to halt + try: + await effect_task + except CancelledError: + pass + # wait for effect cleanup + await effect_gen.aclose() + + return memoize(lambda: hook.add_effect(run_effect)) if function is not None: add_effect(function) @@ -180,6 +191,53 @@ async def effect(stop: asyncio.Event) -> None: return add_effect +def _cast_async_effect(function: Callable[..., Any]) -> _AsyncGeneratorEffect: + if isasyncgenfunction(function): + async_generator_effect = function + elif iscoroutinefunction(function): + async_function_effect = cast(_AsyncFunctionEffect, function) + + async def async_generator_effect() -> AsyncIterator[None]: + cleanup = await async_function_effect() + try: + yield + finally: + if cleanup is not None: + warn( + f"Async effect {async_function_effect} returned a cleanup " + f"function - use an async generator instead by yielding inside " + "a try/finally block. This will be an error in a future " + "version of ReactPy.", + DeprecationWarning, + stacklevel=2, + ) + cleanup() + + elif isgeneratorfunction(function): + sync_generator_effect = cast(_SyncGeneratorEffect, function) + + async def async_generator_effect() -> AsyncIterator[None]: + gen = sync_generator_effect() + gen.send(None) + try: + yield + finally: + gen.close() + + else: + sync_function_effect = cast(_SyncFunctionEffect, function) + + async def async_generator_effect() -> AsyncIterator[None]: + cleanup = sync_function_effect() + try: + yield + finally: + if cleanup is not None: + cleanup() + + return async_generator_effect + + def use_debug_value( message: Any | Callable[[], Any], dependencies: Sequence[Any] | ellipsis | None = ..., diff --git a/src/py/reactpy/tests/test_core/test_hooks.py b/src/py/reactpy/tests/test_core/test_hooks.py index fa6acafd1..3e29eec8c 100644 --- a/src/py/reactpy/tests/test_core/test_hooks.py +++ b/src/py/reactpy/tests/test_core/test_hooks.py @@ -1,4 +1,5 @@ -import asyncio +from asyncio import CancelledError, create_task, sleep, wait_for +from asyncio import Event as EventNoTimeout import pytest @@ -11,6 +12,7 @@ from reactpy.testing import DisplayFixture, HookCatcher, assert_reactpy_did_log, poll from reactpy.testing.logs import assert_reactpy_did_not_log from reactpy.utils import Ref +from tests.tooling.aio import Event from tests.tooling.common import DEFAULT_TYPE_DELAY, update_message @@ -355,8 +357,10 @@ def cleanup(): component_hook.latest.schedule_render() await layout.render() - assert cleanup_triggered.current - assert cleanup_triggered_before_next_effect.current + assert poll( + lambda: cleanup_triggered.current + and cleanup_triggered_before_next_effect.current + ).until_is(True) async def test_use_effect_cleanup_occurs_on_will_unmount(): @@ -468,16 +472,16 @@ def cleanup(): component_hook.latest.schedule_render() await layout.render() - assert cleanup_trigger_count.current == 0 + assert poll(lambda: cleanup_trigger_count.current).until_equals(0) set_state_callback.current(second_value) await layout.render() - assert cleanup_trigger_count.current == 1 + assert poll(lambda: cleanup_trigger_count.current).until_equals(1) async def test_use_async_effect(): - effect_ran = asyncio.Event() + effect_ran = Event() @reactpy.component def ComponentWithAsyncEffect(): @@ -489,13 +493,13 @@ async def effect(): async with reactpy.Layout(ComponentWithAsyncEffect()) as layout: await layout.render() - await asyncio.wait_for(effect_ran.wait(), 1) + await effect_ran.wait() async def test_use_async_effect_cleanup(): component_hook = HookCatcher() - effect_ran = asyncio.Event() - cleanup_ran = asyncio.Event() + effect_ran = Event() + cleanup_ran = Event() @reactpy.component @component_hook.capture @@ -510,19 +514,24 @@ async def effect(): async with reactpy.Layout(ComponentWithAsyncEffect()) as layout: await layout.render() + await effect_ran.wait() + effect_ran.clear() component_hook.latest.schedule_render() await layout.render() + await cleanup_ran.wait() + cleanup_ran.clear() + await effect_ran.wait() - await asyncio.wait_for(cleanup_ran.wait(), 1) + await cleanup_ran.wait() -async def test_use_async_effect_cancel(caplog): +async def test_use_async_effect_cancel(): component_hook = HookCatcher() - effect_ran = asyncio.Event() - effect_was_cancelled = asyncio.Event() + effect_ran = Event() + effect_was_cancelled = Event() - event_that_never_occurs = asyncio.Event() + event_that_never_occurs = EventNoTimeout() @reactpy.component @component_hook.capture @@ -532,7 +541,7 @@ async def effect(): effect_ran.set() try: await event_that_never_occurs.wait() - except asyncio.CancelledError: + except CancelledError: effect_was_cancelled.set() raise @@ -546,7 +555,7 @@ async def effect(): await layout.render() - await asyncio.wait_for(effect_was_cancelled.wait(), 1) + await effect_was_cancelled.wait() # So I know we said the event never occurs but... to ensure the effect's future is # cancelled before the test is cleaned up we need to set the event. This is because @@ -848,7 +857,7 @@ def bad_callback(): async def test_use_effect_automatically_infers_closure_values(): set_count = reactpy.Ref() - did_effect = asyncio.Event() + did_effect = Event() @reactpy.component def CounterWithEffect(): @@ -876,7 +885,7 @@ def some_effect_that_uses_count(): async def test_use_memo_automatically_infers_closure_values(): set_count = reactpy.Ref() - did_memo = asyncio.Event() + did_memo = Event() @reactpy.component def CounterWithEffect(): @@ -1038,7 +1047,7 @@ def SetStateDuringRender(): await poll(lambda: render_count.current).until_equals(2) # give an opportunity for a render to happen if it were to. - await asyncio.sleep(0.1) + await sleep(0.1) # however, we don't expect any more renders assert render_count.current == 2 @@ -1197,8 +1206,8 @@ def SomeComponent(): await layout.render() assert render_count.current == 1 set_state.current(get_value()) - with pytest.raises(asyncio.TimeoutError): - await asyncio.wait_for(layout.render(), timeout=0.1) + with pytest.raises(TimeoutError): + await wait_for(layout.render(), timeout=0.1) @pytest.mark.parametrize("get_value", STRICT_EQUALITY_VALUE_CONSTRUCTORS) @@ -1263,3 +1272,71 @@ def bad_cleanup(): await layout.render() component_hook.latest.schedule_render() await layout.render() # no error + + +async def test_slow_async_generator_effect_is_cancelled_and_cleaned_up(): + hook_catcher = HookCatcher() + + never = Event() + did_run = Event() + did_cancel = Event() + did_cleanup = Event() + + @reactpy.component + @hook_catcher.capture + def some_component(): + @use_effect(dependencies=None) + async def slow_effect(): + try: + did_run.set() + await never.wait() + yield + except CancelledError: + did_cancel.set() + raise + finally: + # should be allowed to await in finally + await sleep(0) + did_cleanup.set() + + async with reactpy.Layout(some_component()) as layout: + await layout.render() + + await did_run.wait() + + hook_catcher.latest.schedule_render() + render_task = create_task(layout.render()) + + await did_cancel.wait() + await did_cleanup.wait() + + await render_task + + +async def test_sync_generator_style_effect(): + hook_catcher = HookCatcher() + + did_run = Event() + did_cleanup = Event() + + @reactpy.component + @hook_catcher.capture + def some_component(): + @use_effect(dependencies=None) + def sync_generator_effect(): + try: + did_run.set() + yield + finally: + did_cleanup.set() + + async with reactpy.Layout(some_component()) as layout: + await layout.render() + + await did_run.wait() + + hook_catcher.latest.schedule_render() + render_task = create_task(layout.render()) + + await did_cleanup.wait() + await render_task From 773e8d4bb8fff394327b32692941c22eff07c3bd Mon Sep 17 00:00:00 2001 From: Ryan Morshead Date: Sat, 9 Dec 2023 11:49:48 -0700 Subject: [PATCH 2/4] add changelog entry --- docs/source/about/changelog.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/source/about/changelog.rst b/docs/source/about/changelog.rst index d874a470f..128e76b8b 100644 --- a/docs/source/about/changelog.rst +++ b/docs/source/about/changelog.rst @@ -34,6 +34,11 @@ Unreleased experimental feature by setting `REACTPY_ASYNC_RENDERING=true`. This should improve the overall responsiveness of your app, particularly when handling larger renders that would otherwise block faster renders from being processed. +- :pull:`1169` - Sync and async effects can now be defined as generators which ``yield`` + control back to ReactPy until they need to be cleaned up. Previously async effects + were only allowed to have synchronous cleanup cleanup. This now allows them to be + cleaned up asynchronously. See the :ref:`updated documentation ` for more + information. v1.0.2 ------ From bd5bb0ccfc8b40ae03f6da0b177383399e620741 Mon Sep 17 00:00:00 2001 From: Ryan Morshead Date: Sat, 9 Dec 2023 12:00:57 -0700 Subject: [PATCH 3/4] 3.9 compat annotations --- src/py/reactpy/reactpy/core/hooks.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/py/reactpy/reactpy/core/hooks.py b/src/py/reactpy/reactpy/core/hooks.py index 87090a3e2..4bddbe06f 100644 --- a/src/py/reactpy/reactpy/core/hooks.py +++ b/src/py/reactpy/reactpy/core/hooks.py @@ -16,8 +16,10 @@ Any, Callable, Generic, + Optional, Protocol, TypeVar, + Union, cast, overload, ) @@ -100,14 +102,14 @@ def dispatch(new: _Type | Callable[[_Type], _Type]) -> None: _SyncGeneratorEffect = Callable[[], Generator[None, None, None]] -_SyncFunctionEffect = Callable[[], Callable[[], None] | None] -_SyncEffect = _SyncGeneratorEffect | _SyncFunctionEffect +_SyncFunctionEffect = Callable[[], Optional[Callable[[], None]]] +_SyncEffect = Union[_SyncGeneratorEffect, _SyncFunctionEffect] _AsyncGeneratorEffect = Callable[[], AsyncGenerator[None, None]] -_AsyncFunctionEffect = Callable[[], Coroutine[None, None, Callable[[], None] | None]] -_AsyncEffect = _AsyncGeneratorEffect | _AsyncFunctionEffect +_AsyncFunctionEffect = Callable[[], Coroutine[None, None, Optional[Callable[[], None]]]] +_AsyncEffect = Union[_AsyncGeneratorEffect, _AsyncFunctionEffect] -_Effect = _SyncEffect | _AsyncEffect +_Effect = Union[_SyncEffect, _AsyncEffect] @overload From 5efe2f60f223702621e9c9aad7ec99eb50a5b484 Mon Sep 17 00:00:00 2001 From: Ryan Morshead Date: Sat, 9 Dec 2023 16:37:15 -0700 Subject: [PATCH 4/4] fix strict equality tests --- src/py/reactpy/tests/test_core/test_hooks.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/py/reactpy/tests/test_core/test_hooks.py b/src/py/reactpy/tests/test_core/test_hooks.py index 3e29eec8c..d95e8d4c1 100644 --- a/src/py/reactpy/tests/test_core/test_hooks.py +++ b/src/py/reactpy/tests/test_core/test_hooks.py @@ -1,4 +1,4 @@ -from asyncio import CancelledError, create_task, sleep, wait_for +from asyncio import CancelledError, TimeoutError, create_task, sleep, wait_for from asyncio import Event as EventNoTimeout import pytest @@ -1213,7 +1213,7 @@ def SomeComponent(): @pytest.mark.parametrize("get_value", STRICT_EQUALITY_VALUE_CONSTRUCTORS) async def test_use_effect_compares_with_strict_equality(get_value): effect_count = reactpy.Ref(0) - value = reactpy.Ref("string") + value = reactpy.Ref(get_value()) hook = HookCatcher() @reactpy.component @@ -1226,7 +1226,7 @@ def incr_effect_count(): async with reactpy.Layout(SomeComponent()) as layout: await layout.render() assert effect_count.current == 1 - value.current = "string" # new string instance but same value + value.current = get_value() hook.latest.schedule_render() await layout.render() # effect does not trigger