flet icon indicating copy to clipboard operation
flet copied to clipboard

support `update_async` only if mounted

Open johnnychen94 opened this issue 1 year ago • 1 comments

a MWE for the background

The following code is a buggy implementation that I didn't realize until I actually hit it, so I feel it's a good candidate to raise a discussion for as either a bug report or feature request. I think it's more of a feature request.

Steps to reproduce:

good user pattern:

  1. run the script, it will show Loading... for 5 seconds, and then it will print Done.
  2. switch to page b

bad user pattern:

  1. run the script, it will show Loading...
  2. switch to page b as soon as possible before page a is fully loaded,
  3. wait until page A's async on-mounting task finishes, it will throw an assertion error like the following:

image

Task exception was never retrieved
future: <Task finished name='Task-14' coro=<__connect_internal_async.<locals>.on_event() done, defined at /Users/jc/Documents/mdc/mdc-simple-gui/.venv/lib/python3.11/site-packages/flet_runtime/app.py:430> exception=AssertionError('Control must be added to the page first.')>
Traceback (most recent call last):
  File "/Users/jc/Documents/mdc/mdc-simple-gui/.venv/lib/python3.11/site-packages/flet_runtime/app.py", line 432, in on_event
    await conn.sessions[e.sessionID].on_event_async(
  File "/Users/jc/Documents/mdc/mdc-simple-gui/.venv/lib/python3.11/site-packages/flet_core/page.py", line 514, in on_event_async
    await handler(ce)
  File "/Users/jc/Documents/mdc/mdc-simple-gui/test.py", line 8, in on_click_a
    await page.go_async("/a")
  File "/Users/jc/Documents/mdc/mdc-simple-gui/.venv/lib/python3.11/site-packages/flet_core/page.py", line 547, in go_async
    await self.__on_route_change.get_handler()(
  File "/Users/jc/Documents/mdc/mdc-simple-gui/.venv/lib/python3.11/site-packages/flet_core/event_handler.py", line 43, in __async_handler
    await h(r)
  File "/Users/jc/Documents/mdc/mdc-simple-gui/test.py", line 57, in on_router_change
    await page.update_async()
  File "/Users/jc/Documents/mdc/mdc-simple-gui/.venv/lib/python3.11/site-packages/flet_core/page.py", line 300, in update_async
    await self.__handle_mount_unmount_async(*r)
  File "/Users/jc/Documents/mdc/mdc-simple-gui/.venv/lib/python3.11/site-packages/flet_core/page.py", line 479, in __handle_mount_unmount_async
    await ctrl.did_mount_async()
  File "/Users/jc/Documents/mdc/mdc-simple-gui/test.py", line 47, in did_mount_async
    await self._view.update_async()
  File "/Users/jc/Documents/mdc/mdc-simple-gui/.venv/lib/python3.11/site-packages/flet_core/control.py", line 274, in update_async
    assert self.__page, "Control must be added to the page first."
AssertionError: Control must be added to the page first.
the flet script
import asyncio

import flet as ft

def build_app_bar(page: ft.Page):
    async def on_click_a(e):
        await page.go_async("/a")

    async def on_click_b(e):
        await page.go_async("/b")

    app_bar = ft.AppBar(
        actions=[
            ft.IconButton(
                ft.icons.HUB_ROUNDED,
                on_click=on_click_a,
                tooltip="go page A",
            ),
            ft.IconButton(
                ft.icons.FORMAT_LIST_BULLETED_ROUNDED,
                on_click=on_click_b,
                tooltip="go page B",
            ),
        ],
    )

    return app_bar


class MyAsyncPage(ft.UserControl):
    def __init__(self):
        super().__init__()
        self._view = ft.Column()

    def build(self):
        return self._view

    async def did_mount_async(self):
        self.running = True

        self._view.controls.clear()
        self._view.controls.append(ft.Text("Loading..."))
        await self._view.update_async()
        await asyncio.sleep(5)

        # there's no guarantee that this page is still mounted after 5 seconds
        # so self._view.update_async() could go wild
        self._view.controls.pop()
        self._view.controls.append(ft.Text("Done."))
        await self._view.update_async()

    async def will_unmount_async(self):
        self.running = False


async def main(page: ft.Page):
    appbar = build_app_bar(page)

    async def on_router_change(e: ft.RouteChangeEvent):
        if e.route == "/a":
            page.views.clear()
            page.views.append(ft.View("/a", [MyAsyncPage()], appbar=appbar))
            await page.update_async()

        elif e.route == "/b":
            page.views.clear()
            page.views.append(ft.View("/b", [ft.Text("This is page b")], appbar=appbar))
            await page.update_async()

    page.on_route_change = on_router_change
    await page.go_async("/a")


ft.app(main)

solution and suggestion

The solution for this specific scenario can be simple: add a status check before the update_async call

    async def did_mount_async(self):
        self.running = True

        self._view.controls.clear()
        self._view.controls.append(ft.Text("Loading..."))
        await self._view.update_async()
        await asyncio.sleep(5)

+       # only build the page while it's still mounted
+       if not self.running:
+           return
        self._view.controls.pop()
        self._view.controls.append(ft.Text("Done."))
        await self._view.update_async()

but this can be troublesome to do in a real-world app as programmers can hardly remember to check the page status when things become complicated (e.g., I myself could hardly understand why we need to double check self.running unless there's a comment explaining it.).

Also, I'm not very confident if the first await self._view.update_async() call could hit the issue. My understanding of Python tells me yes (since it's running in the background, the status can possibly be changed), but in real-world cases, users (real people) can hardly reach this. Then, should we add the seemly redundant if not self.running check to the first update_async call as well?

Thus, my suggestion (or feature request) is to introduce a keyword skip_if_not_mounted for the update_async (or maybe just change the builtin behavior to this) so that update_async can be a no-op when the page is not mounted. This way, programmers don't need to handle all the different mounting/unmounting scenarios.

johnnychen94 avatar Jan 15 '24 04:01 johnnychen94