Add event queue, fix #1320

Author: Archmonger

CHANGELOG.md

@@ -52,7 +52,7 @@ Don't forget to remove deprecated code on each major release!
 - Rewrite the `event-to-object` package to be more robust at handling properties on events.
 - Custom JS components will now automatically assume you are using ReactJS in the absence of a `bind` function.
 - Refactor layout rendering logic to improve readability and maintainability.
-- `@reactpy/client` now exports `React` and `ReactDOM`.
+- The JavaScript package `@reactpy/client` now exports `React` and `ReactDOM`, which allows third-party components to re-use the same React instance as ReactPy.
 - `reactpy.html` will now automatically flatten lists recursively (ex. `reactpy.html(["child1", ["child2"]])`)
 - `reactpy.utils.reactpy_to_string` will now retain the user's original casing for `data-*` and `aria-*` attributes.
 - `reactpy.utils.string_to_reactpy` has been upgraded to handle more complex scenarios without causing ReactJS rendering errors.

src/reactpy/core/layout.py

@@ -8,6 +8,7 @@
     create_task,
     current_task,
     get_running_loop,
+    sleep,
     wait,
 )
 from collections import Counter
@@ -65,6 +66,8 @@ def __init__(self, root: Component | Context[Any] | ContextProvider[Any]) -> Non
     async def __aenter__(self) -> Layout:
         # create attributes here to avoid access before entering context manager
         self._event_handlers: EventHandlerDict = {}
