Skip to content

Commit d145398

Browse files
fix: get_widget could fail with 'x not found in all known widgets'
This happened when we could re-render, and there was a function component in the render tree that wouldn't need a re-execution. It would then reconciliate with the old element data (also its arguments) and the reference to the old element would map to get widget that would be used in get_widget. However, a use_effect/use_event would have a reference to a new element and reacton would not be able to find the widget. We now update the args and kwargs of the old element to the new elements.
1 parent 51e6eda commit d145398

File tree

2 files changed

+119
-4
lines changed

2 files changed

+119
-4
lines changed

reacton/core.py

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,7 +1428,7 @@ def render(self, element: Element, container: widgets.Widget = None):
14281428

14291429
try:
14301430
self._shared_elements_next = set()
1431-
self._render(self.element, "/", parent_key=ROOT_KEY)
1431+
self._render(self.element, "/", parent_key=ROOT_KEY, old_to_new={})
14321432
self.first_render = False
14331433
except BaseException:
14341434
self._is_rendering = False
@@ -1463,7 +1463,7 @@ def format(reason: RerenderReason):
14631463
self.context.exceptions_children = []
14641464
self.context.exceptions_self = []
14651465

1466-
self._render(self.element, "/", parent_key=ROOT_KEY)
1466+
self._render(self.element, "/", parent_key=ROOT_KEY, old_to_new={})
14671467
logger.info("Render done: %r %r", self._rerender_needed, self._rerender_needed_reasons[-1])
14681468
assert self.context is self.context_root
14691469
render_counts += 1
@@ -1565,7 +1565,7 @@ def format(reason: RerenderReason):
15651565
raise exc
15661566
return widget
15671567

1568-
def _render(self, element: Element, default_key: str, parent_key: str):
1568+
def _render(self, element: Element, default_key: str, parent_key: str, old_to_new: dict[Element, Element] = {}):
15691569
if not isinstance(element, Element):
15701570
raise TypeError(f"Expected element, not {element}")
15711571
# for tracking stale data/elements when using get_widget
@@ -1729,12 +1729,43 @@ def _render(self, element: Element, default_key: str, parent_key: str):
17291729
else:
17301730
root_element = context.root_element_next or context.root_element
17311731

1732+
# We don't rerender the component, but that mean that the root_element
1733+
# refers to elements from a previous render pass. We need to update those so
1734+
# that we have a working get_widget()
1735+
# We first store a map of key->element for the invoked element of previous
1736+
# render pass
1737+
key_to_element: dict[str, Element] = {}
1738+
1739+
def _store_key_to_element(el, key, parent_key):
1740+
key_to_element[key] = el
1741+
1742+
self._visit_children_values(el.kwargs, "/", "/", _store_key_to_element)
1743+
self._visit_children_values(el.args, "/", "/", _store_key_to_element)
1744+
# Next, we go over all children in the new element, and we then build up
1745+
# A mapping of element->element from old to new
1746+
# we will mutate, so make a copy
1747+
old_to_new = old_to_new.copy()
1748+
1749+
def _store_old_to_new(el, key, parent_key):
1750+
old_to_new[el] = key_to_element[key]
1751+
1752+
assert el_prev is not None
1753+
self._visit_children_values(el_prev.kwargs, "/", "/", _store_old_to_new)
1754+
self._visit_children_values(el_prev.args, "/", "/", _store_old_to_new)
1755+
1756+
def map_old_to_new(el, key, parent_key):
1757+
return old_to_new.get(el, el)
1758+
1759+
assert root_element is not None
1760+
root_element.kwargs = self._visit_children_values(root_element.kwargs, "/", "/", map_old_to_new)
1761+
root_element.args = self._visit_children_values(root_element.args, "/", "/", map_old_to_new)
1762+
17321763
if self.render_count != render_count:
17331764
raise RuntimeError("Recursive render detected, possible a bug in react")
17341765
if root_element is not None:
17351766
logger.debug("root element: %r %x", root_element, id(root_element))
17361767
new_parent_key = join_key(parent_key, key)
1737-
self._render(root_element, "/", parent_key=new_parent_key) # depth first
1768+
self._render(root_element, "/", parent_key=new_parent_key, old_to_new=old_to_new) # depth first
17381769
context.root_element_next = root_element
17391770
else:
17401771
if el.is_shared:

reacton/core_test.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3204,3 +3204,87 @@ def Test():
32043204
set_state(1)
32053205
rc.render(w.HTML(value="recover").key("HTML"))
32063206
rc.close()
3207+
3208+
3209+
def test_get_widget_fail_on_rerender_use_event():
3210+
@reacton.component
3211+
def Test():
3212+
force_rerender, set_force_rerender = react.use_state(0, key="force_rerender")
3213+
click_works, set_click_works = react.use_state(False, key="click_works")
3214+
3215+
with ContainerFunction():
3216+
el = v.Btn(children=["Works"] if click_works else ["Does not work"])
3217+
3218+
v.use_event(el, "click", lambda *_ignore: set_click_works(True))
3219+
3220+
if force_rerender == 0:
3221+
set_force_rerender(1)
3222+
3223+
box, rc = react.render(Test(), handle_error=False)
3224+
rc.find(ipyvuetify.Btn).widget.click()
3225+
assert rc.find(ipyvuetify.Btn).widget.children[0] == "Works"
3226+
rc.close()
3227+
3228+
3229+
@pytest.mark.parametrize("on_use_effect", [True, False])
3230+
def test_get_widget_fail_on_rerender_simple(on_use_effect):
3231+
@reacton.component
3232+
def Test():
3233+
force_rerender, set_force_rerender = react.use_state(0, key="force_rerender")
3234+
3235+
def effect():
3236+
widget = react.get_widget(el)
3237+
3238+
assert widget is not None
3239+
3240+
use_effect(effect, [])
3241+
with ContainerFunction():
3242+
el = w.Button(description="Hi")
3243+
3244+
react.use_effect(effect, None)
3245+
3246+
if force_rerender == 0 and not on_use_effect:
3247+
set_force_rerender(1)
3248+
3249+
def possibly_rerender():
3250+
if force_rerender == 0 and on_use_effect:
3251+
set_force_rerender(1)
3252+
3253+
use_effect(possibly_rerender, None)
3254+
3255+
box, rc = react.render(Test(), handle_error=False)
3256+
rc.close()
3257+
3258+
3259+
def test_get_widget_fail_on_rerender_complex(Container1, Container2):
3260+
@reacton.component
3261+
def MakeMoreComplex(arg, children=[]):
3262+
return Container1(children=[*children, arg])
3263+
3264+
@reacton.component
3265+
def Test():
3266+
force_rerender, set_force_rerender = react.use_state(0, key="force_rerender")
3267+
3268+
def effect():
3269+
widget1 = react.get_widget(el1)
3270+
widget2 = react.get_widget(el2)
3271+
3272+
assert widget1 is not None
3273+
assert widget2 is not None
3274+
3275+
use_effect(effect, [])
3276+
el1 = w.Button(description="Foo")
3277+
with MakeMoreComplex(el1):
3278+
with Container2():
3279+
el2 = w.Button(description="Bar")
3280+
3281+
react.use_effect(effect, None)
3282+
3283+
def rerender():
3284+
if force_rerender == 0:
3285+
set_force_rerender(1)
3286+
3287+
react.use_effect(rerender, [])
3288+
3289+
box, rc = react.render(Test(), handle_error=False)
3290+
rc.close()

0 commit comments

Comments
 (0)