Skip to content

Fix issue from #1081 #1085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/source/about/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/py/reactpy/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ testpaths = "tests"
xfail_strict = true
python_files = "*asserts.py test_*.py"
asyncio_mode = "auto"
log_cli_level = "INFO"

# --- MyPy -----------------------------------------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion src/py/reactpy/reactpy/core/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=(
Expand Down
2 changes: 1 addition & 1 deletion src/py/reactpy/reactpy/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
60 changes: 60 additions & 0 deletions src/py/reactpy/tests/test_core/test_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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"
44 changes: 44 additions & 0 deletions src/py/reactpy/tests/tooling/layout.py
Original file line number Diff line number Diff line change
@@ -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))
107 changes: 107 additions & 0 deletions src/py/reactpy/tests/tooling/select.py
Original file line number Diff line number Diff line change
@@ -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]