diff --git a/reacton/core.py b/reacton/core.py index 4937bea..7085a5f 100644 --- a/reacton/core.py +++ b/reacton/core.py @@ -1327,6 +1327,11 @@ def __init__(self, element: Element, container: widgets.Widget = None, children_ # when set, the next render phase walks the whole tree instead of # skipping subtrees in which no state changed (see _render) self._walk_all = True + # set (per thread) while a render runs because state changed: such a render does not + # force a full walk, an explicit render() does (see render) + self._state_render = threading.local() + # fast renderer: this render is forced, so reconciliation applies all widget kwargs again + self._reconsolidate_forced_walk = False if initial_state: self.state_set(self.context_root, initial_state) @@ -1591,7 +1596,11 @@ def update(self, element: Element): def _possible_rerender(self): if not self._is_rendering and self._batch_counter.current() == 0: - self.render(self.element, self.container) + self._state_render.active = True + try: + self.render(self.element, self.container) + finally: + self._state_render.active = False else: logger.info("No render phase triggered, already rendering") @@ -1649,6 +1658,10 @@ def render(self, element: Element, container: widgets.Widget = None): # torn down, there is nothing to render into anymore logger.info("Render requested on a closing/closed render context, ignoring") return container + if not getattr(self._state_render, "active", False): + # an explicit render(), like force_update() and update(), walks the whole tree; + # set under the lock, so a render on another thread cannot reset it + self._walk_all = True prev_rc = getattr(local, "rc", None) # an exception that escapes while this is True aborted a render pass (see the except below) in_render_phase = True @@ -1803,6 +1816,8 @@ def format(reason: RerenderReason): finally: local.rc = prev_rc # type: ignore self._is_rendering = False + # a forced render that raised before reconciliation must not force the next one + self._reconsolidate_forced_walk = False # clear before the lock is released: a stale _lock_thread makes the # recursion guard above fire for a thread that merely rendered last, # while a *different* thread holds the lock (false "Recursive render") @@ -2518,6 +2533,8 @@ def _render(self, element: Element, default_key: str, parent_key: str): self._old_element_ids.add(id(element)) context = self.context assert context is not None + if parent_key == ROOT_KEY and default_key == "/" and context is self.context_root and self._walk_all: + self._reconsolidate_forced_walk = True if default_key == "/": # the root element of a component determines which keys are in use, @@ -2857,7 +2874,11 @@ def _reconsolidate(self, el: Element, default_key: str, parent_key: str): # 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)): + if ( + self._reconsolidate_forced_walk + or 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: @@ -2923,6 +2944,10 @@ def _reconsolidate(self, el: Element, default_key: str, parent_key: str): self._shared_elements.add(el) assert el in self._shared_elements_next self._shared_elements_next.remove(el) + if parent_key == ROOT_KEY and default_key == "/" and context is self.context_root: + # only the end of the root element; a child component with key "" also + # gets here with these keys, in its own context + self._reconsolidate_forced_walk = False def _process_effects(self, child_context: "ComponentContext", context: "ComponentContext"): # NOTE: effect/cleanup exceptions are recorded on the context of the diff --git a/reacton/core_test.py b/reacton/core_test.py index fb4b9cb..486dbe1 100644 --- a/reacton/core_test.py +++ b/reacton/core_test.py @@ -4088,3 +4088,159 @@ def cleanup(): assert box.children[0].value == "value 1" finally: rc.close() + + +def test_rc_render_same_element_reasserts_widget_kwargs(): + @react.component + def App(): + return w.IntSlider(value=1) + + slider, rc = react.render_fixed(App(), handle_error=False) + slider.value = 7 + rc.render(rc.element) + assert slider.value == 1 + rc.close() + + +def test_force_update_reasserts_widget_kwargs(): + @react.component + def App(): + return w.IntSlider(value=1) + + slider, rc = react.render_fixed(App(), handle_error=False) + slider.value = 7 + rc.force_update() + assert slider.value == 1 + rc.close() + + +@fast_renderer_only +def test_state_render_after_failed_forced_render_is_not_forced(): + # only a forced render applies the element kwargs again; one that raised before + # reconciliation must not turn the next state-triggered render into a forced one + set_count = lambda x: None # noqa + fail = [False] + + @react.component + def App(): + nonlocal set_count + count, set_count = react.use_state(0) + # the same element every render: an unforced render keeps the widget as it is + slider = react.use_memo(lambda: w.IntSlider(value=1), []) + if fail[0]: + raise ValueError("boom") + return w.VBox(children=[slider, w.Button(description=str(count))]) + + box, rc = react.render_fixed(App(), handle_error=False) + slider = rc.find(widgets.IntSlider).widget + fail[0] = True + with pytest.raises(ValueError, match="boom"): + set_count(1) + # App still needs a render, so the forced render runs it and raises again + with pytest.raises(ValueError, match="boom"): + rc.force_update() + fail[0] = False + slider.value = 7 + set_count(2) + assert rc.find(widgets.Button).widget.description == "2" + assert slider.value == 7 + rc.force_update() + assert slider.value == 1 + rc.close() + + +def test_forced_render_waiting_for_a_state_render_on_another_thread(): + # the explicit render waits for the lock while a state-triggered render runs, and the + # end of that render must not undo the request for a forced walk + entered = threading.Event() + release = threading.Event() + set_count = lambda x: None # noqa + + @react.component + def Slow(count): + if count == 1: + entered.set() + assert release.wait(5) + return w.Label(value=str(count)) + + @react.component + def App(): + nonlocal set_count + count, set_count = react.use_state(0) + slider = react.use_memo(lambda: w.IntSlider(value=1), []) + return w.VBox(children=[slider, Slow(count=count)]) + + import logging as std_logging # this module imports reacton.logging as logging + + # render() logs this right before it blocks on the lock another thread holds + waiting = threading.Event() + + class WaitingHandler(std_logging.Handler): + def emit(self, record): + if "waiting for mutex" in record.getMessage(): + waiting.set() + + handler = WaitingHandler(level=std_logging.INFO) + logger = std_logging.getLogger("reacton") + level = logger.level + logger.addHandler(handler) + logger.setLevel(std_logging.INFO) + box, rc = react.render_fixed(App(), handle_error=False) + slider = rc.find(widgets.IntSlider).widget + state_thread = threading.Thread(target=lambda: set_count(1)) + state_thread.start() + forced_thread = None + try: + assert entered.wait(5) + slider.value = 7 + forced_thread = threading.Thread(target=lambda: rc.render(rc.element)) + forced_thread.start() + assert waiting.wait(5) + finally: + release.set() + state_thread.join(5) + if forced_thread is not None: + forced_thread.join(5) + logger.removeHandler(handler) + logger.setLevel(level) + assert not state_thread.is_alive() + assert forced_thread is not None and not forced_thread.is_alive() + assert slider.value == 1 + rc.close() + + +def test_force_update_with_empty_key_child(): + # a child component with key "" reconciles its root with the same keys as the root + # element; that must not end the forced walk before the siblings after it + @react.component + def Child(): + return w.Label(value="child") + + box, rc = react.render_fixed(w.VBox(children=[Child().key(""), w.IntSlider(value=1)]), handle_error=False) + slider = box.children[1] + slider.value = 7 + rc.force_update() + assert slider.value == 1 + rc.close() + + +@fast_renderer_only +def test_state_render_after_force_update_is_not_forced(): + set_count = lambda x: None # noqa + + @react.component + def App(): + nonlocal set_count + count, set_count = react.use_state(0) + slider = react.use_memo(lambda: w.IntSlider(value=1), []) + return w.VBox(children=[slider, w.Button(description=str(count))]) + + box, rc = react.render_fixed(App(), handle_error=False) + slider = rc.find(widgets.IntSlider).widget + slider.value = 7 + rc.force_update() + assert slider.value == 1 + slider.value = 7 + set_count(1) + assert slider.value == 7 + rc.close()