From e565744d92e1195da7a27b5c424d4230456474d1 Mon Sep 17 00:00:00 2001 From: Maarten Breddels Date: Fri, 25 Sep 2026 21:35:48 +0200 Subject: [PATCH] Do not re-set container children that resolve to the same widgets The fast renderer skips the update of a widget when its element is the same object as last render and its kwargs resolve to the same values. But it compared the resolved kwargs (elements replaced by widgets) with the element kwargs (still elements), so for every container the compare failed and its children were assigned again. A leaf update next to 300 rows re-set the children of the VBox (~170 us for 301 real widgets), a root update re-set the children of all 300 HBoxes. For an unchanged element whose kwargs hold elements, we now compare the resolved kwargs with the values the widget holds. When they are the same objects, setting them is a no-op, so skipping cannot change what the user sees. A value changed from the frontend is still set back, as before, and there is no per-widget cache that could go stale. Co-Authored-By: Claude Opus 5.5 (1M context) --- reacton/core.py | 36 +++++++- reacton/core_test.py | 210 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 243 insertions(+), 3 deletions(-) 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()