fix #652

.pre-commit-config.yaml

@@ -3,10 +3,6 @@ repos:
     rev: 22.1.0
     hooks:
       - id: black
-  - repo: https://github.com/PyCQA/flake8
-    rev: 3.7.9
-    hooks:
-      - id: flake8
   - repo: https://github.com/pycqa/isort
     rev: 5.6.3
     hooks:

requirements/pkg-extras.txt

@@ -12,6 +12,7 @@ uvicorn[standard] >=0.13.4
 
 # extra=flask
 flask<2.0
+markupsafe<2.1
 flask-cors
 flask-sockets
 

setup.cfg

@@ -24,6 +24,7 @@ testpaths = tests
 xfail_strict = True
 markers =
     slow: marks tests as slow (deselect with '-m "not slow"')
+python_files = assert_*.py test_*.py
 
 [coverage:report]
 fail_under = 100

src/client/packages/idom-client-react/src/components.js

@@ -20,6 +20,10 @@ export function Layout({ saveUpdateHook, sendEvent, loadImportSource }) {
 
   React.useEffect(() => saveUpdateHook(patchModel), [patchModel]);
 
+  if (!Object.keys(model).length) {
+    return html`<${React.Fragment} />`;
+  }
+
   return html`
     <${LayoutContext.Provider} value=${{ sendEvent, loadImportSource }}>
       <${Element} model=${model} />
@@ -28,7 +32,7 @@ export function Layout({ saveUpdateHook, sendEvent, loadImportSource }) {
 }
 
 export function Element({ model }) {
-  if (!model.tagName) {
+  if (model.error !== undefined) {
     if (model.error) {
       return html`<pre>${model.error}</pre>`;
     } else {
@@ -45,11 +49,19 @@ export function Element({ model }) {
 
 function StandardElement({ model }) {
   const layoutContext = React.useContext(LayoutContext);
+
+  let type;
+  if (model.tagName == "") {
+    type = React.Fragment;
+  } else {
+    type = model.tagName;
+  }
+
   // Use createElement here to avoid warning about variable numbers of children not
   // having keys. Warning about this must now be the responsibility of the server
   // providing the models instead of the client rendering them.
   return React.createElement(
-    model.tagName,
+    type,
     createElementAttributes(model, layoutContext.sendEvent),
     ...createElementChildren(
       model,

src/idom/core/component.py

@@ -8,68 +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]],
+        function: Callable[..., ComponentType | VdomDict | None],
         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:
-        model = self._func(*self._args, **self._kwargs)
-        if isinstance(model, ComponentType):
-            model = {"tagName": "div", "children": [model]}
-        return model
+    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
@@ -31,7 +32,7 @@
 
 from ._event_proxy import _wrap_in_warning_event_proxies
 from .hooks import LifeCycleHook
-from .proto import ComponentType, EventHandlerDict, VdomJson
+from .proto import ComponentType, EventHandlerDict, VdomDict, VdomJson
 from .vdom import validate_vdom_json
 
 
@@ -199,7 +200,15 @@ def _render_component(
                 raw_model = component.render()
             finally:
                 life_cycle_hook.unset_current()
-            self._render_model(old_state, new_state, raw_model)
+
+            # 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
+            wrapper_model: VdomDict = {"tagName": ""}
+            if raw_model is not None:
+                wrapper_model["children"] = [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 = {
@@ -233,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", []))
 
@@ -371,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(
@@ -381,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
@@ -411,10 +411,18 @@ def _render_model_children(
     def _render_model_children_without_old_state(
         self, new_state: _ModelState, raw_children: List[Any]
     ) -> None:
+        child_type_key_tuples = list(_process_child_type_and_key(raw_children))
+
+        new_keys = {item[2] for item in child_type_key_tuples}
+        if len(new_keys) != len(raw_children):
+            key_counter = Counter(item[2] for item in child_type_key_tuples)
+            duplicate_keys = [key for key, count in key_counter.items() if count > 1]
+            raise ValueError(
+                f"Duplicate keys {duplicate_keys} at {new_state.patch_path or '/'!r}"
+            )
+
         new_children = new_state.model.current["children"] = []
-        for index, (child, child_type, key) in enumerate(
-            _process_child_type_and_key(raw_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(None, child_state, child)

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:
-        """Render the component's :class:`VdomDict`."""
+    def render(self) -> VdomDict | ComponentType | None:
+        """Render the component's view model."""
 
 
 _Self = TypeVar("_Self")
@@ -88,7 +87,7 @@ class _VdomDictOptional(TypedDict, total=False):
     children: Sequence[
         # recursive types are not allowed yet:
         # https://github.com/python/mypy/issues/731
-        Union[ComponentType, Dict[str, Any], str]
+        Union[ComponentType, Dict[str, Any], str, Any]
     ]
     attributes: VdomAttributes
     eventHandlers: EventHandlerDict  # noqa

src/idom/html.py

@@ -1,4 +1,8 @@
 """
+**Fragment**
+
+- :func:`_`
+
 **Dcument metadata**
 
 - :func:`base`
@@ -148,6 +152,8 @@
 
 - :func:`slot`
 - :func:`template`
+
+.. autofunction:: _
 """
 
 from __future__ import annotations
@@ -158,6 +164,14 @@
 from .core.vdom import coalesce_attributes_and_children, make_vdom_constructor
 
 
+def _(*children: Any) -> VdomDict:
+    """An HTML fragment - this element will not appear in the DOM"""
+    attributes, coalesced_children = coalesce_attributes_and_children(children)
+    if attributes:
+        raise TypeError("Fragments cannot have attributes")
+    return {"tagName": "", "children": coalesced_children}
+
+
 # Dcument metadata
 base = make_vdom_constructor("base")
 head = make_vdom_constructor("head")

temp.py

@@ -0,0 +1,9 @@
+import idom
+
+
+@idom.component
+def Demo():
+    return idom.vdom("", idom.html.h1("hello"))
+
+
+idom.run(Demo)

tests/assert_utils.py

@@ -0,0 +1,5 @@
+def assert_same_items(left, right):
+    """Check that two unordered sequences are equal (only works if reprs are equal)"""
+    sorted_left = list(sorted(left, key=repr))
+    sorted_right = list(sorted(right, key=repr))
+    assert sorted_left == sorted_right

tests/test_core/test_dispatcher.py

@@ -57,15 +57,20 @@ async def recv():
 def make_events_and_expected_model():
     events = [LayoutEvent(STATIC_EVENT_HANDLER.target, [])] * 4
     expected_model = {
-        "tagName": "div",
-        "attributes": {"count": 4},
-        "eventHandlers": {
-            EVENT_NAME: {
-                "target": STATIC_EVENT_HANDLER.target,
-                "preventDefault": False,
-                "stopPropagation": False,
+        "tagName": "",
+        "children": [
+            {
+                "tagName": "div",
+                "attributes": {"count": 4},
+                "eventHandlers": {
+                    EVENT_NAME: {
+                        "target": STATIC_EVENT_HANDLER.target,
+                        "preventDefault": False,
+                        "stopPropagation": False,
+                    }
+                },
             }
-        },
+        ],
     }
     return events, expected_model