fix: use_effect() skipping cleanups and re-running setups before cleanups finish

CHANGELOG.md

@@ -47,7 +47,7 @@
 
 ### Bug fixes
 
-* 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.

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)
 

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,41 +198,36 @@ 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)
 
 
 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)