diff --git a/CHANGELOG.md b/CHANGELOG.md index a240fd1149..059e0d4f8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,7 +49,7 @@ * Fix `flet build` not reinstalling the app's packages after `requirements.txt` is edited or `build/site-packages` is deleted ([#6994](https://github.com/flet-dev/flet/issues/6994), [#6996](https://github.com/flet-dev/flet/pull/6996)) by @ndonkoHenri. * Fix `flet test` failing when the app under test starts slowly, as on the first run after a build: the test now waits until the app finishes initializing its first page ([#6993](https://github.com/flet-dev/flet/issues/6993), [#6995](https://github.com/flet-dev/flet/pull/6995)) by @ndonkoHenri. -* Fix `use_effect()` losing setup-returned cleanup callbacks after a component re-renders, so they run before dependency-triggered setup and on unmount; cancel pending async setup tasks on unmount ([#6963](https://github.com/flet-dev/flet/issues/6963), [#6983](https://github.com/flet-dev/flet/pull/6983)) by @mangod12. +* Fix `use_effect()` cleanups being skipped: a cleanup returned by the setup was lost once the component re-rendered, only one ran when `cleanup=` was passed too, and `async` cleanups never ran when a dependency change re-ran the effect. A pending `async` setup is also cancelled on unmount ([#6963](https://github.com/flet-dev/flet/issues/6963), [#6983](https://github.com/flet-dev/flet/pull/6983), [#7005](https://github.com/flet-dev/flet/pull/7005)) by @mangod12. * iOS apps built with `flet build ipa` no longer fail App Store Connect validation with ITMS-90683 (missing `NSPhotoLibraryUsageDescription` and `NSCameraUsageDescription` purpose strings): the `file_picker` plugin is updated to 13.x, which picks photos with the system photo picker and no longer links DKImagePickerController's camera and photo-library code. `device_info_plus`, `package_info_plus` and `share_plus` move to their next major versions with it (`win32` 6). `FilePicker.save_file()` on desktop now creates the chosen file (empty when no `src_bytes` are given) before returning its path by @FeodorFitsner. * Fix `Tester.find_by_key()` not finding `Tab` and `PopupMenuItem` controls by their keys ([#6965](https://github.com/flet-dev/flet/issues/6965), [#6976](https://github.com/flet-dev/flet/pull/6976)) by @RezaShamsipour. * Fix `GestureDetector.on_tap_move` not firing, while preserving tap accessibility and updates to `allowed_devices` ([#6961](https://github.com/flet-dev/flet/issues/6961), [#6982](https://github.com/flet-dev/flet/pull/6982)) by @inchang-ing. diff --git a/sdk/python/packages/flet/src/flet/components/component.py b/sdk/python/packages/flet/src/flet/components/component.py index 62b4917dd1..9786095bb7 100644 --- a/sdk/python/packages/flet/src/flet/components/component.py +++ b/sdk/python/packages/flet/src/flet/components/component.py @@ -341,7 +341,7 @@ def _run_render_effects(self): or hook.deps != hook.prev_deps ) if deps_changed: - if callable(hook.cleanup) or callable(hook._returned_cleanup): + if hook.has_cleanup(): self._schedule_effect(hook, is_cleanup=True) self._schedule_effect(hook, is_cleanup=False) diff --git a/sdk/python/packages/flet/src/flet/components/hooks/use_effect.py b/sdk/python/packages/flet/src/flet/components/hooks/use_effect.py index fb88f6561f..4e1a7b49cf 100644 --- a/sdk/python/packages/flet/src/flet/components/hooks/use_effect.py +++ b/sdk/python/packages/flet/src/flet/components/hooks/use_effect.py @@ -1,6 +1,6 @@ import asyncio from collections.abc import Awaitable, Callable, Sequence -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import Any from flet.components.hooks.hook import Hook @@ -21,30 +21,102 @@ class EffectHook(Hook): setup: Callable[[], Any | Awaitable[Any]] cleanup: Callable[[], Any | Awaitable[Any]] | None = None _returned_cleanup: Callable[[], Any | Awaitable[Any]] | None = None + """The cleanup the last non-`async` setup run returned, if any.""" + deps: list[Any] | None = None prev_deps: list[Any] | None = None - # runtime - _setup_task: asyncio.Task | None = None # last scheduled setup task - _cleanup_task: asyncio.Task | None = None # last scheduled cleanup task + _setup_task: asyncio.Task | None = None + """The task running the last `async` setup, cancelled when the effect re-runs or + the component unmounts.""" + + _cleanup_task: asyncio.Task | None = None + """The task finishing the effect's last cleanups, when one of them is `async`. + Until it has finished and the effect's waiting setups and cleanups have run, + new ones wait in `_queued`.""" + + _queued: list[Callable[[], Any | Awaitable[Any]] | None] = field( + default_factory=list + ) + """Setups and cleanups waiting for `_cleanup_task`, in the order they were + scheduled: a setup as the setup function it was scheduled with, a cleanup as + `None`.""" + + _active: bool = False + """Whether a setup has run since the last cleanup, so there is something to + clean up.""" + + def has_cleanup(self) -> bool: + """ + Whether the effect has a cleanup to run: one its setup returned, the + `cleanup` argument, or both. + """ + return callable(self._returned_cleanup) or callable(self.cleanup) def has_lifecycle_work(self) -> bool: + """ + Whether unmounting has work to do: a cleanup to run, or a setup task still + running to cancel. + """ + return self.has_cleanup() or ( + self._setup_task is not None and not self._setup_task.done() + ) + + def take_cleanups(self) -> list[Callable[[], Any | Awaitable[Any]]]: + """ + Returns the cleanups to run, in order: the one the last setup returned, then + the `cleanup` argument. + + The returned cleanup belongs to the setup run that returned it, so it is + forgotten here and runs only once. + """ + cleanups = [c for c in (self._returned_cleanup, self.cleanup) if callable(c)] + self._returned_cleanup = None + return cleanups + + def running_cleanup(self) -> asyncio.Task | None: + """ + Returns the task finishing the effect's last cleanups, until it has finished + and the setups and cleanups waiting for it have run. + """ + return self._cleanup_task + + def is_live(self) -> bool: + """ + Whether the effect still belongs to a mounted component: it hasn't unmounted + and hasn't been replaced by a remount. + """ + component = self.component return ( - callable(self.cleanup) - or callable(self._returned_cleanup) - or (self._setup_task is not None and not self._setup_task.done()) + component is not None + and component._state.mounted + and self in component._state.hooks ) - def cancel(self): + def take_queued(self) -> list[Callable[[], Any | Awaitable[Any]] | None]: + """ + Returns what to run of the setups and cleanups that waited for the effect's + cleanups, in `_queued`'s form: one cleanup if any was queued, then the last + setup if it came last. Earlier setups never ran and were superseded, so + there is nothing to run them for. No setup is returned once the effect + isn't live (see `is_live()`). """ - Cancels currently scheduled setup and cleanup tasks, if still pending. + queued, self._queued = self._queued, [] + ops: list[Callable[[], Any | Awaitable[Any]] | None] = [] + if any(op is None for op in queued): + ops.append(None) + if queued and queued[-1] is not None and self.is_live(): + ops.append(queued[-1]) + return ops + + def cancel_setup(self): + """ + Cancels the last setup task, if it is still running. - Safe to call repeatedly; completed tasks are ignored. + A running cleanup task is left to finish. Safe to call repeatedly. """ if self._setup_task and not self._setup_task.done(): self._setup_task.cancel() - if self._cleanup_task and not self._cleanup_task.done(): - self._cleanup_task.cancel() def use_effect( @@ -53,17 +125,56 @@ def use_effect( cleanup: Callable[[], Any | Awaitable[Any]] | None = None, ): """ - Perform side effects in function components. + Runs a side effect, such as starting a timer or opening a connection, after the + component renders, and cleans it up when it is no longer needed. + + The setup runs after the component mounts, then again after each render that + `dependencies` selects. Before it runs again, and when the component unmounts, + the effect's cleanups run: the cleanup the setup returned, then `cleanup`. They + run once for each time the setup ran, and always finish, in order, before the + setup runs again, even when one of them is `async`. + + A cleanup the setup returns belongs to that setup run, so it sees the same + values the setup saw. `cleanup` is the function the latest render passed, so it + sees that render's values. Returning the cleanup from the setup is usually what + you want. Args: - setup: A function that performs the side effect. It may optionally return - a cleanup function. + setup: A function that performs the side effect. It may be `async`. A + non-`async` setup may return a cleanup function. An `async` setup's + return value is not used: pass `cleanup` instead. An `async` setup still + running when the effect re-runs or the component unmounts is cancelled. dependencies: Values the effect depends on. The effect always runs after the component mounts. If absent, it also re-runs after every render. If empty, it runs only on mount. Otherwise, it re-runs only when one of the dependencies has changed. - cleanup: An optional function that cleans up after the effect. It is run - before the effect is re-run, and when the component unmounts. + cleanup: A function that cleans up after the effect, run after the cleanup + the setup returned. It may be `async`. + + Example: + ```python + import asyncio + from datetime import datetime + + import flet as ft + + + @ft.component + def Clock(): + now, set_now = ft.use_state(datetime.now()) + + def start_ticking(): + async def tick(): + while True: + await asyncio.sleep(1) + set_now(datetime.now()) + + task = asyncio.create_task(tick()) + return task.cancel # stops ticking when Clock unmounts + + ft.use_effect(start_ticking, []) + return ft.Text(f"{now:%H:%M:%S}") + ``` """ component = current_component() deps = list(dependencies) if dependencies is not None else None @@ -87,24 +198,21 @@ def use_effect( def on_mounted(fn: Callable[[], Any | Awaitable[Any]]) -> None: """ - Run exactly once after the component mounts. + Runs `fn` once, after the component mounts. Args: - fn (Callable[[], Any | Awaitable[Any]]): A function to run after the component - mounts. + fn: The function to run. It may be `async`. """ use_effect(fn, dependencies=[]) def on_unmounted(fn: Callable[[], Any | Awaitable[Any]]) -> None: """ - Run exactly once when the component unmounts. + Runs `fn` once, when the component unmounts. Args: - fn (Callable[[], Any | Awaitable[Any]]): A function to run when the component - unmounts. + fn: The function to run. It may be `async`. """ - # No-op setup; only need cleanup to fire on unmount use_effect(lambda: None, dependencies=[], cleanup=fn) @@ -112,16 +220,14 @@ def on_updated( fn: Callable[[], Any | Awaitable[Any]], dependencies: Sequence[Any] | None = None ) -> None: """ - Run after each post-mount render (or when dependencies change). - With dependencies=None this fires every update; with dependencies=[...] only - on changes. + Runs `fn` after the component mounts, then again after each render that + `dependencies` selects. Args: - fn (Callable[[], Any | Awaitable[Any]]): A function to run after each - post-mount render (or when dependencies change). - dependencies (Sequence[Any] | None): If present, `fn` is only run when one - of the dependencies has changed. If absent, `fn` is run after every - render. + fn: The function to run. It may be `async`. + dependencies: Values `fn` depends on. If absent, `fn` re-runs after every + render. Otherwise, it re-runs only when one of the dependencies has + changed. """ use_effect(fn, dependencies=dependencies) diff --git a/sdk/python/packages/flet/src/flet/messaging/session.py b/sdk/python/packages/flet/src/flet/messaging/session.py index b7fab782ac..14dfb47b5c 100644 --- a/sdk/python/packages/flet/src/flet/messaging/session.py +++ b/sdk/python/packages/flet/src/flet/messaging/session.py @@ -4,6 +4,7 @@ import logging import traceback import weakref +from collections.abc import Awaitable, Callable from datetime import datetime, timedelta, timezone from typing import TYPE_CHECKING, Any, Optional @@ -60,7 +61,12 @@ def __init__(self, conn: Connection): self.__method_call_results: dict[asyncio.Event, tuple[Any, Optional[str]]] = {} self.__updates_ready: asyncio.Event = asyncio.Event() self.__pending_updates: set[BaseControl] = set() - self.__pending_effects: list[tuple[weakref.ref[EffectHook], bool]] = [] + self.__pending_effects: list[ + tuple[EffectHook, Callable[[], Any | Awaitable[Any]] | None] + ] = [] + self.__effect_cleanup_tasks: set[asyncio.Task] = set() + """Tasks awaiting `async` effect cleanups, held until they finish so they + aren't garbage collected while pending.""" self.__updates_task: Optional[asyncio.Task] = None self.__closed = False # Whether this session's app renders via the declarative components @@ -707,7 +713,7 @@ def schedule_effect(self, hook: "EffectHook", is_cleanup: bool): # Hold a strong reference to the hook until it runs. A weakref would # get cleared when the owning component unmounts and clears # `_state.hooks` — dropping queued cleanup effects on the floor. - self.__pending_effects.append((hook, is_cleanup)) + self.__pending_effects.append((hook, None if is_cleanup else hook.setup)) self.__updates_ready.set() def start_updates_scheduler(self): @@ -743,31 +749,139 @@ async def __updates_scheduler(self): pending_effects = list(self.__pending_effects) self.__pending_effects.clear() - for effect in pending_effects: - try: - hook = effect[0] - is_cleanup = effect[1] - # print(f"**** Running effect: {hook} {is_cleanup}") - if hook and hook.setup and not is_cleanup: - hook.cancel() - res = None - if inspect.iscoroutinefunction(hook.setup): - hook._setup_task = asyncio.create_task(hook.setup()) - else: - res = hook.setup() - hook._returned_cleanup = res if callable(res) else None - elif hook and is_cleanup: - hook.cancel() - cleanup = hook.cleanup or hook._returned_cleanup - hook._returned_cleanup = None - if callable(cleanup) and inspect.iscoroutinefunction( - cleanup - ): - hook._cleanup_task = asyncio.create_task(cleanup()) - elif callable(cleanup): - cleanup() - except Exception as ex: - tb = traceback.format_exc() - self.error(f"Exception in effect: {ex}\n{tb}") + for hook, setup in pending_effects: + if hook: + self.__run_effect(hook, setup) except asyncio.CancelledError: pass + + def __run_effect( + self, hook: "EffectHook", setup: Callable[[], Any | Awaitable[Any]] | None + ): + """ + Runs an effect's `setup`, the setup function it was scheduled with, or its + cleanups when `setup` is `None`. Queues it while the effect's previous + cleanups are still finishing. + + Cleanups run only if a setup ran since the last ones, and always before the + next setup, even when that setup was scheduled without them. A setup that + runs after its component unmounted is cleaned up right away. A + `CancelledError` the effect raises itself means it was cancelled, which + isn't an error: running an effect never awaits, so it can't come from the + scheduler being cancelled. Other errors are reported. + """ + try: + if hook.running_cleanup(): + hook._queued.append(setup) + return + hook.cancel_setup() + if hook._active and (setup is None or hook.has_cleanup()): + hook._active = False + self.__run_effect_cleanups(hook, hook.take_cleanups()) + if hook.running_cleanup() and setup is not None: + hook._queued.append(setup) + return + if setup is not None: + hook._active = True + if inspect.iscoroutinefunction(setup): + hook._setup_task = asyncio.create_task(setup()) + else: + res = setup() + hook._returned_cleanup = res if callable(res) else None + if not hook.is_live(): + hook.cancel_setup() + hook._active = False + self.__run_effect_cleanups(hook, hook.take_cleanups()) + except asyncio.CancelledError: + pass + except Exception as ex: + self.__report_effect_error(ex) + + def __run_effect_cleanups( + self, + hook: "EffectHook", + cleanups: list[Callable[[], Any | Awaitable[Any]]], + ): + """ + Runs an effect's cleanups in order. + + Non-`async` cleanups run right away, up to the first `async` one. That one + and the rest are finished in a task. Until it finishes, the effect's next + setups and cleanups wait. Then they run right away, before anything + scheduled later, unless the session has closed or disconnected. + """ + started = None + for i, cleanup in enumerate(cleanups): + if inspect.iscoroutinefunction(cleanup): + break + started = self.__call_effect_cleanup(cleanup) + if started is not None: + i += 1 + break + else: + return + task = asyncio.create_task(self.__finish_effect_cleanups(cleanups[i:], started)) + hook._cleanup_task = task + self.__effect_cleanup_tasks.add(task) + + def done(task: asyncio.Task): + self.__effect_cleanup_tasks.discard(task) + hook._cleanup_task = None + queued = hook.take_queued() + if self.__closed or (self.__conn is None and self.__expires_at is not None): + return + for setup in queued: + self.__run_effect(hook, setup) + + task.add_done_callback(done) + + def __call_effect_cleanup( + self, cleanup: Callable[[], Any | Awaitable[Any]] + ) -> Awaitable[Any] | None: + """ + Calls a cleanup and returns what it returned if that is awaitable. + + A cleanup that raises is reported; one that raises `CancelledError` was + cancelled, which isn't an error. + """ + try: + res = cleanup() + return res if inspect.isawaitable(res) else None + except asyncio.CancelledError: + return None + except Exception as ex: + self.__report_effect_error(ex) + return None + + async def __finish_effect_cleanups( + self, + cleanups: list[Callable[[], Any | Awaitable[Any]]], + started: Awaitable[Any] | None, + ): + """ + Finishes an effect's cleanups in order: awaits `started`, then calls and + awaits each of `cleanups`. + + One that raises is reported, and one that raises or is cancelled doesn't + keep the others from running. If this task itself is cancelled, the + cleanups not reached are skipped. Telling that apart from a cleanup + cancelled on its own needs `Task.cancelling()`, so on Python 3.10 they + still run. + """ + pending = [started] if started is not None else [] + for item in pending + cleanups: + try: + res = item if inspect.isawaitable(item) else item() + if inspect.isawaitable(res): + await res + except asyncio.CancelledError: + task = asyncio.current_task() + if task is not None and getattr(task, "cancelling", lambda: 0)(): + raise + except Exception as ex: + self.__report_effect_error(ex) + + def __report_effect_error(self, ex: Exception): + """Reports an exception raised by an effect setup or cleanup.""" + tb = "".join(traceback.format_exception(type(ex), ex, ex.__traceback__)) + self.error(f"Exception in effect: {ex}\n{tb}") diff --git a/sdk/python/packages/flet/tests/test_component_effects.py b/sdk/python/packages/flet/tests/test_component_effects.py deleted file mode 100644 index 7151e1e2f5..0000000000 --- a/sdk/python/packages/flet/tests/test_component_effects.py +++ /dev/null @@ -1,120 +0,0 @@ -import asyncio - -import pytest - -import flet.components.hooks.use_effect as use_effect_module -from flet.components.component import Component -from flet.components.hooks.use_effect import EffectHook - - -def test_render_effect_changed_deps_runs_cleanup_then_setup_once_each(): - c = Component(fn=lambda: None, args=(), kwargs={}) - c._state.mounted = True - calls: list[bool] = [] - c._schedule_effect = lambda hook, is_cleanup=False: calls.append(is_cleanup) # type: ignore[method-assign] - - c._state.hooks = [ - EffectHook( - c, - setup=lambda: None, - _returned_cleanup=lambda: None, - deps=[2], - prev_deps=[1], - ) - ] - - c._run_render_effects() - - assert calls == [True, False] - - -def test_render_effect_unchanged_deps_schedules_nothing(): - c = Component(fn=lambda: None, args=(), kwargs={}) - c._state.mounted = True - calls: list[bool] = [] - c._schedule_effect = lambda hook, is_cleanup=False: calls.append(is_cleanup) # type: ignore[method-assign] - - c._state.hooks = [ - EffectHook( - c, - setup=lambda: None, - cleanup=lambda: None, - deps=[1], - prev_deps=[1], - ) - ] - - c._run_render_effects() - - assert calls == [] - - -def test_will_unmount_releases_cached_render_references(): - c = Component(fn=lambda: None, args=(1, 2, 3), kwargs={"k": "v"}) - c._state.mounted = True - c._b = ["body"] - c._state.last_b = ["memo-body"] - c._state.last_args = ("a", "b") - c._state.last_kwargs = {"x": "y"} - c._contexts = {object(): object()} - - c.will_unmount() - - assert c._b is None - assert c._state.last_b is None - assert c._state.last_args == () - assert c._state.last_kwargs == {} - assert c._contexts == {} - - -def test_use_effect_retains_setup_returned_cleanup_across_renders(monkeypatch): - c = Component(fn=lambda: None, args=(), kwargs={}) - - def cleanup(): - return None - - def setup(): - return cleanup - - monkeypatch.setattr(use_effect_module, "current_component", lambda: c) - - use_effect_module.use_effect(setup, dependencies=[]) - hook = c._state.hooks[0] - hook._returned_cleanup = cleanup - - c._state.hook_cursor = 0 - use_effect_module.use_effect(setup, dependencies=[]) - - assert hook._returned_cleanup is cleanup - - -def test_unmount_schedules_returned_cleanup(): - c = Component(fn=lambda: None, args=(), kwargs={}) - c._state.mounted = True - calls: list[bool] = [] - c._schedule_effect = lambda hook, is_cleanup=False: calls.append(is_cleanup) # type: ignore[method-assign] - hook = EffectHook(c, setup=lambda: None, _returned_cleanup=lambda: None) - c._state.hooks = [hook] - - c._run_unmount_effects() - - assert calls == [True] - - -@pytest.mark.asyncio -async def test_unmount_schedules_cleanup_for_pending_setup_task(): - c = Component(fn=lambda: None, args=(), kwargs={}) - c._state.mounted = True - calls: list[bool] = [] - c._schedule_effect = lambda hook, is_cleanup=False: calls.append(is_cleanup) # type: ignore[method-assign] - task = asyncio.create_task(asyncio.sleep(60)) - hook = EffectHook(c, setup=lambda: None) - hook._setup_task = task - c._state.hooks = [hook] - - c._run_unmount_effects() - task.cancel() - with pytest.raises(asyncio.CancelledError): - await task - - assert calls == [True] diff --git a/sdk/python/packages/flet/tests/test_component_unmount.py b/sdk/python/packages/flet/tests/test_component_unmount.py new file mode 100644 index 0000000000..51285c857d --- /dev/null +++ b/sdk/python/packages/flet/tests/test_component_unmount.py @@ -0,0 +1,20 @@ +from flet.components.component import Component + + +def test_will_unmount_releases_cached_render_references(): + """Unmounting drops the cached body, render arguments and contexts.""" + c = Component(fn=lambda: None, args=(1, 2, 3), kwargs={"k": "v"}) + c._state.mounted = True + c._b = ["body"] + c._state.last_b = ["memo-body"] + c._state.last_args = ("a", "b") + c._state.last_kwargs = {"x": "y"} + c._contexts = {object(): object()} + + c.will_unmount() + + assert c._b is None + assert c._state.last_b is None + assert c._state.last_args == () + assert c._state.last_kwargs == {} + assert c._contexts == {} diff --git a/sdk/python/packages/flet/tests/test_session_disconnect_buffering.py b/sdk/python/packages/flet/tests/test_session_disconnect_buffering.py index 5d439f6f64..2ef7110308 100644 --- a/sdk/python/packages/flet/tests/test_session_disconnect_buffering.py +++ b/sdk/python/packages/flet/tests/test_session_disconnect_buffering.py @@ -1,8 +1,5 @@ -import asyncio from datetime import datetime, timezone -import pytest - from flet.components.component import Component from flet.components.hooks.use_effect import EffectHook from flet.messaging.connection import Connection @@ -73,56 +70,3 @@ def test_attach_connection_restores_state_and_flushes_buffer(): assert session.expires_at is None assert session._Session__send_buffer == [] assert new_conn.messages == [buffered_message] - - -@pytest.mark.asyncio -async def test_cleanup_returned_by_effect_setup_runs_on_unmount(): - conn = Connection() - conn.pubsubhub = PubSubHub() - session = Session(conn) - component = Component(fn=lambda: None, args=(), kwargs={}) - component._state.mounted = True - cleanup_ran = asyncio.Event() - - def cleanup(): - cleanup_ran.set() - - hook = EffectHook(component, setup=lambda: None, _returned_cleanup=cleanup) - component._state.hooks = [hook] - component._schedule_effect = session.schedule_effect # type: ignore[method-assign] - component._run_unmount_effects() - session.start_updates_scheduler() - - await asyncio.wait_for(cleanup_ran.wait(), timeout=1) - session._Session__closed = True - session._Session__updates_ready.set() - await session._Session__updates_task - - -@pytest.mark.asyncio -async def test_unmount_cancels_pending_async_effect_setup(): - conn = Connection() - conn.pubsubhub = PubSubHub() - session = Session(conn) - component = Component(fn=lambda: None, args=(), kwargs={}) - component._state.mounted = True - started = asyncio.Event() - - async def setup(): - started.set() - await asyncio.Event().wait() - - task = asyncio.create_task(setup()) - await started.wait() - hook = EffectHook(component, setup=setup) - hook._setup_task = task - component._state.hooks = [hook] - component._schedule_effect = session.schedule_effect # type: ignore[method-assign] - component._run_unmount_effects() - session.start_updates_scheduler() - - with pytest.raises(asyncio.CancelledError): - await asyncio.wait_for(task, timeout=1) - session._Session__closed = True - session._Session__updates_ready.set() - await session._Session__updates_task diff --git a/sdk/python/packages/flet/tests/test_use_effect.py b/sdk/python/packages/flet/tests/test_use_effect.py new file mode 100644 index 0000000000..571254c723 --- /dev/null +++ b/sdk/python/packages/flet/tests/test_use_effect.py @@ -0,0 +1,781 @@ +import asyncio +import gc +import sys +import weakref +from datetime import datetime, timezone + +import pytest +import pytest_asyncio + +import flet.components.hooks.use_effect as use_effect_module +from flet.components.component import Component +from flet.components.hooks.use_effect import EffectHook +from flet.messaging.connection import Connection +from flet.messaging.session import Session +from flet.pubsub.pubsub_hub import PubSubHub + + +class EffectRunner: + """A component with a session effect loop, rendered by hand.""" + + def __init__(self, monkeypatch: pytest.MonkeyPatch): + conn = Connection() + conn.pubsubhub = PubSubHub() + self.session = Session(conn) + self.errors: list[str] = [] + self.session.error = self.errors.append # type: ignore[method-assign] + self.component = Component(fn=lambda: None, args=(), kwargs={}) + self.component._schedule_effect = self.session.schedule_effect # type: ignore[method-assign] + monkeypatch.setattr( + use_effect_module, "current_component", lambda: self.component + ) + self.session.start_updates_scheduler() + + def _render(self, body): + self.component._state.hook_cursor = 0 + body() + + async def _drain(self): + for _ in range(3): + self.session._Session__updates_ready.set() + await asyncio.sleep(0.01) + + async def mount(self, body): + self._render(body) + self.component._state.mounted = True + self.component._run_mount_effects() + await self._drain() + + async def rerender(self, body): + self._render(body) + self.component._run_render_effects() + await self._drain() + + async def unmount(self): + self.component._state.mounted = False + self.component._run_unmount_effects() + await self._drain() + + async def close(self): + self.session._Session__closed = True + self.session._Session__updates_ready.set() + await self.session._Session__updates_task + + +@pytest_asyncio.fixture +async def runner(monkeypatch): + r = EffectRunner(monkeypatch) + yield r + await r.close() + + +@pytest.fixture +def scheduled() -> tuple[Component, list[bool]]: + """A mounted component that records what it schedules instead of running it.""" + c = Component(fn=lambda: None, args=(), kwargs={}) + c._state.mounted = True + calls: list[bool] = [] + c._schedule_effect = lambda hook, is_cleanup=False: calls.append(is_cleanup) # type: ignore[method-assign] + return c, calls + + +def returning_cleanup(log: list[str], tag: str): + def setup(): + log.append(f"setup {tag}") + return lambda: log.append(f"cleanup {tag}") + + return setup + + +CLEANUP_KINDS = pytest.mark.parametrize( + "cleanup_field", ["cleanup", "_returned_cleanup"], ids=["argument", "returned"] +) + + +class TestEffectScheduling: + """What a component queues for its effects on re-render and unmount.""" + + @CLEANUP_KINDS + def test_changed_deps_schedule_cleanup_then_setup(self, scheduled, cleanup_field): + """Changed dependencies queue the cleanup, then the setup, once each.""" + c, calls = scheduled + c._state.hooks = [ + EffectHook( + c, + setup=lambda: None, + deps=[2], + prev_deps=[1], + **{cleanup_field: lambda: None}, + ) + ] + + c._run_render_effects() + + assert calls == [True, False] + + def test_unchanged_deps_schedule_nothing(self, scheduled): + """Unchanged dependencies queue nothing.""" + c, calls = scheduled + c._state.hooks = [ + EffectHook( + c, setup=lambda: None, cleanup=lambda: None, deps=[1], prev_deps=[1] + ) + ] + + c._run_render_effects() + + assert calls == [] + + @CLEANUP_KINDS + def test_unmount_schedules_cleanup(self, scheduled, cleanup_field): + """Unmounting queues the effect's cleanup.""" + c, calls = scheduled + c._state.hooks = [ + EffectHook(c, setup=lambda: None, **{cleanup_field: lambda: None}) + ] + + c._run_unmount_effects() + + assert calls == [True] + + @pytest.mark.asyncio + async def test_unmount_schedules_cleanup_for_running_setup_task(self, scheduled): + """Unmounting queues a cleanup for an effect whose setup task still runs.""" + c, calls = scheduled + task = asyncio.create_task(asyncio.sleep(60)) + hook = EffectHook(c, setup=lambda: None) + hook._setup_task = task + c._state.hooks = [hook] + + c._run_unmount_effects() + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + assert calls == [True] + + +class TestEffectLoop: + """Effects driven end-to-end: render, mount, re-render and unmount.""" + + @pytest.mark.asyncio + async def test_returned_cleanup_runs_on_unmount_after_rerender(self, runner): + """A cleanup returned by the setup runs on unmount after a re-render.""" + log: list[str] = [] + setup = returning_cleanup(log, "a") + + def body(): + use_effect_module.use_effect(setup, []) + + await runner.mount(body) + await runner.rerender(body) + await runner.unmount() + + assert log == ["setup a", "cleanup a"] + + @pytest.mark.asyncio + async def test_returned_cleanup_runs_before_setup_on_dependency_change( + self, runner + ): + """A dependency change runs the returned cleanup before the setup re-runs.""" + log: list[str] = [] + + await runner.mount( + lambda: use_effect_module.use_effect(returning_cleanup(log, "1"), [1]) + ) + await runner.rerender( + lambda: use_effect_module.use_effect(returning_cleanup(log, "2"), [2]) + ) + await runner.unmount() + + assert log == ["setup 1", "cleanup 1", "setup 2", "cleanup 2"] + + @pytest.mark.asyncio + async def test_returned_cleanup_and_cleanup_argument_both_run(self, runner): + """The returned cleanup runs first, then the `cleanup` argument.""" + log: list[str] = [] + + await runner.mount( + lambda: use_effect_module.use_effect( + returning_cleanup(log, "returned"), + [], + cleanup=lambda: log.append("cleanup argument"), + ) + ) + await runner.unmount() + + assert log == ["setup returned", "cleanup returned", "cleanup argument"] + + @pytest.mark.asyncio + async def test_setup_waits_for_async_cleanup(self, runner): + """A dependency change runs the setup again only after an `async` cleanup has + finished.""" + log: list[str] = [] + + async def cleanup(): + await asyncio.sleep(0) + log.append("cleanup") + + await runner.mount( + lambda: use_effect_module.use_effect( + lambda: log.append("setup 1"), [1], cleanup + ) + ) + await runner.rerender( + lambda: use_effect_module.use_effect( + lambda: log.append("setup 2"), [2], cleanup + ) + ) + + assert log == ["setup 1", "cleanup", "setup 2"] + + @pytest.mark.asyncio + async def test_cleanups_finish_in_order_before_setup_reruns(self, runner): + """An `async` returned cleanup finishes before a non-`async` `cleanup` + argument runs, and both before the setup runs again.""" + log: list[str] = [] + + def make_setup(tag): + def setup(): + log.append(f"setup {tag}") + + async def flush(): + await asyncio.sleep(0) + log.append(f"flush {tag}") + + return flush + + return setup + + def close(): + log.append("close") + + await runner.mount( + lambda: use_effect_module.use_effect(make_setup("1"), [1], close) + ) + await runner.rerender( + lambda: use_effect_module.use_effect(make_setup("2"), [2], close) + ) + + assert log == ["setup 1", "flush 1", "close", "setup 2"] + + @pytest.mark.asyncio + async def test_async_cleanup_finishes_when_nothing_else_references_it(self, runner): + """An `async` cleanup finishes even when nothing else references it across a + garbage collection.""" + log: list[str] = [] + gates: list[weakref.ref[asyncio.Future]] = [] + + def make_setup(tag): + def setup(): + log.append(f"setup {tag}") + + async def cleanup(): + gate = asyncio.get_running_loop().create_future() + gates.append(weakref.ref(gate)) + await gate + log.append(f"cleanup {tag}") + + return cleanup + + return setup + + await runner.mount(lambda: use_effect_module.use_effect(make_setup("1"), [1])) + await runner.rerender( + lambda: use_effect_module.use_effect(make_setup("2"), [2]) + ) + + gc.collect() + (gate,) = [g for ref in gates if (g := ref()) is not None] + gate.set_result(None) + await asyncio.sleep(0.01) + + assert log == ["setup 1", "cleanup 1", "setup 2"] + assert not runner.session._Session__effect_cleanup_tasks + + @pytest.mark.asyncio + async def test_setup_superseded_while_waiting_never_runs(self, runner): + """A setup still waiting for cleanups when the effect re-runs again is + skipped, and only the latest setup runs.""" + log: list[str] = [] + release = asyncio.Event() + + async def first_cleanup(): + await release.wait() + log.append("cleanup 1") + + def setup_1(): + log.append("setup 1") + return first_cleanup + + await runner.mount(lambda: use_effect_module.use_effect(setup_1, [1])) + await runner.rerender( + lambda: use_effect_module.use_effect(lambda: log.append("setup 2"), [2]) + ) + await runner.rerender( + lambda: use_effect_module.use_effect(lambda: log.append("setup 3"), [3]) + ) + release.set() + await asyncio.sleep(0.01) + + assert log == ["setup 1", "cleanup 1", "setup 3"] + + @pytest.mark.asyncio + async def test_unmount_cancels_setup_waiting_for_cleanups(self, runner): + """Unmounting while a setup waits for cleanups means it never runs.""" + log: list[str] = [] + release = asyncio.Event() + + async def first_cleanup(): + await release.wait() + log.append("cleanup 1") + + def setup_1(): + log.append("setup 1") + return first_cleanup + + await runner.mount(lambda: use_effect_module.use_effect(setup_1, [1])) + await runner.rerender( + lambda: use_effect_module.use_effect(lambda: log.append("setup 2"), [2]) + ) + await runner.unmount() + release.set() + await asyncio.sleep(0.01) + + assert log == ["setup 1", "cleanup 1"] + + @pytest.mark.asyncio + async def test_cleanup_argument_runs_once_per_setup_while_cleanups_wait( + self, runner + ): + """Re-renders during a slow cleanup don't run the `cleanup` argument again + for setups that never ran.""" + log: list[str] = [] + release = asyncio.Event() + + def setup(): + log.append("setup") + + async def flush(): + await release.wait() + log.append("flush") + + return flush + + def close(): + log.append("close") + + def body(): + use_effect_module.use_effect(setup, None, close) + + await runner.mount(body) + for _ in range(3): + await runner.rerender(body) + release.set() + await asyncio.sleep(0.01) + + assert log == ["setup", "flush", "close", "setup"] + + @pytest.mark.asyncio + async def test_back_to_back_setups_run_the_cleanup_between(self, runner): + """Two setups scheduled before the effect loop runs still run the first + one's cleanup between them.""" + log: list[str] = [] + runs = iter(["1", "2"]) + + def setup(): + tag = next(runs) + log.append(f"setup {tag}") + return lambda: log.append(f"cleanup {tag}") + + runner._render(lambda: use_effect_module.use_effect(setup)) + runner.component._state.mounted = True + (hook,) = runner.component._state.hooks + runner.session.schedule_effect(hook, False) + runner.session.schedule_effect(hook, False) + await runner._drain() + + assert log == ["setup 1", "cleanup 1", "setup 2"] + + @pytest.mark.asyncio + async def test_waiting_setup_never_runs_after_unmount_while_disconnected( + self, runner + ): + """A setup waiting for cleanups doesn't run once the component has unmounted, + even when the session was disconnected and dropped the unmount cleanup.""" + log: list[str] = [] + release = asyncio.Event() + + async def first_cleanup(): + await release.wait() + log.append("cleanup 1") + + def setup_1(): + log.append("setup 1") + return first_cleanup + + await runner.mount(lambda: use_effect_module.use_effect(setup_1, [1])) + await runner.rerender( + lambda: use_effect_module.use_effect(lambda: log.append("setup 2"), [2]) + ) + runner.session._Session__conn = None + runner.session._Session__expires_at = datetime.now(timezone.utc) + await runner.unmount() + release.set() + await asyncio.sleep(0.01) + + assert log == ["setup 1", "cleanup 1"] + + @pytest.mark.asyncio + async def test_unmount_as_cleanups_finish_skips_the_waiting_setup(self, runner): + """Unmounting just as slow cleanups finish means the setup waiting for them + never runs.""" + log: list[str] = [] + release = asyncio.Event() + + async def first_cleanup(): + await release.wait() + log.append("cleanup 1") + + def setup_1(): + log.append("setup 1") + return first_cleanup + + def setup_2(): + log.append("setup 2") + return lambda: log.append("cleanup 2") + + await runner.mount(lambda: use_effect_module.use_effect(setup_1, [1])) + await runner.rerender(lambda: use_effect_module.use_effect(setup_2, [2])) + release.set() + runner.component._state.mounted = False + runner.component._run_unmount_effects() + await runner._drain() + + assert log == ["setup 1", "cleanup 1"] + + @pytest.mark.asyncio + async def test_render_as_cleanups_finish_runs_the_latest_setup_once(self, runner): + """A render just as slow cleanups finish runs only its own setup, once.""" + log: list[str] = [] + release = asyncio.Event() + + async def first_cleanup(): + await release.wait() + log.append("cleanup 1") + + def setup_1(): + log.append("setup 1") + return first_cleanup + + def make_setup(tag): + def setup(): + log.append(f"setup {tag}") + return lambda: log.append(f"cleanup {tag}") + + return setup + + await runner.mount(lambda: use_effect_module.use_effect(setup_1, [1])) + await runner.rerender( + lambda: use_effect_module.use_effect(make_setup("2"), [2]) + ) + release.set() + runner._render(lambda: use_effect_module.use_effect(make_setup("3"), [3])) + runner.component._run_render_effects() + await runner._drain() + + assert log == ["setup 1", "cleanup 1", "setup 3"] + + @pytest.mark.asyncio + async def test_each_render_runs_its_own_setup_once(self, runner): + """A render landing between slow cleanups finishing and the waiting setup + running doesn't run its setup twice: each render's setup runs once, in + order.""" + log: list[str] = [] + release = asyncio.Event() + + async def first_cleanup(): + await release.wait() + log.append("cleanup 1") + + def setup_1(): + log.append("setup 1") + return first_cleanup + + def make_setup(tag): + def setup(): + log.append(f"setup {tag}") + return lambda: log.append(f"cleanup {tag}") + + return setup + + def render_3(): + runner._render(lambda: use_effect_module.use_effect(make_setup("3"), [3])) + runner.component._run_render_effects() + + await runner.mount(lambda: use_effect_module.use_effect(setup_1, [1])) + await runner.rerender( + lambda: use_effect_module.use_effect(make_setup("2"), [2]) + ) + release.set() + asyncio.get_running_loop().call_soon(render_3) + await runner._drain() + + assert log.count("setup 3") == 1 + assert log[:2] == ["setup 1", "cleanup 1"] + assert log[-1] == "setup 3" + + @pytest.mark.asyncio + async def test_waiting_setup_never_runs_after_the_session_closes(self, runner): + """A setup waiting for cleanups doesn't run once the session has closed.""" + log: list[str] = [] + release = asyncio.Event() + + async def first_cleanup(): + await release.wait() + log.append("cleanup 1") + + def setup_1(): + log.append("setup 1") + return first_cleanup + + await runner.mount(lambda: use_effect_module.use_effect(setup_1, [1])) + await runner.rerender( + lambda: use_effect_module.use_effect(lambda: log.append("setup 2"), [2]) + ) + runner.session._Session__closed = True + release.set() + await asyncio.sleep(0.01) + + assert log == ["setup 1", "cleanup 1"] + + @pytest.mark.asyncio + async def test_quick_mount_and_unmount_runs_setup_then_cleanup(self, runner): + """A component unmounted before its effects ran still runs the setup, then + the cleanup, once each.""" + log: list[str] = [] + + runner._render( + lambda: use_effect_module.use_effect( + lambda: log.append("setup"), [], cleanup=lambda: log.append("cleanup") + ) + ) + runner.component._state.mounted = True + runner.component._run_mount_effects() + runner.component._state.mounted = False + runner.component._run_unmount_effects() + await runner._drain() + + assert log == ["setup", "cleanup"] + + @pytest.mark.asyncio + async def test_setup_running_after_unmount_is_cleaned_up_right_away(self, runner): + """A setup that runs after its component unmounted, with no cleanup + scheduled, is cleaned up right away.""" + log: list[str] = [] + runs = iter(["1", "2"]) + + def setup(): + tag = next(runs) + log.append(f"setup {tag}") + return lambda: log.append(f"cleanup {tag}") + + await runner.mount(lambda: use_effect_module.use_effect(setup, [1])) + (hook,) = runner.component._state.hooks + runner.component._state.mounted = False + runner.component._state.hooks.clear() + runner.session.schedule_effect(hook, False) + await runner._drain() + + assert log == ["setup 1", "cleanup 1", "setup 2", "cleanup 2"] + + @pytest.mark.asyncio + async def test_cancelled_error_from_a_cleanup_keeps_effects_running(self, runner): + """A cleanup raising `CancelledError` counts as cancelled, not failed, and + the effect's setup and later effects still run.""" + log: list[str] = [] + + def make_setup(tag): + def setup(): + log.append(f"setup {tag}") + + async def cleanup(): + log.append(f"async cleanup {tag}") + + return cleanup + + return setup + + def cancelled_cleanup(): + raise asyncio.CancelledError + + for i, deps in enumerate(([1], [2], [3])): + body = lambda deps=deps: use_effect_module.use_effect( # noqa: E731 + make_setup(deps[0]), deps, cancelled_cleanup + ) + await (runner.mount(body) if i == 0 else runner.rerender(body)) + + assert log == [ + "setup 1", + "async cleanup 1", + "setup 2", + "async cleanup 2", + "setup 3", + ] + assert runner.errors == [] + + @pytest.mark.asyncio + async def test_cancelled_async_cleanup_does_not_skip_the_other(self, runner): + """An `async` cleanup that gets cancelled doesn't keep the other `async` + cleanup from running.""" + log: list[str] = [] + + def setup(): + async def cleanup(): + future = asyncio.get_running_loop().create_future() + future.cancel() + await future + + return cleanup + + async def cleanup_argument(): + log.append("cleanup argument") + + await runner.mount( + lambda: use_effect_module.use_effect(setup, [], cleanup_argument) + ) + await runner.unmount() + + assert log == ["cleanup argument"] + assert runner.errors == [] + + @pytest.mark.asyncio + async def test_async_cleanups_run_one_after_the_other(self, runner): + """The `cleanup` argument starts only once an `async` returned cleanup has + finished.""" + log: list[str] = [] + + def setup(): + async def flush(): + for _ in range(3): + await asyncio.sleep(0) + log.append("flushed") + + return flush + + async def close(): + log.append("closed") + + await runner.mount(lambda: use_effect_module.use_effect(setup, [], close)) + await runner.unmount() + + assert log == ["flushed", "closed"] + + @pytest.mark.asyncio + async def test_async_cleanup_failure_is_reported_while_the_other_runs(self, runner): + """An `async` cleanup that raises is reported without waiting for the + cleanup after it, which may never finish.""" + + def setup(): + async def cleanup(): + raise RuntimeError("cleanup failed") + + return cleanup + + async def hanging_cleanup(): + await asyncio.get_running_loop().create_future() + + await runner.mount( + lambda: use_effect_module.use_effect(setup, [], hanging_cleanup) + ) + await runner.unmount() + + assert len(runner.errors) == 1 + assert "cleanup failed" in runner.errors[0] + for task in runner.session._Session__effect_cleanup_tasks: + task.cancel() + + @pytest.mark.skipif(sys.version_info < (3, 11), reason="needs Task.cancelling()") + @pytest.mark.asyncio + async def test_cancelling_cleanups_skips_the_ones_not_reached(self, runner): + """Cancelling the task finishing `async` cleanups skips the ones it hasn't + reached.""" + log: list[str] = [] + + def setup(): + async def cleanup(): + await asyncio.get_running_loop().create_future() + + return cleanup + + async def cleanup_argument(): + log.append("cleanup argument") + + await runner.mount( + lambda: use_effect_module.use_effect(setup, [], cleanup_argument) + ) + await runner.unmount() + (task,) = runner.session._Session__effect_cleanup_tasks + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + assert log == [] + assert runner.errors == [] + + @pytest.mark.asyncio + async def test_async_cleanup_return_value_is_ignored(self, runner): + """An `async` cleanup returning an exception object isn't reported.""" + + async def cleanup(): + return RuntimeError("just a value") + + await runner.mount( + lambda: use_effect_module.use_effect(lambda: None, [], cleanup) + ) + await runner.unmount() + + assert runner.errors == [] + + @pytest.mark.asyncio + async def test_failing_cleanup_does_not_skip_the_other(self, runner): + """A cleanup that raises is reported, and the other cleanup still runs.""" + log: list[str] = [] + + def setup(): + def cleanup(): + raise RuntimeError("cleanup failed") + + return cleanup + + await runner.mount( + lambda: use_effect_module.use_effect( + setup, [], cleanup=lambda: log.append("cleanup argument") + ) + ) + await runner.unmount() + + assert log == ["cleanup argument"] + assert len(runner.errors) == 1 + assert "cleanup failed" in runner.errors[0] + + @pytest.mark.asyncio + async def test_running_async_setup_is_cancelled_on_unmount(self, runner): + """An `async` setup still running at unmount is cancelled.""" + log: list[str] = [] + + async def setup(): + try: + await asyncio.sleep(10) + except asyncio.CancelledError: + log.append("cancelled") + raise + + def body(): + use_effect_module.use_effect(setup, []) + + await runner.mount(body) + await runner.rerender(body) + await runner.unmount() + + assert log == ["cancelled"]