diff --git a/docs/source/reference-material/_examples/snake_game.py b/docs/source/reference-material/_examples/snake_game.py index 8f1478499..483eec8b3 100644 --- a/docs/source/reference-material/_examples/snake_game.py +++ b/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): diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 511e2ee3f..a08457f53 100644 --- a/src/idom/core/layout.py +++ b/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() diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index 366d9c3bd..c3bc23f1b 100644 --- a/tests/test_core/test_layout.py +++ b/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():