Skip to content

Commit 99219f3

Browse files
committed
fix #652
In short, because components do not have a node in the model state tree managed by the layout, keys for elements in the components were being registered to the parent of the component rather than to a node for the component which contained the elements. The short-term solution was to make it so components wrap their contents in a div without a key. The long-term solution is to refactor the Layout() and give components a dedicated node in the model state tree
1 parent dcd507f commit 99219f3

File tree

2 files changed

+55
-3
lines changed

2 files changed

+55
-3
lines changed

src/idom/core/component.py

+1-3
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,7 @@ def definition_id(self) -> int:
5757

5858
def render(self) -> VdomDict:
5959
model = self._func(*self._args, **self._kwargs)
60-
if isinstance(model, ComponentType):
61-
model = {"tagName": "div", "children": [model]}
62-
return model
60+
return {"tagName": "div", "children": [model]}
6361

6462
def __repr__(self) -> str:
6563
sig = inspect.signature(self._func)

tests/test_core/test_layout.py

+54
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import idom
1111
from idom import html
1212
from idom.config import IDOM_DEBUG_MODE
13+
from idom.core.component import component
1314
from idom.core.dispatcher import render_json_patch
1415
from idom.core.hooks import use_effect, use_state
1516
from idom.core.layout import LayoutEvent
@@ -923,3 +924,56 @@ def SecondComponent():
923924

924925
assert first_used_state.current == "first"
925926
assert second_used_state.current is None
927+
928+
929+
async def test_element_keys_inside_components_do_not_reset_state_of_component():
930+
"""This is a regression test for a bug.
931+
932+
You would not expect that calling `set_child_key_num` would trigger state to be
933+
reset in any `Child()` components but there was a bug where that happened.
934+
"""
935+
936+
effect_calls_without_state = []
937+
set_child_key_num = StaticEventHandler()
938+
did_call_effect = asyncio.Event()
939+
940+
@component
941+
def Parent():
942+
state, set_state = use_state(0)
943+
return html.div(
944+
html.button(
945+
{"onClick": set_child_key_num.use(lambda: set_state(state + 1))},
946+
"click me",
947+
),
948+
Child("some-key"),
949+
Child(f"key-{state}"),
950+
)
951+
952+
@component
953+
def Child(child_key):
954+
state, set_state = use_state(0)
955+
956+
@use_effect
957+
async def record_if_state_is_reset():
958+
if state:
959+
return
960+
effect_calls_without_state.append(child_key)
961+
set_state(1)
962+
did_call_effect.set()
963+
964+
return html.div(
965+
child_key,
966+
key=child_key,
967+
)
968+
969+
with idom.Layout(Parent()) as layout:
970+
await layout.render()
971+
await did_call_effect.wait()
972+
assert effect_calls_without_state == ["some-key", "key-0"]
973+
did_call_effect.clear()
974+
975+
for i in range(1, 5):
976+
await layout.deliver(LayoutEvent(set_child_key_num.target, []))
977+
await layout.render()
978+
assert effect_calls_without_state == ["some-key", "key-0"]
979+
did_call_effect.clear()

0 commit comments

Comments
 (0)