Fix unmount (#901)

* remove upper bound

* revert #886 + add test + misc

* changelog entry

* changes for latest black

Author: rmorshea

docs/source/_exts/idom_example.py

@@ -18,7 +18,6 @@
 
 
 class WidgetExample(SphinxDirective):
-
     has_content = False
     required_arguments = 1
     _next_id = 0

docs/source/_exts/idom_view.py

@@ -13,7 +13,6 @@
 
 
 class IteractiveWidget(SphinxDirective):
-
     has_content = False
     required_arguments = 1
     _next_id = 0

docs/source/about/changelog.rst

@@ -23,7 +23,13 @@ more info, see the :ref:`Contributor Guide <Creating a Changelog Entry>`.
 Unreleased
 ----------
 
-No changes.
+**Reverted**
+
+- :pull:`901` - reverts :pull:`886` due to :issue:`896`
+
+**Fixed**
+
+- :issue:`896` - Stale event handlers after disconnect/reconnect cycle
 
 
 v1.0.0-a1

requirements/pkg-deps.txt

@@ -7,4 +7,4 @@ requests >=2
 colorlog >=6
 asgiref >=3
 lxml >=4
-click >=8, <9
+click >=8

src/client/packages/idom-client-react/src/mount.js

@@ -38,6 +38,9 @@ function mountLayoutWithReconnectingWebSocket(
   socket.onopen = (event) => {
     console.info(`IDOM WebSocket connected.`);
 
+    if (mountState.everMounted) {
+      ReactDOM.unmountComponentAtNode(element);
+    }
     _resetOpenMountState(mountState);
 
     mountLayout(element, {

src/idom/backend/tornado.py

@@ -158,7 +158,6 @@ def _setup_single_view_dispatcher_route(
 
 
 class IndexHandler(RequestHandler):
-
     _index_html: str
 
     def initialize(self, index_html: str) -> None:

src/idom/core/hooks.py

@@ -75,7 +75,6 @@ def use_state(initial_value: _Type | Callable[[], _Type]) -> State[_Type]:
 
 
 class _CurrentState(Generic[_Type]):
-
     __slots__ = "value", "dispatch"
 
     def __init__(
@@ -148,11 +147,9 @@ def use_effect(
     last_clean_callback: Ref[Optional[_EffectCleanFunc]] = 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() -> Optional[_EffectCleanFunc]:

src/idom/core/layout.py

@@ -651,7 +651,6 @@ class _LifeCycleState(NamedTuple):
 
 
 class _ThreadSafeQueue(Generic[_Type]):
-
     __slots__ = "_loop", "_queue", "_pending"
 
     def __init__(self) -> None:

src/idom/core/vdom.py

@@ -258,7 +258,6 @@ def separate_attributes_and_event_handlers(
     separated_handlers: DefaultDict[str, list[EventHandlerType]] = DefaultDict(list)
 
     for k, v in attributes.items():
-
         handler: EventHandlerType
 
         if callable(v):

src/idom/testing/common.py

@@ -56,6 +56,7 @@ async def until(
         condition: Callable[[_R], bool],
         timeout: float = IDOM_TESTING_DEFAULT_TIMEOUT.current,
         delay: float = _DEFAULT_POLL_DELAY,
+        description: str = "condition to be true",
     ) -> None:
         """Check that the coroutines result meets a condition within the timeout"""
         started_at = time.time()
@@ -66,7 +67,7 @@ async def until(
                 break
             elif (time.time() - started_at) > timeout:  # pragma: no cover
                 raise TimeoutError(
-                    f"Condition not met within {timeout} "
+                    f"Expected {description} after {timeout} "
                     f"seconds - last value was {result!r}"
                 )
 
@@ -77,7 +78,12 @@ async def until_is(
         delay: float = _DEFAULT_POLL_DELAY,
     ) -> None:
         """Wait until the result is identical to the given value"""
-        return await self.until(lambda left: left is right, timeout, delay)
+        return await self.until(
+            lambda left: left is right,
+            timeout,
+            delay,
+            f"value to be identical to {right!r}",
+        )
 
     async def until_equals(
         self,
@@ -86,7 +92,12 @@ async def until_equals(
         delay: float = _DEFAULT_POLL_DELAY,
     ) -> None:
         """Wait until the result is equal to the given value"""
-        return await self.until(lambda left: left == right, timeout, delay)
+        return await self.until(
+            lambda left: left == right,
+            timeout,
+            delay,
+            f"value to equal {right!r}",
+        )
 
 
 class HookCatcher:

tests/test_client.py

@@ -6,8 +6,9 @@
 
 import idom
 from idom.backend.utils import find_available_port
-from idom.testing import BackendFixture, DisplayFixture
+from idom.testing import BackendFixture, DisplayFixture, poll
 from tests.tooling.common import DEFAULT_TYPE_DELAY
+from tests.tooling.hooks import use_counter
 
 
 JS_DIR = Path(__file__).parent / "js"
@@ -21,45 +22,53 @@ async def test_automatic_reconnect(browser: Browser):
     page.set_default_timeout(10000)
 
     @idom.component
-    def OldComponent():
-        return idom.html.p("old", id="old-component")
+    def SomeComponent():
+        count, incr_count = use_counter(0)
+        return idom.html._(
+            idom.html.p("count", count, data_count=count, id="count"),
+            idom.html.button("incr", on_click=lambda e: incr_count(), id="incr"),
+        )
 
     async with AsyncExitStack() as exit_stack:
         server = await exit_stack.enter_async_context(BackendFixture(port=port))
         display = await exit_stack.enter_async_context(
             DisplayFixture(server, driver=page)
         )
 
-        await display.show(OldComponent)
+        await display.show(SomeComponent)
 
-        # ensure the element is displayed before stopping the server
-        await page.wait_for_selector("#old-component")
-
-    # the server is disconnected but the last view state is still shown
-    await page.wait_for_selector("#old-component")
+        count = await page.wait_for_selector("#count")
+        incr = await page.wait_for_selector("#incr")
 
-    set_state = idom.Ref(None)
+        for i in range(3):
+            assert (await count.get_attribute("data-count")) == str(i)
+            await incr.click()
 
-    @idom.component
-    def NewComponent():
-        state, set_state.current = idom.hooks.use_state(0)
-        return idom.html.p(f"new-{state}", id=f"new-component-{state}")
+    # the server is disconnected but the last view state is still shown
+    await page.wait_for_selector("#count")
 
     async with AsyncExitStack() as exit_stack:
         server = await exit_stack.enter_async_context(BackendFixture(port=port))
         display = await exit_stack.enter_async_context(
             DisplayFixture(server, driver=page)
         )
 
-        await display.show(NewComponent)
+        # use mount instead of show to avoid a page refesh
+        display.backend.mount(SomeComponent)
+
+        async def get_count():
+            # need to refetch element because may unmount on reconnect
+            count = await page.wait_for_selector("#count")
+            return await count.get_attribute("data-count")
+
+        for i in range(3):
+            # it may take a moment for the websocket to reconnect so need to poll
+            await poll(get_count).until_equals(str(i))
 
-        # Note the lack of a page refresh before looking up this new component. The
-        # client should attempt to reconnect and display the new view automatically.
-        await page.wait_for_selector("#new-component-0")
+            # need to refetch element because may unmount on reconnect
+            incr = await page.wait_for_selector("#incr")
 
-        # check that we can resume normal operation
-        set_state.current(1)
-        await page.wait_for_selector("#new-component-1")
+            await incr.click()
 
 
 async def test_style_can_be_changed(display: DisplayFixture):

tests/test_core/test_hooks.py

@@ -1057,7 +1057,6 @@ def SomeComponent():
         return idom.html.div()
 
     async with idom.Layout(SomeComponent()) as layout:
-
         with assert_idom_did_log(r"SomeComponent\(.*?\) message is 'hello'"):
             await layout.render()
 
@@ -1085,7 +1084,6 @@ def SomeComponent():
         return idom.html.div()
 
     async with idom.Layout(SomeComponent()) as layout:
-
         with assert_idom_did_log(r"SomeComponent\(.*?\) message is 'hello'"):
             await layout.render()
 
@@ -1111,7 +1109,6 @@ def SomeComponent():
         return idom.html.div()
 
     async with idom.Layout(SomeComponent()) as layout:
-
         with assert_idom_did_not_log(r"SomeComponent\(.*?\) message is 'hello'"):
             await layout.render()
 

tests/test_core/test_layout.py

@@ -174,7 +174,6 @@ def BadChild():
         raise ValueError("error from bad child")
 
     with assert_idom_did_log(match_error="error from bad child"):
-
         async with idom.Layout(Main()) as layout:
             assert (await layout.render()) == update_message(
                 path="",
@@ -225,7 +224,6 @@ def BadChild():
         raise ValueError("error from bad child")
 
     with assert_idom_did_log(match_error="error from bad child"):
-
         async with idom.Layout(Main()) as layout:
             assert (await layout.render()) == update_message(
                 path="",
@@ -754,7 +752,6 @@ def raise_error():
         return idom.html.button(on_click=raise_error)
 
     with assert_idom_did_log(match_error="bad event handler"):
-
         async with idom.Layout(ComponentWithBadEventHandler()) as layout:
             await layout.render()
             event = event_message(bad_handler.target)

tests/tooling/hooks.py

@@ -12,4 +12,4 @@ def use_toggle(init=False):
 
 def use_counter(initial_value):
     state, set_state = use_state(initial_value)
-    return state, lambda: set_state(state + 1)
+    return state, lambda: set_state(lambda old: old + 1)