add tests for coverage

Author: rmorshea

src/idom/core/component.py

@@ -8,65 +8,60 @@
 
 
 def component(
-    function: Callable[..., Union[ComponentType, VdomDict]]
+    function: Callable[..., Union[ComponentType, VdomDict | None]]
 ) -> Callable[..., "Component"]:
-    """A decorator for defining an :class:`Component`.
+    """A decorator for defining a new component.
 
     Parameters:
-        function: The function that will render a :class:`VdomDict`.
+        function: The component's :meth:`idom.core.proto.ComponentType.render` function.
     """
     sig = inspect.signature(function)
-    key_is_kwarg = "key" in sig.parameters and sig.parameters["key"].kind in (
+
+    if "key" in sig.parameters and sig.parameters["key"].kind in (
         inspect.Parameter.KEYWORD_ONLY,
         inspect.Parameter.POSITIONAL_OR_KEYWORD,
-    )
-    if key_is_kwarg:
+    ):
         raise TypeError(
             f"Component render function {function} uses reserved parameter 'key'"
         )
 
     @wraps(function)
     def constructor(*args: Any, key: Optional[Any] = None, **kwargs: Any) -> Component:
-        if key_is_kwarg:
-            kwargs["key"] = key
-        return Component(function, key, args, kwargs)
+        return Component(function, key, args, kwargs, sig)
 
     return constructor
 
 
 class Component:
     """An object for rending component models."""
 
-    __slots__ = "__weakref__", "_func", "_args", "_kwargs", "key"
+    __slots__ = "__weakref__", "_func", "_args", "_kwargs", "_sig", "key", "type"
 
     def __init__(
         self,
         function: Callable[..., Union[ComponentType, VdomDict]],
         key: Optional[Any],
         args: Tuple[Any, ...],
         kwargs: Dict[str, Any],
+        sig: inspect.Signature,
     ) -> None:
+        self.key = key
+        self.type = function
         self._args = args
-        self._func = function
         self._kwargs = kwargs
-        self.key = key
-
-    @property
-    def definition_id(self) -> int:
-        return id(self._func)
+        self._sig = sig
 
-    def render(self) -> VdomDict:
-        return self._func(*self._args, **self._kwargs)
+    def render(self) -> VdomDict | ComponentType | None:
+        return self.type(*self._args, **self._kwargs)
 
     def __repr__(self) -> str:
-        sig = inspect.signature(self._func)
         try:
-            args = sig.bind(*self._args, **self._kwargs).arguments
+            args = self._sig.bind(*self._args, **self._kwargs).arguments
         except TypeError:
-            return f"{self._func.__name__}(...)"
+            return f"{self.type.__name__}(...)"
         else:
             items = ", ".join(f"{k}={v!r}" for k, v in args.items())
             if items:
-                return f"{self._func.__name__}({id(self):02x}, {items})"
+                return f"{self.type.__name__}({id(self):02x}, {items})"
             else:
-                return f"{self._func.__name__}({id(self):02x})"
+                return f"{self.type.__name__}({id(self):02x})"

src/idom/core/layout.py

@@ -18,6 +18,7 @@
     Set,
     Tuple,
     TypeVar,
+    cast,
 )
 from uuid import uuid4
 from weakref import ref as weakref
@@ -203,12 +204,11 @@ def _render_component(
             # wrap the model in a fragment (i.e. tagName="") to ensure components have
             # a separate node in the model state tree. This could be removed if this
             # components are given a node in the tree some other way
-            raw_model = {
-                "tagName": "",
-                "children": [] if raw_model is None else [raw_model],
-            }
+            wrapper_model = {"tagName": ""}
+            if raw_model is not None:
+                wrapper_model["children"] = [raw_model]
 
-            self._render_model(old_state, new_state, raw_model)
+            self._render_model(old_state, new_state, wrapper_model)
         except Exception as error:
             logger.exception(f"Failed to render {component}")
             new_state.model.current = {
@@ -242,15 +242,6 @@ def _render_model(
             new_state.key = new_state.model.current["key"] = raw_model["key"]
         if "importSource" in raw_model:
             new_state.model.current["importSource"] = raw_model["importSource"]
-
-        if old_state is not None and old_state.key != new_state.key:
-            self._unmount_model_states([old_state])
-            if new_state.is_component_state:
-                self._model_states_by_life_cycle_state_id[
-                    new_state.life_cycle_state.id
-                ] = new_state
-            old_state = None
-
         self._render_model_attributes(old_state, new_state, raw_model)
         self._render_model_children(old_state, new_state, raw_model.get("children", []))
 
@@ -380,6 +371,7 @@ def _render_model_children(
                 new_children.append(new_child_state.model.current)
                 new_state.children_by_key[key] = new_child_state
             elif child_type is _COMPONENT_TYPE:
+                child = cast(ComponentType, child)
                 old_child_state = old_state.children_by_key.get(key)
                 if old_child_state is None:
                     new_child_state = _make_component_model_state(
@@ -390,8 +382,7 @@ def _render_model_children(
                         self._rendering_queue.put,
                     )
                 elif old_child_state.is_component_state and (
-                    old_child_state.life_cycle_state.component.definition_id
-                    != child.definition_id
+                    old_child_state.life_cycle_state.component.type != child.type
                 ):
                     self._unmount_model_states([old_child_state])
                     old_child_state = None

src/idom/core/proto.py

@@ -32,15 +32,14 @@ class ComponentType(Protocol):
     key: Key | None
     """An identifier which is unique amongst a component's immediate siblings"""
 
