Set a controlled widget back after an event that caused a render

reacton/core.py

@@ -215,6 +215,65 @@ def wrapper(*args, **kwargs):
     return wrapper
 
 
+def _has_elements(value) -> bool:
+    if isinstance(value, Element):
+        return True
+    if isinstance(value, (list, tuple)):
+        return any(_has_elements(v) for v in value)
+    if isinstance(value, dict):
+        return any(_has_elements(v) for v in value.values())
+    return False
+
+
+class _TraitListener:
+    """Calls the on_<name> handler of an element when the trait <name> of its widget changes.
+
+    When the handler caused a render, the trait is set back to what the widget's current
+    element says afterwards. The default renderer does that for every widget in a render
+    (it walks the whole tree), but the fast renderer skips components that did not change.
+    Without this, a handler that leaves its own state as it is (for instance it upper-cases
+    "Bb" to the "BB" we already have) but changes other state, leaves the widget showing
+    "Bb" in the fast renderer.
+
+    When nothing rendered, the widget keeps what the user entered, in both renderers:
+    solara's inputs with continuous_update=False rely on this, they pass the old value and
+    take the new one on blur.
+    """
+
+    def __init__(self, element: "Element", widget: widgets.Widget, name: str, callback: Callable):
+        # the element that currently owns the widget, see Element._update_widget_event_listener
+        self.element = element
+        self.widget = widget
+        self.name = name
+        self.trait_name = name[3:]
+        self.callback = _event_handler_exception_wrapper(callback)
+        self.rc = get_render_context()
+        self.removed = False
+
+    def __call__(self, change):
+        if are_events_supressed():
+            return
+        logger.info("event %r on %r with %r", self.name, self.widget, change)
+        render_count = self.rc.render_count
+        self.callback(change["new"])
+        if self.rc.render_count != render_count:
+            self._set_back()
+
+    def _set_back(self):
+        kwargs = self.element.kwargs
+        if self.removed or self.trait_name not in kwargs or self.rc._is_rendering or self.widget.comm is None:
+            # the render replaced this listener (and has set the new values), the element does
+            # not give the value (not controlled), a render in progress will reconcile the
+            # widget, or the render removed the widget
+            return
+        value = kwargs[self.trait_name]
+        if _has_elements(value):
+            # we would need the widgets these elements resolve to
+            return
+        with self.widget.hold_sync(), suppress_events():
+            setattr(self.widget, self.trait_name, value)
+
+
 def join_key(parent_key, key):
     return f"{parent_key}{key}"
 
