allow elements with the same key to change type

Author: rmorshea

docs/source/reference-material/_examples/snake_game.py

@@ -18,12 +18,7 @@ def GameView():
     game_state, set_game_state = idom.hooks.use_state(GameState.init)
 
     if game_state == GameState.play:
-        return GameLoop(
-            grid_size=6,
-            block_scale=50,
-            set_game_state=set_game_state,
-            key="game loop",
-        )
+        return GameLoop(grid_size=6, block_scale=50, set_game_state=set_game_state)
 
     start_button = idom.html.button(
         {"onClick": lambda event: set_game_state(GameState.play)},
@@ -45,12 +40,7 @@ def GameView():
         """
     )
 
-    return idom.html.div(
-        {"className": "snake-game-menu"},
-        menu_style,
-        menu,
-        key="menu",
-    )
+    return idom.html.div({"className": "snake-game-menu"}, menu_style, menu)
 
 
 class Direction(enum.Enum):

src/idom/core/layout.py

@@ -100,7 +100,7 @@ def __enter__(self: _Self) -> _Self:
     def __exit__(self, *exc: Any) -> None:
         root_csid = self._root_life_cycle_state_id
         root_model_state = self._model_states_by_life_cycle_state_id[root_csid]
-        self._unmount_model_states([root_model_state])
+        self._deep_unmount_model_states([root_model_state])
 
         # delete attributes here to avoid access after exiting context manager
         del self._event_handlers
@@ -166,7 +166,7 @@ def _create_layout_update(self, old_state: _ModelState) -> LayoutUpdate:
 
         # hook effects must run after the update is complete
         for model_state in _iter_model_state_children(new_state):
-            if hasattr(model_state, "life_cycle_state"):
+            if model_state.is_component_state:
                 model_state.life_cycle_state.hook.component_did_render()
 
         old_model: Optional[VdomJson]
@@ -271,10 +271,10 @@ def _render_model_attributes(
 
         model_event_handlers = new_state.model.current["eventHandlers"] = {}
         for event, handler in handlers_by_event.items():
-            target = old_state.targets_by_event.get(
-                event,
-                uuid4().hex if handler.target is None else handler.target,
-            )
+            if event in old_state.targets_by_event:
+                target = old_state.targets_by_event[event]
+            else:
+                target = uuid4().hex if handler.target is None else handler.target
             new_state.targets_by_event[event] = target
             self._event_handlers[target] = handler
             model_event_handlers[event] = {
@@ -320,7 +320,7 @@ def _render_model_children(
                 self._render_model_children_without_old_state(new_state, raw_children)
             return None
         elif not raw_children:
-            self._unmount_model_states(list(old_state.children_by_key.values()))
+            self._deep_unmount_model_states(list(old_state.children_by_key.values()))
             return None
 
         child_type_key_tuples = list(_process_child_type_and_key(raw_children))
@@ -335,12 +335,13 @@ def _render_model_children(
 
         old_keys = set(old_state.children_by_key).difference(new_keys)
         if old_keys:
-            self._unmount_model_states(
+            self._deep_unmount_model_states(
                 [old_state.children_by_key[key] for key in old_keys]
             )
 
         new_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:
                 old_child_state = old_state.children_by_key.get(key)
                 if old_child_state is None:
@@ -350,6 +351,8 @@ def _render_model_children(
                         key,
                     )
                 else:
+                    if old_child_state.is_component_state:
+                        self._shallow_unmount_model_state(old_child_state)
                     new_child_state = _update_element_model_state(
                         old_child_state,
                         new_state,
@@ -374,9 +377,13 @@ def _render_model_children(
                         new_state,
                         index,
                         child,
+                        self._rendering_queue.put,
                     )
                 self._render_component(old_child_state, new_child_state, child)
             else:
+                old_child_state = old_state.children_by_key.get(key)
+                if old_child_state is not None:
+                    self._deep_unmount_model_states([old_child_state])
                 new_children.append(child)
 
     def _render_model_children_without_old_state(
@@ -399,20 +406,21 @@ def _render_model_children_without_old_state(
             else:
                 new_children.append(child)
 
-    def _unmount_model_states(self, old_states: List[_ModelState]) -> None:
+    def _deep_unmount_model_states(self, old_states: List[_ModelState]) -> None:
         to_unmount = old_states[::-1]  # unmount in reversed order of rendering
         while to_unmount:
             model_state = to_unmount.pop()
+            self._shallow_unmount_model_state(model_state)
+            to_unmount.extend(model_state.children_by_key.values())
 
-            for target in model_state.targets_by_event.values():
-                del self._event_handlers[target]
-
-            if hasattr(model_state, "life_cycle_state"):
-                life_cycle_state = model_state.life_cycle_state
-                del self._model_states_by_life_cycle_state_id[life_cycle_state.id]
-                life_cycle_state.hook.component_will_unmount()
+    def _shallow_unmount_model_state(self, old_state: _ModelState) -> None:
+        for target in old_state.targets_by_event.values():
+            del self._event_handlers[target]
 
-            to_unmount.extend(model_state.children_by_key.values())
+        if old_state.is_component_state:
+            life_cycle_state = old_state.life_cycle_state
+            del self._model_states_by_life_cycle_state_id[life_cycle_state.id]
+            life_cycle_state.hook.component_will_unmount()
 
     def __repr__(self) -> str:
         return f"{type(self).__name__}({self.root})"
@@ -459,7 +467,6 @@ def _make_component_model_state(
 
 
 def _copy_component_model_state(old_model_state: _ModelState) -> _ModelState:
-
     # use try/except here because not having a parent is rare (only the root state)
     try:
         parent: Optional[_ModelState] = old_model_state.parent
@@ -483,15 +490,8 @@ def _update_component_model_state(
     new_parent: _ModelState,
     new_index: int,
     new_component: ComponentType,
+    schedule_render: Callable[[_LifeCycleStateId], None],
 ) -> _ModelState:
-    try:
-        old_life_cycle_state = old_model_state.life_cycle_state
-    except AttributeError:
-        raise ValueError(
-            f"Failed to render layout at {old_model_state.patch_path!r} with key "
-            f"{old_model_state.key!r} - prior element with this key wasn't a component"
-        )
-
     return _ModelState(
         parent=new_parent,
         index=new_index,
@@ -500,7 +500,11 @@ def _update_component_model_state(
         patch_path=old_model_state.patch_path,
         children_by_key={},
         targets_by_event={},
-        life_cycle_state=_update_life_cycle_state(old_life_cycle_state, new_component),
+        life_cycle_state=(
+            _update_life_cycle_state(old_model_state.life_cycle_state, new_component)
+            if old_model_state.is_component_state
+            else _make_life_cycle_state(new_component, schedule_render)
+        ),
     )
 
 
@@ -525,12 +529,6 @@ def _update_element_model_state(
     new_parent: _ModelState,
     new_index: int,
 ) -> _ModelState:
-    if hasattr(old_model_state, "life_cycle_state"):
-        raise ValueError(
-            f"Failed to render layout at {old_model_state.patch_path!r} with key "
-            f"{old_model_state.key!r} - prior element with this key was a component"
-        )
-
     return _ModelState(
         parent=new_parent,
         index=new_index,
@@ -597,6 +595,10 @@ def __init__(
             self.life_cycle_state = life_cycle_state
             """The state for the element's component (if it has one)"""
 
+    @property
+    def is_component_state(self) -> bool:
+        return hasattr(self, "life_cycle_state")
+
     @property
     def parent(self) -> _ModelState:
         parent = self._parent_ref()

tests/test_core/test_layout.py

@@ -9,6 +9,7 @@
 import idom
 from idom.config import IDOM_DEBUG_MODE
 from idom.core.dispatcher import render_json_patch
+from idom.core.hooks import use_effect
 from idom.core.layout import LayoutEvent
 from idom.testing import (
     HookCatcher,
@@ -736,66 +737,45 @@ def Child(state):
             await layout.render()
 
 
-async def test_layout_element_cannot_become_a_component():
-    set_child_type = idom.Ref()
+async def test_elements_and_components_with_the_same_key_can_be_interchanged():
+    set_toggle = idom.Ref()
+    effects = []
+
+    def use_toggle():
+        state, set_state = idom.hooks.use_state(True)
+        return state, lambda: set_state(not state)
 
     @idom.component
     def Root():
-        child_type, set_child_type.current = idom.hooks.use_state("element")
-        return idom.html.div(child_nodes[child_type])
+        toggle, set_toggle.current = use_toggle()
+        if toggle:
+            return idom.html.div(SomeComponent("x"))
+        else:
+            return idom.html.div(idom.html.div(SomeComponent("y")))
 
     @idom.component
-    def Child():
-        return idom.html.div()
-
-    child_nodes = {
-        "element": idom.html.div(key="the-same-key"),
-        "component": Child(key="the-same-key"),
-    }
-
-    with assert_idom_logged(
-        error_type=ValueError,
-        match_error="prior element with this key wasn't a component",
-        clear_matched_records=True,
-    ):
-
-        with idom.Layout(Root()) as layout:
-            await layout.render()
-
-            set_child_type.current("component")
-
-            await layout.render()
-
+    def SomeComponent(name):
+        @use_effect
+        def some_effect():
+            effects.append("mount " + name)
+            return lambda: effects.append("unmount " + name)
 
-async def test_layout_component_cannot_become_an_element():
-    set_child_type = idom.Ref()
+        return idom.html.div(name)
 
-    @idom.component
-    def Root():
-        child_type, set_child_type.current = idom.hooks.use_state("component")
-        return idom.html.div(child_nodes[child_type])
-
-    @idom.component
-    def Child():
-        return idom.html.div()
+    with idom.Layout(Root()) as layout:
+        await layout.render()
 
-    child_nodes = {
-        "element": idom.html.div(key="the-same-key"),
-        "component": Child(key="the-same-key"),
-    }
+        assert effects == ["mount x"]
 
-    with assert_idom_logged(
-        error_type=ValueError,
-        match_error="prior element with this key was a component",
-        clear_matched_records=True,
-    ):
+        set_toggle.current()
+        await layout.render()
 
-        with idom.Layout(Root()) as layout:
-            await layout.render()
+        assert effects == ["mount x", "unmount x", "mount y"]
 
-            set_child_type.current("element")
+        set_toggle.current()
+        await layout.render()
 
-            await layout.render()
+        assert effects == ["mount x", "unmount x", "mount y", "unmount y", "mount x"]
 
 
 async def test_layout_does_not_copy_element_children_by_key():