diff --git a/reacton/core.py b/reacton/core.py index 93441c8..158e5e9 100644 --- a/reacton/core.py +++ b/reacton/core.py @@ -255,6 +255,34 @@ def _values_identical(a, b): return False +_missing = object() + + +def _widget_holds_values(widget: widgets.Widget, kwargs: Dict[str, Any]) -> bool: + """Does the widget already hold these (resolved) kwargs, so that setting them is a no-op? + + Only for the same element as last render: its event listeners are the same callbacks, + and no argument was dropped. Compares by identity, where a list and a tuple with the + same items count as the same (a Tuple trait stores a list as a tuple). + """ + for name, value in kwargs.items(): + if name.startswith("on_") and not widget.has_trait(name): + continue + if not _value_held(value, getattr(widget, name, _missing)): + return False + return True + + +def _value_held(value, held) -> bool: + if value is held: + return True + if isinstance(value, (list, tuple)): + return type(held) in (list, tuple) and len(value) == len(held) and all(_value_held(x, y) for x, y in zip(value, held)) + if type(value) is dict: + return type(held) is dict and len(value) == len(held) and all(k in held and _value_held(v, held[k]) for k, v in value.items()) + return False + + def _with_tracebacks(e, tracebacks): # copy it, and we need with_traceback for unknown reasons not to cause # an infinite loop @@ -2798,15 +2826,17 @@ def _reconsolidate(self, el: Element, default_key: str, parent_key: str): # update the existing widget in place kwargs = self._visit_children_values(el.kwargs, key, parent_key, self._reconsolidate) if not context.exceptions_children: - if el is not el_prev or not _values_identical(kwargs, el.kwargs): + # the same element whose kwargs hold no elements: nothing can have changed. + # With elements (a container), the kwargs resolve to widgets, so compare + # them with what the widget holds: equal means setting them is a no-op + # (and a value changed from the frontend is still set back) + if el is not el_prev or not (_values_identical(kwargs, el.kwargs) or _widget_holds_values(widget_previous, kwargs)): try: el._update_widget(widget_previous, el_prev, kwargs) except BaseException as e: context.exceptions_self.append(e) self._set_rerender_needed("Exception ocurred during reconciliation (updating widget)") _mark_needs_render_ancestors(context) - # else: identical element and all children reconciled to the - # same widgets, nothing can have changed self._store_widget(context, el, key, widget_previous) else: assert el_prev is not None, "widget_previous is not None, but el_prev is" diff --git a/reacton/core_test.py b/reacton/core_test.py index 9e24e35..cd049ce 100644 --- a/reacton/core_test.py +++ b/reacton/core_test.py @@ -3534,3 +3534,213 @@ def cleanup(): finally: thread.join(5) assert not thread.is_alive() + + +# Tests for the update path: what a state change re-renders and which widgets it touches. +# Some of these pin properties of the fast renderer only (REACTON_FAST=1); the default +# renderer walks and updates the whole tree on every render. +fast_renderer_only = pytest.mark.skipif(core._render_context_class() is not core._RenderContextFast, reason="a property of the fast renderer (REACTON_FAST=1)") + + +class UpdateSpy: + """Records the widgets that get a (re)assignment of their kwargs via Element._update_widget.""" + + def __init__(self): + self.updated: List[widgets.Widget] = [] + + def __enter__(self): + original = core.Element._update_widget + spy = self + + def _update_widget(self, widget, el_prev, kwargs): + spy.updated.append(widget) + return original(self, widget, el_prev, kwargs) + + self._patch = unittest.mock.patch.object(core.Element, "_update_widget", _update_widget) + self._patch.__enter__() + return self + + def __exit__(self, *args): + self._patch.__exit__(*args) + + def types(self): + return sorted(type(widget).__name__ for widget in self.updated) + + +@fast_renderer_only +def test_leaf_update_does_not_update_sibling_containers(): + set_value = lambda x: None # noqa + + @react.component + def Row(i): + return w.HBox(children=[w.Button(description=f"button-{i}"), w.Label(value=f"label-{i}")]) + + @react.component + def Leaf(): + nonlocal set_value + value, set_value = react.use_state(0) + return w.Button(description=f"leaf-{value}") + + @react.component + def App(): + # the HBox is a container next to the leaf, in the same (not re-rendered) component + return w.VBox(children=[w.HBox(children=[w.Label(value="sibling")]), Row(0), Row(1), Leaf()]) + + vbox, rc = react.render_fixed(App(), handle_error=False) + children_before = vbox.children + with UpdateSpy() as spy: + set_value(1) + assert rc.find(widgets.Button, description="leaf-1").widget is vbox.children[-1] + # only the leaf button gets new kwargs, the containers keep their children + assert spy.types() == ["Button"] + assert vbox.children == children_before + rc.close() + + +def test_container_updates_when_child_widget_changes(Container): + # the component holding the containers does not re-render, but the root widget of + # a child component changes type: the container must get the new widget + setters = {} + + @react.component + def Switch(name): + label, set_label = react.use_state(False) + setters[name] = set_label + if label: + return w.Label(value=name) + return w.Button(description=name) + + @react.component + def App(): + return w.VBox(children=[Container(children=[w.Button(description="sibling"), Switch("inner")]), Switch("outer")]) + + def describe(widget): + return (type(widget).__name__, widget.value if isinstance(widget, widgets.Label) else widget.description) + + vbox, rc = react.render_fixed(App(), handle_error=False) + box = vbox.children[0] + assert isinstance(box, widgets.HBox) + assert [describe(child) for child in box.children] == [("Button", "sibling"), ("Button", "inner")] + assert describe(vbox.children[1]) == ("Button", "outer") + + setters["outer"](True) + assert vbox.children[0] is box + assert describe(vbox.children[1]) == ("Label", "outer") + + setters["inner"](True) + assert vbox.children[0] is box + assert [describe(child) for child in box.children] == [("Button", "sibling"), ("Label", "inner")] + label = box.children[1] + + setters["inner"](False) + assert [describe(child) for child in box.children] == [("Button", "sibling"), ("Button", "inner")] + assert label.comm is None # closed + setters["outer"](False) + assert describe(vbox.children[1]) == ("Button", "outer") + rc.close() + + +def test_container_updates_when_fragment_child_changes(): + # a child component returns a fragment: its widgets are spliced into the parent + # container, which must follow when the fragment changes + set_count = lambda x: None # noqa + + @react.component + def Items(): + nonlocal set_count + count, set_count = react.use_state(1) + return reacton.Fragment(children=[w.Button(description=str(i)) for i in range(count)]) + + @react.component + def App(): + return w.VBox(children=[w.Label(value="first"), Items(), w.Label(value="last")]) + + def describe(vbox): + return [child.value if isinstance(child, widgets.Label) else child.description for child in vbox.children] + + vbox, rc = react.render_fixed(App(), handle_error=False) + assert describe(vbox) == ["first", "0", "last"] + set_count(3) + assert describe(vbox) == ["first", "0", "1", "2", "last"] + set_count(0) + assert describe(vbox) == ["first", "last"] + set_count(2) + assert describe(vbox) == ["first", "0", "1", "last"] + rc.close() + + +def test_same_element_sets_back_changes_from_outside(): + # the elements below are not re-created (App does not re-render), but a sibling update + # walks them: a value changed from the frontend (or by hand) is set back, like before + # the fast renderer skipped containers whose children resolve to the same widgets + set_value = lambda x: None # noqa + + @react.component + def Leaf(): + nonlocal set_value + value, set_value = react.use_state(0) + return w.Button(description=f"leaf-{value}") + + @react.component + def App(): + return w.VBox(children=[w.HBox(children=[w.Label(value="child")]), w.IntSlider(value=5, layout=w.Layout(width="100px")), Leaf()]) + + vbox, rc = react.render_fixed(App(), handle_error=False) + hbox, slider = vbox.children[:2] + label = hbox.children[0] + hbox.children = () + slider.value = 9 + set_value(1) + assert hbox.children == (label,) + assert slider.value == 5 + rc.close() + + +def test_same_element_updates_when_slot_child_changes(): + # an element nested in a list of dicts (like v_slots) that now resolves to a new widget + set_label = lambda x: None # noqa + + @react.component + def Activator(): + nonlocal set_label + label, set_label = react.use_state(False) + return w.Label(value="label") if label else w.Button(description="button") + + @react.component + def App(): + return w.VBox(children=[v.Menu(v_slots=[{"name": "activator", "children": Activator()}])]) + + vbox, rc = react.render_fixed(App(), handle_error=False) + menu = vbox.children[0] + assert isinstance(menu.v_slots[0]["children"], widgets.Button) + set_label(True) + assert vbox.children[0] is menu + assert isinstance(menu.v_slots[0]["children"], widgets.Label) + set_label(False) + assert isinstance(menu.v_slots[0]["children"], widgets.Button) + rc.close() + + +def test_container_updates_when_fragment_children_are_replaced(): + # the fragment keeps its length, but its widgets change: the container must follow + set_offset = lambda x: None # noqa + + @react.component + def Items(): + nonlocal set_offset + offset, set_offset = react.use_state(0) + return reacton.Fragment(children=[w.Button(description=str(i + offset)).key(f"item-{i + offset}") for i in range(2)]) + + @react.component + def App(): + return w.VBox(children=[w.Label(value="first"), Items()]) + + vbox, rc = react.render_fixed(App(), handle_error=False) + first_button = vbox.children[1] + assert [child.description for child in vbox.children[1:]] == ["0", "1"] + set_offset(1) + assert [child.description for child in vbox.children[1:]] == ["1", "2"] + assert first_button not in vbox.children + set_offset(0) + assert [child.description for child in vbox.children[1:]] == ["0", "1"] + rc.close()