implement element fragment + try to fix tests

rmorshea · rmorshea · commit 781c6f66b1c4 · 2022-02-17T22:54:11.000-08:00
diff --git a/setup.cfg b/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
diff --git a/src/client/packages/idom-client-react/src/components.js b/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,
diff --git a/src/idom/core/component.py b/src/idom/core/component.py
@@ -56,8 +56,7 @@ def definition_id(self) -> int:
         return id(self._func)
 
     def render(self) -> VdomDict:
-        model = self._func(*self._args, **self._kwargs)
-        return {"tagName": "div", "children": [model]}
+        return self._func(*self._args, **self._kwargs)
 
     def __repr__(self) -> str:
         sig = inspect.signature(self._func)
diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py
@@ -199,6 +199,12 @@ def _render_component(
                 raw_model = component.render()
             finally:
                 life_cycle_hook.unset_current()
+
+            # 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": [raw_model]}
+
             self._render_model(old_state, new_state, raw_model)
         except Exception as error:
             logger.exception(f"Failed to render {component}")
diff --git a/temp.py b/temp.py
@@ -0,0 +1,9 @@
+import idom
+
+
+@idom.component
+def Demo():
+    return idom.vdom("", idom.html.h1("hello"))
+
+
+idom.run(Demo)
diff --git a/tests/assert_utils.py b/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
diff --git a/tests/general_utils.py b/tests/general_utils.py
diff --git a/tests/test_core/test_dispatcher.py b/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
 
diff --git a/tests/test_core/test_hooks.py b/tests/test_core/test_hooks.py
@@ -7,7 +7,7 @@
 from idom.core.dispatcher import render_json_patch
 from idom.core.hooks import LifeCycleHook
 from idom.testing import HookCatcher, assert_idom_logged
-from tests.general_utils import assert_same_items
+from tests.assert_utils import assert_same_items
 
 
 async def test_must_be_rendering_in_layout_to_use_hooks():
@@ -38,21 +38,25 @@ def SimpleStatefulComponent():
         assert_same_items(
             patch_1.changes,
             [
-                {"op": "add", "path": "/children", "value": ["0"]},
-                {"op": "add", "path": "/tagName", "value": "div"},
+                {"op": "add", "path": "/tagName", "value": ""},
+                {
+                    "op": "add",
+                    "path": "/children",
+                    "value": [{"children": ["0"], "tagName": "div"}],
+                },
             ],
         )
 
         patch_2 = await render_json_patch(layout)
         assert patch_2.path == ""
         assert patch_2.changes == [
-            {"op": "replace", "path": "/children/0", "value": "1"}
+            {"op": "replace", "path": "/children/0/children/0", "value": "1"}
         ]
 
         patch_3 = await render_json_patch(layout)
         assert patch_3.path == ""
         assert patch_3.changes == [
-            {"op": "replace", "path": "/children/0", "value": "2"}
+            {"op": "replace", "path": "/children/0/children/0", "value": "2"}
         ]
 
 
diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py
@@ -20,7 +20,7 @@
     assert_idom_logged,
     capture_idom_logs,
 )
-from tests.general_utils import assert_same_items
+from tests.assert_utils import assert_same_items
 
 
 @pytest.fixture(autouse=True)
@@ -79,13 +79,21 @@ def SimpleComponent():
         path, changes = await render_json_patch(layout)
 
         assert path == ""
-        assert changes == [{"op": "add", "path": "/tagName", "value": "div"}]
+        assert_same_items(
+            changes,
+            [
+                {"op": "add", "path": "/children", "value": [{"tagName": "div"}]},
+                {"op": "add", "path": "/tagName", "value": ""},
+            ],
+        )
 
         set_state_hook.current("table")
         path, changes = await render_json_patch(layout)
 
         assert path == ""
-        assert changes == [{"op": "replace", "path": "/tagName", "value": "table"}]
+        assert changes == [
+            {"op": "replace", "path": "/children/0/tagName", "value": "table"}
+        ]
 
 
 async def test_nested_component_layout():
@@ -112,23 +120,38 @@ def Child():
                 {
                     "op": "add",
                     "path": "/children",
-                    "value": ["0", {"tagName": "div", "children": ["0"]}],
+                    "value": [
+                        {
+                            "children": [
+                                "0",
+                                {
+                                    "children": [{"children": ["0"], "tagName": "div"}],
+                                    "tagName": "",
+                                },
+                            ],
+                            "tagName": "div",
+                        }
+                    ],
                 },
-                {"op": "add", "path": "/tagName", "value": "div"},
+                {"op": "add", "path": "/tagName", "value": ""},
             ],
         )
 
         parent_set_state.current(1)
         path, changes = await render_json_patch(layout)
 
         assert path == ""
-        assert changes == [{"op": "replace", "path": "/children/0", "value": "1"}]
+        assert changes == [
+            {"op": "replace", "path": "/children/0/children/0", "value": "1"}
+        ]
 
         child_set_state.current(1)
         path, changes = await render_json_patch(layout)
 
         assert path == "/children/1"
-        assert changes == [{"op": "replace", "path": "/children/0", "value": "1"}]
+        assert changes == [
+            {"op": "replace", "path": "/children/0/children/0", "value": "1"}
+        ]
 
 
 @pytest.mark.skipif(
@@ -202,16 +225,21 @@ def BadChild():
             assert_same_items(
                 patch.changes,
                 [
+                    {"op": "add", "path": "/tagName", "value": ""},
                     {
                         "op": "add",
                         "path": "/children",
                         "value": [
-                            {"tagName": "div", "children": ["hello"]},
-                            {"tagName": "", "error": ""},
-                            {"tagName": "div", "children": ["hello"]},
+                            {
+                                "children": [
+                                    {"children": [...], "tagName": ""},
+                                    {"error": "", "tagName": ""},
+                                    {"children": [...], "tagName": ""},
+                                ],
+                                "tagName": "div",
+                            }
                         ],
                     },
-                    {"op": "add", "path": "/tagName", "value": "div"},
                 ],
             )
 
@@ -233,9 +261,24 @@ def Child():
                 {
                     "op": "add",
                     "path": "/children",
-                    "value": [{"tagName": "div", "children": [{"tagName": "h1"}]}],
+                    "value": [
+                        {
+                            "children": [
+                                {
+                                    "children": [
+                                        {
+                                            "children": [{"tagName": "h1"}],
+                                            "tagName": "div",
+                                        }
+                                    ],
+                                    "tagName": "",
+                                }
+                            ],
+                            "tagName": "div",
+                        }
+                    ],
                 },
-                {"op": "add", "path": "/tagName", "value": "div"},
+                {"op": "add", "path": "/tagName", "value": ""},
             ],
         )
 
@@ -422,7 +465,7 @@ def Child():
         hook.latest.schedule_render()
 
         update = await render_json_patch(layout)
-        assert update.path == "/children/0/children/0"
+        assert update.path == "/children/0/children/0/children/0"
 
 
 async def test_log_on_dispatch_to_missing_event_handler(caplog):
