Skip to content

Commit

Permalink
fix append to stale children ref (#807)
Browse files Browse the repository at this point in the history
  • Loading branch information
rmorshea authored Sep 12, 2022
1 parent a1ebebd commit d682407
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 9 deletions.
8 changes: 5 additions & 3 deletions docs/source/about/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ more info, see the :ref:`Contributor Guide <Creating a Changelog Entry>`.
Unreleased
----------

No changes.
**Fixed**

- :issue:`806` - child models after a component fail to render

v0.40.0
-------

v0.40.0 (yanked)
----------------
:octicon:`milestone` *released on 2022-08-13*

**Fixed**
Expand Down
15 changes: 9 additions & 6 deletions src/idom/core/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ def _render_model_children(
[old_state.children_by_key[key] for key in old_keys]
)

new_children = new_state.model.current["children"] = []
new_state.model.current["children"] = []
for index, (child, child_type, key) in enumerate(child_type_key_tuples):
old_child_state = old_state.children_by_key.get(key)
if child_type is _DICT_TYPE:
Expand All @@ -388,7 +388,7 @@ def _render_model_children(
index,
)
self._render_model(exit_stack, old_child_state, new_child_state, child)
new_children.append(new_child_state.model.current)
new_state.append_child(new_child_state.model.current)
new_state.children_by_key[key] = new_child_state
elif child_type is _COMPONENT_TYPE:
child = cast(ComponentType, child)
Expand Down Expand Up @@ -428,7 +428,7 @@ def _render_model_children(
old_child_state = old_state.children_by_key.get(key)
if old_child_state is not None:
self._unmount_model_states([old_child_state])
new_children.append(child)
new_state.append_child(child)

def _render_model_children_without_old_state(
self,
Expand All @@ -446,20 +446,20 @@ def _render_model_children_without_old_state(
f"Duplicate keys {duplicate_keys} at {new_state.patch_path or '/'!r}"
)

new_children = new_state.model.current["children"] = []
new_state.model.current["children"] = []
for index, (child, child_type, key) in enumerate(child_type_key_tuples):
if child_type is _DICT_TYPE:
child_state = _make_element_model_state(new_state, index, key)
self._render_model(exit_stack, None, child_state, child)
new_children.append(child_state.model.current)
new_state.append_child(child_state.model.current)
new_state.children_by_key[key] = child_state
elif child_type is _COMPONENT_TYPE:
child_state = _make_component_model_state(
new_state, index, key, child, self._rendering_queue.put
)
self._render_component(exit_stack, None, child_state, child)
else:
new_children.append(child)
new_state.append_child(child)

def _unmount_model_states(self, old_states: List[_ModelState]) -> None:
to_unmount = old_states[::-1] # unmount in reversed order of rendering
Expand Down Expand Up @@ -661,6 +661,9 @@ def parent(self) -> _ModelState:
assert parent is not None, "detached model state"
return parent

def append_child(self, child: Any) -> None:
self.model.current["children"].append(child)

def __repr__(self) -> str: # pragma: no cover
return f"ModelState({ {s: getattr(self, s, None) for s in self.__slots__} })"

Expand Down
40 changes: 40 additions & 0 deletions tests/test_core/test_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -1207,3 +1207,43 @@ def bad_should_render(new):
await layout.render()
root_hook.latest.schedule_render()
await layout.render()


async def test_does_render_children_after_component():
"""Regression test for bug where layout was appending children to a stale ref
The stale reference was created when a component got rendered. Thus, everything
after the component failed to display.
"""

@idom.component
def Parent():
return html.div(
html.p("first"),
Child(),
html.p("third"),
)

@idom.component
def Child():
return html.p("second")

async with idom.Layout(Parent()) as layout:
update = await layout.render()
print(update.new)
assert update.new == {
"tagName": "",
"children": [
{
"tagName": "div",
"children": [
{"tagName": "p", "children": ["first"]},
{
"tagName": "",
"children": [{"tagName": "p", "children": ["second"]}],
},
{"tagName": "p", "children": ["third"]},
],
}
],
}

0 comments on commit d682407

Please sign in to comment.