fix #801

accidental mutation of old parent model caused invalid JSON patches

Author: rmorshea

docs/source/about/changelog.rst

@@ -27,6 +27,7 @@ Unreleased
 
 - :issue:`789` - Conditionally rendered components cannot use contexts
 - :issue:`773` - Use strict equality check for text, numeric, and binary types in hooks
+- :issue:`801` - Accidental mutation of old model causes invalid JSON Patch
 
 **Changed**
 

src/idom/core/layout.py

@@ -161,9 +161,6 @@ def _create_layout_update(self, old_state: _ModelState) -> LayoutUpdate:
         new_state = _copy_component_model_state(old_state)
         component = new_state.life_cycle_state.component
 
-        with ExitStack() as exit_stack:
-            self._render_component(exit_stack, old_state, new_state, component)
-
         old_model: Optional[VdomJson]
         try:
             old_model = old_state.model.current
@@ -230,10 +227,17 @@ def _render_component(
         else:
             key, index = new_state.key, new_state.index
             parent.children_by_key[key] = new_state
-            # need to do insertion in case where old_state is None and we're appending
-            parent.model.current["children"][index : index + 1] = [
-                new_state.model.current
-            ]
+            # need to add this model to parent's children without mutating parent model
+            old_parent_model = parent.model.current
+            old_parent_children = old_parent_model["children"]
+            parent.model.current = {
+                **old_parent_model,
+                "children": [
+                    *old_parent_children[:index],
+                    new_state.model.current,
+                    *old_parent_children[index + 1 :],
+                ],
+            }
         finally:
             # avoid double render
             self._rendering_queue.remove_if_pending(life_cycle_state.id)

tests/test_client.py

@@ -5,13 +5,15 @@
 from playwright.async_api import Browser
 
 import idom
+from idom.backend.utils import find_available_port
 from idom.testing import BackendFixture, DisplayFixture
 
 
 JS_DIR = Path(__file__).parent / "js"
 
 
 async def test_automatic_reconnect(browser: Browser):
+    port = find_available_port("localhost")
     page = await browser.new_page()
 
     # we need to wait longer here because the automatic reconnect is not instant
@@ -22,7 +24,7 @@ def OldComponent():
         return idom.html.p({"id": "old-component"}, "old")
 
     async with AsyncExitStack() as exit_stack:
-        server = await exit_stack.enter_async_context(BackendFixture(port=8000))
+        server = await exit_stack.enter_async_context(BackendFixture(port=port))
         display = await exit_stack.enter_async_context(
             DisplayFixture(server, driver=page)
         )
@@ -43,7 +45,7 @@ def NewComponent():
         return idom.html.p({"id": f"new-component-{state}"}, f"new-{state}")
 
     async with AsyncExitStack() as exit_stack:
-        server = await exit_stack.enter_async_context(BackendFixture(port=8000))
+        server = await exit_stack.enter_async_context(BackendFixture(port=port))
         display = await exit_stack.enter_async_context(
             DisplayFixture(server, driver=page)
         )

tests/test_core/test_hooks.py

@@ -11,12 +11,10 @@
     current_hook,
     strictly_equal,
 )
-from idom.core.layout import Layout
-from idom.core.serve import render_json_patch
+from idom.core.layout import Layout, LayoutUpdate
 from idom.testing import DisplayFixture, HookCatcher, assert_idom_did_log, poll
 from idom.testing.logs import assert_idom_did_not_log
 from idom.utils import Ref
-from tests.tooling.asserts import assert_same_items
 
 
 async def test_must_be_rendering_in_layout_to_use_hooks():
@@ -42,31 +40,35 @@ def SimpleStatefulComponent():
     sse = SimpleStatefulComponent()
 
     async with idom.Layout(sse) as layout:
-        patch_1 = await render_json_patch(layout)
-        assert patch_1.path == ""
-        assert_same_items(
-            patch_1.changes,
-            [
-                {"op": "add", "path": "/tagName", "value": ""},
-                {
-                    "op": "add",
-                    "path": "/children",
-                    "value": [{"children": ["0"], "tagName": "div"}],
-                },
-            ],
+        update_1 = await layout.render()
+        assert update_1 == LayoutUpdate(
+            path="",
+            old=None,
+            new={
+                "tagName": "",
+                "children": [{"tagName": "div", "children": ["0"]}],
+            },
         )
 
-        patch_2 = await render_json_patch(layout)
-        assert patch_2.path == ""
-        assert patch_2.changes == [
-            {"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/children/0", "value": "2"}
-        ]
+        update_2 = await layout.render()
+        assert update_2 == LayoutUpdate(
+            path="",
+            old=update_1.new,
+            new={
+                "tagName": "",
+                "children": [{"tagName": "div", "children": ["1"]}],
+            },
+        )
+
+        update_3 = await layout.render()
+        assert update_3 == LayoutUpdate(
+            path="",
+            old=update_2.new,
+            new={
+                "tagName": "",
+                "children": [{"tagName": "div", "children": ["2"]}],
+            },
+        )
 
 
 async def test_set_state_callback_identity_is_preserved():