-    @property
-    def definition_id(self) -> int:
-        """A globally unique identifier for this component definition.
+    type: type[Any] | Callable[..., Any]
+    """The function or class defining the behavior of this component
 
-        Usually the :func:`id` of this class or an underlying function.
-        """
+    This is used to see if two component instances share the same definition.
+    """
 
     def render(self) -> VdomDict | ComponentType | None:
-        """Render the component's :class:`VdomDict`."""
+        """Render the component's view model."""
 
 
 _Self = TypeVar("_Self")

src/idom/html.py

@@ -168,7 +168,7 @@ def _(*children: Any) -> VdomDict:
     """An HTML fragment - this element will not appear in the DOM"""
     attributes, children = coalesce_attributes_and_children(children)
     if attributes:
-        raise ValueError("Fragments cannot have attributes")
+        raise TypeError("Fragments cannot have attributes")
     return {"tagName": "", "children": children}
 
 

tests/test_core/test_layout.py

@@ -13,13 +13,14 @@
 from idom.core.component import component
 from idom.core.dispatcher import render_json_patch
 from idom.core.hooks import use_effect, use_state
-from idom.core.layout import LayoutEvent
+from idom.core.layout import Layout, LayoutEvent
 from idom.testing import (
     HookCatcher,
     StaticEventHandler,
     assert_idom_logged,
     capture_idom_logs,
 )
+from idom.utils import Ref
 from tests.assert_utils import assert_same_items
 
 
@@ -96,6 +97,15 @@ def SimpleComponent():
         ]
 
 
+async def test_component_can_return_none():
+    @idom.component
+    def SomeComponent():
+        return None
+
+    with idom.Layout(SomeComponent()) as layout:
+        assert (await layout.render()).new == {"tagName": ""}
+
+
 async def test_nested_component_layout():
     parent_set_state = idom.Ref(None)
     child_set_state = idom.Ref(None)
@@ -716,14 +726,18 @@ def HasNestedEventHandler():
 
 async def test_duplicate_sibling_keys_causes_error(caplog):
     hook = HookCatcher()
+    should_error = True
 
     @idom.component
     @hook.capture
     def ComponentReturnsDuplicateKeys():