@@ -557,17 +616,15 @@ def _update_widget_event_listener(self, widget: widgets.Widget, name: str, callb
             self._remove_widget_event_listener(widget, name, callback_prev)
         if callback is not None and callback != callback_prev:
             self._add_widget_event_listener(widget, name, callback)
+        elif callback is not None:
+            # the same callback: the listener stays, but it should read this element from now on
+            listener = self._callback_wrappers.get((widget.model_id, name, callback))
+            if isinstance(listener, _TraitListener):
+                listener.element = self
 
     def _add_widget_event_listener(self, widget: widgets.Widget, name: str, callback: Callable):
         target_name = name[3:]
-        callback_exception_safe = _event_handler_exception_wrapper(callback)
-
-        def on_change(change):
-            if are_events_supressed():
-                return
-            logger.info("event %r on %r with %r", name, widget, change)
-            callback_exception_safe(change["new"])
-
+        on_change = _TraitListener(self, widget, name, callback)
         key = (widget.model_id, name, callback)
         self._callback_wrappers[key] = on_change
         widget.observe(on_change, target_name)
@@ -577,6 +634,10 @@ def _remove_widget_event_listener(self, widget: widgets.Widget, name: str, callb
         key = (widget.model_id, name, callback)
         on_change = self._callback_wrappers[key]
         del self._callback_wrappers[key]
+        if isinstance(on_change, _TraitListener):
+            # it may be running now (its handler caused this render): a new listener or
+            # no listener is in charge of the widget from now on
+            on_change.removed = True
         try:
             widget.unobserve(on_change, target_name)
         except ValueError:

reacton/core_test.py

@@ -3794,3 +3794,144 @@ def App():
     set_offset(0)
     assert [child.description for child in vbox.children[1:]] == ["0", "1"]
     rc.close()
+
+
+# A controlled widget (the element gives the value, a handler maps a new value to state)
+# must show the value of its element after an event that caused a render, in both
+# renderers. Without a render, the widget keeps what the user entered.
+# See https://github.com/vuejs/vue/issues/13237 for the same problem in Vue.
+
+
+def test_controlled_widget_no_render_keeps_user_value():
+    # solara's inputs with continuous_update=False pass the old value, ignore v_model
+    # changes, and take the new value on blur: typing must not snap back
+    @react.component
+    def Test():
+        value, set_value = react.use_state("AA")
+        # add layout to make sure kwargs are transformed from elements to widgets
+        return w.Text(value=value, on_value=lambda new_value: None, layout=w.Layout(width="100%"))
+
+    box, rc = react.render(Test(), handle_error=False)
+    text = rc.find(widgets.Text).widget
+    text.value = "Bb"
+    assert text.value == "Bb"
+    rc.close()
+
+
+def test_controlled_widget_no_state_change_parent_renders():
+    # the parent renders on every event, and re-renders the child with the same props
+
+    @react.component
+    def UpperCaseText(on_value):
+        value, set_value = react.use_state("AA")
+
+        def on_value_self(new_value):
+            set_value(new_value.upper())
+            on_value(new_value)
+
+        return w.Text(value=value, on_value=on_value_self)
+
+    @react.component
+    def Test():
+        count, set_count = react.use_state(0)
+        return UpperCaseText(on_value=lambda value: set_count(lambda count: count + 1))
+
+    box, rc = react.render(Test(), handle_error=False)
+    text = rc.find(widgets.Text).widget
+    text.value = "bb"
+    assert text.value == "BB"
+    text.value = "Bb"
+    assert text.value == "BB"
+    rc.close()
+
+
+@pytest.mark.parametrize("layout", [False, True])
+def test_controlled_widget_no_state_change_sibling_renders(layout):
+    # the handler changes the state of a sibling component: a render happens, but it does
+    # not re-render the component of the widget (the fast renderer skips it)
+    set_count = lambda x: None  # noqa
+
+    @react.component
+    def Counter():
+        nonlocal set_count
+        count, set_count = react.use_state(0)
+        return w.Label(value=f"events: {count}")
+
+    @react.component
+    def UpperCaseText():
+        value, set_value = react.use_state("AA")
+
+        def on_value(new_value):
+            set_value(new_value.upper())
+            set_count(lambda count: count + 1)
+
+        kwargs = {"layout": w.Layout(width="100%")} if layout else {}
+        return w.Text(value=value, on_value=on_value, **kwargs)
+
+    @react.component
+    def Test():
+        return w.VBox(children=[UpperCaseText(), Counter()])
+
+    box, rc = react.render(Test(), handle_error=False)
+    text = rc.find(widgets.Text).widget
+    label = rc.find(widgets.Label).widget
+    text.value = "bb"
+    assert text.value == "BB"
+    text.value = "Bb"
+    assert text.value == "BB"
+    assert label.value == "events: 2"
+    rc.close()
+
+
+def test_controlled_widget_same_callback_new_value():
+    # the handler is the same object every render, so its listener stays: it must set
+    # back the value of the new element, not of the element that added the listener
+    set_tens = lambda x: None  # noqa
+
+    def on_value(value):
+        set_tens(value // 10)
+
+    @react.component
+    def Test():
+        nonlocal set_tens
+        tens, set_tens = react.use_state(1)
+        return w.IntText(value=tens * 10, on_value=on_value)
+
+    box, rc = react.render(Test(), handle_error=False)
+    text = rc.find(widgets.IntText).widget
+    assert text.value == 10
+    text.value = 25
+    assert text.value == 20
+    rc.close()
+
+
+def test_uncontrolled_widget_keeps_its_value():
+    # without a value from the element, the widget keeps what the user entered
+    values: List[str] = []
+
+    @react.component
+    def Test():
+        return w.Text(on_value=values.append)
+
+    box, rc = react.render(Test(), handle_error=False)
+    text = rc.find(widgets.Text).widget
+    text.value = "Bb"
+    assert text.value == "Bb"
+    assert values == ["Bb"]
+    rc.close()
+
+
+def test_controlled_widget_removed_by_handler():
+    @react.component
+    def Test():
+        show, set_show = react.use_state(True)
+        if show:
+            return w.VBox(children=[w.Text(value="keep", on_value=lambda value: set_show(False))])
+        return w.VBox(children=[w.Label(value="gone")])
+
+    box, rc = react.render(Test(), handle_error=False)
+    text = rc.find(widgets.Text).widget
+    text.value = "other"
+    rc.find(widgets.Label, value="gone").assert_single()
+    assert text.comm is None  # closed
+    rc.close()