+        self._event_queues: dict[str, Queue[LayoutEventMessage | dict[str, Any]]] = {}
+        self._event_processing_tasks: dict[str, Task[None]] = {}
         self._render_tasks: set[Task[LayoutUpdateMessage]] = set()
         self._render_tasks_by_id: dict[
             _LifeCycleStateId, Task[LayoutUpdateMessage]
@@ -88,10 +91,18 @@ async def __aexit__(
             t.cancel()
             with suppress(CancelledError):
                 await t
+
+        for t in self._event_processing_tasks.values():
+            t.cancel()
+            with suppress(CancelledError):
+                await t
+
         await self._unmount_model_states([root_model_state])
 
         # delete attributes here to avoid access after exiting context manager
         del self._event_handlers
+        del self._event_queues
+        del self._event_processing_tasks
         del self._rendering_queue
         del self._render_tasks_by_id
         del self._root_life_cycle_state_id
@@ -103,20 +114,51 @@ async def deliver(self, event: LayoutEventMessage | dict[str, Any]) -> None:
         # associated with a backend model that has been deleted. We only handle
         # events if the element and the handler exist in the backend. Otherwise
         # we just ignore the event.
-        handler = self._event_handlers.get(event["target"])
-
-        if handler is not None:
-            try:
-                data = [Event(d) if isinstance(d, dict) else d for d in event["data"]]
-                await handler.function(data)
-            except Exception:
-                logger.exception(f"Failed to execute event handler {handler}")
-        else:
-            logger.info(
-                f"Ignored event - handler {event['target']!r} "
-                "does not exist or its component unmounted"
+        target = event["target"]
+        if target not in self._event_queues:
+            self._event_queues[target] = cast(
+                "Queue[LayoutEventMessage | dict[str, Any]]", Queue()
+            )
+            self._event_processing_tasks[target] = create_task(
+                self._process_event_queue(target, self._event_queues[target])
             )
 
+        await self._event_queues[target].put(event)
+
+        # In test environments, we yield to the event loop to let the processing tasks run.
+        if REACTPY_DEBUG.current:
+            await sleep(0)
+
+    async def _process_event_queue(
+        self, target: str, queue: Queue[LayoutEventMessage | dict[str, Any]]
+    ) -> None:
+        while True:
+            event = await queue.get()
+
+            # Retry a few times to handle potential re-render race conditions where
+            # the handler is temporarily removed and then re-added.
+            handler = self._event_handlers.get(target)
+            if handler is None:
+                for _ in range(3):
+                    await sleep(0.01)
+                    handler = self._event_handlers.get(target)
+                    if handler is not None:
+                        break
+
+            if handler is not None:
+                try:
+                    data = [
+                        Event(d) if isinstance(d, dict) else d for d in event["data"]
+                    ]
+                    await handler.function(data)
+                except Exception:
+                    logger.exception(f"Failed to execute event handler {handler}")
+            else:
+                logger.info(
+                    f"Ignored event - handler {event['target']!r} "
+                    "does not exist or its component unmounted"
+                )
+
     async def render(self) -> LayoutUpdateMessage:
         if REACTPY_ASYNC_RENDERING.current:
             return await self._parallel_render()
@@ -346,10 +388,11 @@ def _render_model_attributes(
 
         model_event_handlers = new_state.model.current["eventHandlers"] = {}
         for event, handler in handlers_by_event.items():
-            if event in old_state.targets_by_event:
-                target = old_state.targets_by_event[event]
+            if handler.target is not None:
+                target = handler.target
             else:
-                target = uuid4().hex if handler.target is None else handler.target
+                target = f"{new_state.key_path}:{event}"
+
             new_state.targets_by_event[event] = target
             self._event_handlers[target] = handler
             model_event_handlers[event] = {
@@ -370,7 +413,11 @@ def _render_model_event_handlers_without_old_state(
 
         model_event_handlers = new_state.model.current["eventHandlers"] = {}
         for event, handler in handlers_by_event.items():
-            target = uuid4().hex if handler.target is None else handler.target
+            if handler.target is not None:
+                target = handler.target
+            else:
+                target = f"{new_state.key_path}:{event}"
+
             new_state.targets_by_event[event] = target
             self._event_handlers[target] = handler
             model_event_handlers[event] = {
@@ -485,6 +532,7 @@ def _new_root_model_state(
         children_by_key={},
         targets_by_event={},
         life_cycle_state=_make_life_cycle_state(component, schedule_render),
+        key_path="",
     )
 
 
@@ -504,6 +552,7 @@ def _make_component_model_state(
         children_by_key={},
         targets_by_event={},
         life_cycle_state=_make_life_cycle_state(component, schedule_render),
+        key_path=f"{parent.key_path}/{key}" if parent else "",
     )
 
 
@@ -523,6 +572,7 @@ def _copy_component_model_state(old_model_state: _ModelState) -> _ModelState:
         children_by_key={},
         targets_by_event={},
         life_cycle_state=old_model_state.life_cycle_state,
+        key_path=old_model_state.key_path,
     )
 
 
@@ -546,6 +596,7 @@ def _update_component_model_state(
             if old_model_state.is_component_state
             else _make_life_cycle_state(new_component, schedule_render)
         ),
+        key_path=f"{new_parent.key_path}/{old_model_state.key}",
     )
 
 
@@ -562,6 +613,7 @@ def _make_element_model_state(
         patch_path=f"{parent.patch_path}/children/{index}",
         children_by_key={},
         targets_by_event={},
+        key_path=f"{parent.key_path}/{key}",
     )
 
 
@@ -575,9 +627,10 @@ def _update_element_model_state(
         index=new_index,
         key=old_model_state.key,
         model=Ref(),  # does not copy the model
-        patch_path=old_model_state.patch_path,
+        patch_path=f"{new_parent.patch_path}/children/{new_index}",
         children_by_key={},
         targets_by_event={},
+        key_path=f"{new_parent.key_path}/{old_model_state.key}",
     )
 
 
@@ -591,6 +644,7 @@ class _ModelState:
         "children_by_key",
         "index",
         "key",
+        "key_path",
         "life_cycle_state",
         "model",
         "patch_path",
@@ -607,6 +661,7 @@ def __init__(
         children_by_key: dict[Key, _ModelState],
         targets_by_event: dict[str, str],
         life_cycle_state: _LifeCycleState | None = None,
+        key_path: str = "",
     ):
         self.index = index
         """The index of the element amongst its siblings"""
@@ -626,6 +681,9 @@ def __init__(
         self.targets_by_event = targets_by_event
         """The element's event handler target strings indexed by their event name"""
 
+        self.key_path = key_path
+        """A slash-delimited path using element keys"""
+
         # === Conditionally Available Attributes ===
         # It's easier to conditionally assign than to force a null check on every usage
 

tests/test_core/test_events.py

@@ -1,8 +1,10 @@
+import asyncio
+from functools import partial
+
 import pytest
 
 import reactpy
-from functools import partial
-from reactpy import component, html
+from reactpy import component, html, use_state
 from reactpy.core.events import (
     EventHandler,
     merge_event_handler_funcs,
@@ -419,3 +421,171 @@ def handler(e: Event):
     eh = EventHandler(handler)
     assert eh.prevent_default is True
     assert eh.stop_propagation is True
+
+
+async def test_event_queue_sequential_processing(display: DisplayFixture):
+    """Ensure events are processed sequentially for the same target"""
+
+    events_processed = []
+
+    @component
+    def SequentialEvents():
+        async def handle_click(event):
+            # Simulate slow processing
+            await asyncio.sleep(0.1)
+            events_processed.append(event["target"])
+
+        return html.button({"id": "btn", "onClick": handle_click}, "Click me")
+
+    await display.show(SequentialEvents)
+
+    # Get the element
+    btn = display.page.locator("#btn")
+
+    # Click 3 times rapidly
+    # We use evaluate to trigger clicks rapidly from client side perspective if possible,
+    # or just click rapidly via playwright.
+    # Playwright's click is awaited, so we need to run them concurrently.
+
+    await asyncio.gather(
+        btn.click(),
+        btn.click(),
+        btn.click(),
+    )
+
+    # Wait for processing to complete (0.1s * 3 = 0.3s approx)
+    await asyncio.sleep(0.5)
+
+    assert len(events_processed) == 3
+
+
+async def test_event_targeting_with_shifting_elements(display: DisplayFixture):
+    """
+    Ensure that events are delivered to the correct component even when
+    elements shift around it, provided explicit keys are used.
+    """
+
+    clicked_items = []
+
+    @component
+    def Item(id_val):
+        async def handle_click(event):
+            clicked_items.append(id_val)
+
+        return html.div(
+            {"id": f"item-{id_val}", "onClick": handle_click}, f"Item {id_val}"
+        )
+
+    @component
+    def ListContainer():
+        items, set_items = use_state(["B", "C"])
+
+        def add_top(event):
+            set_items(["A", *items])
+
+        return html.div(
+            html.button({"id": "add-btn", "onClick": add_top}, "Add Top"),
+            html.div({"id": "list"}, [Item(i, key=i) for i in items]),
+        )
+
+    await display.show(ListContainer)
+
+    # Initial state: Items B, C are present.
+    # Click Item B.
+    btn_b = display.page.locator("#item-B")
+    await btn_b.click()
+
+    # Add Item A to the top.
+    add_btn = display.page.locator("#add-btn")
+    await add_btn.click()
+
+    # Wait for Item A to appear to ensure render is complete
+    await display.page.locator("#item-A").wait_for()
+
+    # Now the list is [A, B, C].
+    # Item B has shifted position in the DOM (index 0 -> index 1).
+    # Its key path should remain .../B regardless of index if we implemented it right?
+    # Actually, let's verify how key_path is constructed.
+    # In layout.py: key_path=f"{parent.key_path}/{key}"
+    # So if the parent is the div container, and items have keys "A", "B", "C".
+    # The paths are .../list/A, .../list/B, .../list/C.
+    # The index in the children array changes, but the key_path relies on the key, not the index (if key is provided).
+
+    # Click Item B again.
+    # It should still trigger the handler for B, not A (which is now at index 0) and not C.
+    await btn_b.click()
+
+    # Click Item C.
+    btn_c = display.page.locator("#item-C")
+    await btn_c.click()
+
+    # Assertions
+    # We expect 'B' (first click), then 'B' (second click after shift), then 'C'.
+    assert clicked_items == ["B", "B", "C"]
+
+
+async def test_event_targeting_with_index_shifting(display: DisplayFixture):
+    """
+    Ensure that when keys are NOT provided (using indices),
+    events might target the element at the same *index* if the user isn't careful,
+    but we verify that the system behaves predictably (target is based on path).
+
+    If we insert at top without keys:
+    Old: Index 0 (Item B) -> Path .../0
+    New: Index 0 (Item A), Index 1 (Item B) -> Path .../0 turns into Item A.
+
+    If an event was in-flight for Index 0 (Item B) when the update happened:
+    The event target ID was ".../0:click".
+    After update, ".../0:click" is now Item A's handler.
+
+    So the event intended for B would execute on A. This is standard React behavior for index keys.
+    We just want to ensure our system works this way and doesn't crash or lose the event.
+    """
+
+    clicked_items = []
+
+    @component
+    def Item(id_val):
+        async def handle_click(event):
+            clicked_items.append(id_val)
+
+        return html.div(
+            {"id": f"item-{id_val}", "onClick": handle_click}, f"Item {id_val}"
+        )
+
+    @component
+    def ListContainer():
+        items, set_items = use_state(["B"])
+
+        async def add_top(event):
+            set_items(["A", *items])
+            # We want to create a race condition where we click Item B (index 0)
+            # just narrowly before the re-render places Item A at index 0.
+            # But 'display.show' and playwright interactions are sequential usually.
+            # We can simulate the state change.
+
+        return html.div(
+            html.button({"id": "add-btn", "onClick": add_top}, "Add Top"),
+            html.div({"id": "list"}, [Item(i) for i in items]),
+        )
+
+    await display.show(ListContainer)
+
+    # Initial: Item B at Index 0.
+    # We want to send an event to Index 0 *effectively*, but have it process *after* A is inserted at Index 0.
+    # This is hard to orchestrate with exact timing in an integration test without hooks into the internal loop.
+    # However, we can verifying that basic interaction works after the shift.
+
+    add_btn = display.page.locator("#add-btn")
+    await add_btn.click()
+
+    await display.page.locator("#item-A").wait_for()
+
+    # Now Item A is at Index 0 (".../0"). Item B is at Index 1 (".../1").
+    # Clicking Item B should technically work fine because we are clicking the DOM element for B,
+    # which should generate an event for target ".../1".
+
+    btn_b = display.page.locator("#item-B")
+    await btn_b.click()  # This generates event for .../1
+
+    assert clicked_items == ["B"]