-        return idom.html.div(
-            idom.html.div(key="duplicate"),
-            idom.html.div(key="duplicate"),
-        )
+        if should_error:
+            return idom.html.div(
+                idom.html.div(key="duplicate"),
+                idom.html.div(key="duplicate"),
+            )
+        else:
+            return idom.html.div()
 
     with idom.Layout(ComponentReturnsDuplicateKeys()) as layout:
         with assert_idom_logged(
@@ -735,6 +749,11 @@ def ComponentReturnsDuplicateKeys():
 
         hook.latest.schedule_render()
 
+        should_error = False
+        await layout.render()
+
+        should_error = True
+        hook.latest.schedule_render()
         with assert_idom_logged(
             error_type=ValueError,
             match_error=r"Duplicate keys \['duplicate'\] at '/children/0'",
@@ -838,9 +857,9 @@ def use_toggle():
     def Root():
         toggle, set_toggle.current = use_toggle()
         if toggle:
-            return idom.html.div(SomeComponent("x"))
+            return SomeComponent("x")
         else:
-            return idom.html.div(idom.html.div(SomeComponent("y")))
+            return idom.html.div(SomeComponent("y"))
 
     @idom.component
     def SomeComponent(name):
@@ -1063,3 +1082,82 @@ async def record_if_state_is_reset():
             await layout.render()
             assert effect_calls_without_state == ["some-key", "key-0"]
             did_call_effect.clear()
+
+
+async def test_changing_key_of_component_resets_state():
+    set_key = Ref()
+    did_init_state = Ref(0)
+    hook = HookCatcher()
+
+    @component
+    @hook.capture
+    def Root():
+        key, set_key.current = use_state("key-1")
+        return Child(key=key)
+
+    @component
+    def Child():
+        use_state(lambda: did_init_state.set_current(did_init_state.current + 1))
+
+    with Layout(Root()) as layout:
+        await layout.render()
+        assert did_init_state.current == 1
+
+        set_key.current("key-2")
+        await layout.render()
+        assert did_init_state.current == 2
+
+        hook.latest.schedule_render()
+        await layout.render()
+        assert did_init_state.current == 2
+
+
+async def test_changing_event_handlers_in_the_next_render():
+    set_event_name = Ref()
+    event_handler = StaticEventHandler()
+    did_trigger = Ref(False)
+
+    @component
+    def Root():
+        event_name, set_event_name.current = use_state("first")
+        return html.button(
+            {event_name: event_handler.use(lambda: did_trigger.set_current(True))}
+        )
+
+    with Layout(Root()) as layout:
+        await layout.render()
+        await layout.deliver(LayoutEvent(event_handler.target, []))
+        assert did_trigger.current
+        did_trigger.current = False
+
+        set_event_name.current("second")
+        await layout.render()
+        await layout.deliver(LayoutEvent(event_handler.target, []))
+        assert did_trigger.current
+        did_trigger.current = False
+
+
+async def test_change_element_to_string_causes_unmount():
+    set_toggle = Ref()
+    did_unmount = Ref(False)
+
+    @component
+    def Root():
+        toggle, set_toggle.current = use_toggle(True)
+        if toggle:
+            return html.div(Child())
+        else:
+            return html.div("some-string")
+
+    @component
+    def Child():
+        use_effect(lambda: lambda: did_unmount.set_current(True))
+
+    with Layout(Root()) as layout:
+        await layout.render()
+
+        set_toggle.current()
+
+        await layout.render()
+
+        assert did_unmount.current

tests/test_html.py

@@ -142,3 +142,12 @@ def test_script_may_only_have_one_child():
 def test_child_of_script_must_be_string():
     with pytest.raises(ValueError, match="The child of a 'script' must be a string"):
         html.script(1)
+
+
+def test_simple_fragment():
+    assert html._(1, 2, 3) == {"tagName": "", "children": [1, 2, 3]}
+
+
+def test_fragment_can_have_no_attributes():
+    with pytest.raises(TypeError, match="Fragments cannot have attributes"):
+        html._({"some-attribute": 1})