Fix issue from #1081 (#1085)

* identify issue from #1081

* fix the bug

* update doc

* make ruff happy

* add changelog entry

Author: rmorshea

docs/source/about/changelog.rst

@@ -31,6 +31,7 @@ Unreleased
 
 - :issue:`930` - better traceback for JSON serialization errors (via :pull:`1008`)
 - :issue:`437` - explain that JS component attributes must be JSON (via :pull:`1008`)
+- :issue:`1086` - fix rendering bug when children change positions (via :pull:`1085`)
 
 
 v1.0.0

src/py/reactpy/pyproject.toml

@@ -139,6 +139,7 @@ testpaths = "tests"
 xfail_strict = true
 python_files = "*asserts.py test_*.py"
 asyncio_mode = "auto"
+log_cli_level = "INFO"
 
 # --- MyPy -----------------------------------------------------------------------------
 

src/py/reactpy/reactpy/core/layout.py

@@ -489,7 +489,7 @@ def _update_component_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={},
         life_cycle_state=(

src/py/reactpy/reactpy/utils.py

@@ -27,7 +27,7 @@ class Ref(Generic[_RefValue]):
         You can compare the contents for two ``Ref`` objects using the ``==`` operator.
     """
 
-    __slots__ = "current"
+    __slots__ = ("current",)
 
     def __init__(self, initial_value: _RefValue = _UNDEFINED) -> None:
         if initial_value is not _UNDEFINED:

src/py/reactpy/tests/test_core/test_layout.py

@@ -13,15 +13,19 @@
 from reactpy.core.component import component
 from reactpy.core.hooks import use_effect, use_state
 from reactpy.core.layout import Layout
+from reactpy.core.types import State
 from reactpy.testing import (
     HookCatcher,
     StaticEventHandler,
     assert_reactpy_did_log,
     capture_reactpy_logs,
 )
 from reactpy.utils import Ref
+from tests.tooling import select
 from tests.tooling.common import event_message, update_message
 from tests.tooling.hooks import use_force_render, use_toggle
+from tests.tooling.layout import layout_runner
+from tests.tooling.select import element_exists, find_element
 
 
 @pytest.fixture(autouse=True)
@@ -1190,3 +1194,59 @@ def Child():
         done, pending = await asyncio.wait([render_task], timeout=0.1)
         assert not done and pending
         render_task.cancel()
+
+
+async def test_ensure_model_path_udpates():
+    """
+    This is regression test for a bug in which we failed to update the path of a bug
+    that arose when the "path" of a component within the overall model was not updated
+    when the component changes position amongst its siblings. This meant that when
+    a component whose position had changed would attempt to update the view at its old
+    position.
+    """
+
+    @component
+    def Item(item: str, all_items: State[list[str]]):
+        color = use_state(None)
+
+        def deleteme(event):
+            all_items.set_value([i for i in all_items.value if (i != item)])
+
+        def colorize(event):
+            color.set_value("blue" if not color.value else None)
+
+        return html.div(
+            {"id": item, "color": color.value},
+            html.button({"on_click": colorize}, f"Color {item}"),
+            html.button({"on_click": deleteme}, f"Delete {item}"),
+        )
+
+    @component
+    def App():
+        items = use_state(["A", "B", "C"])
+        return html._([Item(item, items, key=item) for item in items.value])
+
+    async with layout_runner(reactpy.Layout(App())) as runner:
+        tree = await runner.render()
+
+        # Delete item B
+        b, b_info = find_element(tree, select.id_equals("B"))
+        assert b_info.path == (0, 1, 0)
+        b_delete, _ = find_element(b, select.text_equals("Delete B"))
+        await runner.trigger(b_delete, "on_click", {})
+
+        tree = await runner.render()
+
+        # Set color of item C
+        assert not element_exists(tree, select.id_equals("B"))
+        c, c_info = find_element(tree, select.id_equals("C"))
+        assert c_info.path == (0, 1, 0)
+        c_color, _ = find_element(c, select.text_equals("Color C"))
+        await runner.trigger(c_color, "on_click", {})
+
+        tree = await runner.render()
+
+        # Ensure position and color of item C are correct
+        c, c_info = find_element(tree, select.id_equals("C"))
+        assert c_info.path == (0, 1, 0)
+        assert c["attributes"]["color"] == "blue"

src/py/reactpy/tests/tooling/layout.py

@@ -0,0 +1,44 @@
+from __future__ import annotations
+
+import logging
+from collections.abc import AsyncIterator
+from contextlib import asynccontextmanager
+from typing import Any
+
+from jsonpointer import set_pointer
+
+from reactpy.core.layout import Layout
+from reactpy.core.types import VdomJson
+from tests.tooling.common import event_message
+
+logger = logging.getLogger(__name__)
+
+
+@asynccontextmanager
+async def layout_runner(layout: Layout) -> AsyncIterator[LayoutRunner]:
+    async with layout:
+        yield LayoutRunner(layout)
+
+
+class LayoutRunner:
+    def __init__(self, layout: Layout) -> None:
+        self.layout = layout
+        self.model = {}
+
+    async def render(self) -> VdomJson:
+        update = await self.layout.render()
+        logger.info(f"Rendering element at {update['path'] or '/'!r}")
+        if not update["path"]:
+            self.model = update["model"]
+        else:
+            self.model = set_pointer(
+                self.model, update["path"], update["model"], inplace=False
+            )
+        return self.model
+
+    async def trigger(self, element: VdomJson, event_name: str, *data: Any) -> None:
+        event_handler = element.get("eventHandlers", {}).get(event_name, {})
+        logger.info(f"Triggering {event_name!r} with target {event_handler['target']}")
+        if not event_handler:
+            raise ValueError(f"Element has no event handler for {event_name}")
+        await self.layout.deliver(event_message(event_handler["target"], *data))

src/py/reactpy/tests/tooling/select.py

@@ -0,0 +1,107 @@
+from __future__ import annotations
+
+from collections.abc import Iterator, Sequence
+from dataclasses import dataclass
+from typing import Callable
+
+from reactpy.core.types import VdomJson
+
+Selector = Callable[[VdomJson, "ElementInfo"], bool]
+
+
+def id_equals(id: str) -> Selector:
+    return lambda element, _: element.get("attributes", {}).get("id") == id
+
+
+def class_equals(class_name: str) -> Selector:
+    return (
+        lambda element, _: class_name
+        in element.get("attributes", {}).get("class", "").split()
+    )
+
+
+def text_equals(text: str) -> Selector:
+    return lambda element, _: _element_text(element) == text
+
+
+def _element_text(element: VdomJson) -> str:
+    if isinstance(element, str):
+        return element
+    return "".join(_element_text(child) for child in element.get("children", []))
+
+
+def element_exists(element: VdomJson, selector: Selector) -> bool:
+    return next(find_elements(element, selector), None) is not None
+
+
+def find_element(
+    element: VdomJson,
+    selector: Selector,
+    *,
+    first: bool = False,
+) -> tuple[VdomJson, ElementInfo]:
+    """Find an element by a selector.
+
+    Parameters:
+        element:
+            The tree to search.
+        selector:
+            A function that returns True if the element matches.
+        first:
+            If True, return the first element found. If False, raise an error if
+            multiple elements are found.
+
+    Returns:
+        Element info, or None if not found.
+    """
+    find_iter = find_elements(element, selector)
+    found = next(find_iter, None)
+    if found is None:
+        raise ValueError("Element not found")
+    if not first:
+        try:
+            next(find_iter)
+            raise ValueError("Multiple elements found")
+        except StopIteration:
+            pass
+    return found
+
+
+def find_elements(
+    element: VdomJson, selector: Selector
+) -> Iterator[tuple[VdomJson, ElementInfo]]:
+    """Find an element by a selector.
+
+    Parameters:
+        element:
+            The tree to search.
+        selector:
+            A function that returns True if the element matches.
+
+    Returns:
+        Element info, or None if not found.
+    """
+    return _find_elements(element, selector, (), ())
+
+
+def _find_elements(
+    element: VdomJson,
+    selector: Selector,
+    parents: Sequence[VdomJson],
+    path: Sequence[int],
+) -> tuple[VdomJson, ElementInfo] | None:
+    info = ElementInfo(parents, path)
+    if selector(element, info):
+        yield element, info
+
+    for index, child in enumerate(element.get("children", [])):
+        if isinstance(child, dict):
+            yield from _find_elements(
+                child, selector, (*parents, element), (*path, index)
+            )
+
+
+@dataclass
+class ElementInfo:
+    parents: Sequence[VdomJson]
+    path: Sequence[int]