fix model mutation in client

The client-side model was being mutated on each update
in order to try and be more performant. This is a problem
though because React relies on checking the identity of
some parts of the model to see if things have changed.

No we create a copy of the model each time it changed

Author: rmorshea

src/client/package-lock.json

@@ -1,7 +1,7 @@
 {
   "name": "idom-client-react",
   "description": "A client for IDOM implemented in React",
-  "version": "0.9.1",
+  "version": "0.9.2",
   "author": "Ryan Morshead",
   "license": "MIT",
   "type": "module",

src/client/packages/idom-client-react/package.json

@@ -4,7 +4,7 @@ import htm from "htm";
 
 import serializeEvent from "./event-to-object.js";
 
-import { applyPatchInplace, joinUrl } from "./utils.js";
+import { useJsonPatchCallback } from "./utils.js";
 
 const html = htm.bind(React.createElement);
 export const LayoutConfigContext = React.createContext({
@@ -13,7 +13,7 @@ export const LayoutConfigContext = React.createContext({
 });
 
 export function Layout({ saveUpdateHook, sendEvent, loadImportSource }) {
-  const [model, patchModel] = useInplaceJsonPatch({});
+  const [model, patchModel] = useJsonPatchCallback({});
 
   React.useEffect(() => saveUpdateHook(patchModel), [patchModel]);
 
@@ -181,33 +181,3 @@ function loadImportSource(config, importSource) {
       }
     });
 }
-
-function useInplaceJsonPatch(doc) {
-  const ref = React.useRef(doc);
-  const forceUpdate = useForceUpdate();
-
-  const applyPatch = React.useCallback(
-    (path, patch) => {
-      applyPatchInplace(ref.current, path, patch);
-      forceUpdate();
-    },
-    [ref, forceUpdate]
-  );
-
-  return [ref.current, applyPatch];
-}
-
-function useForceUpdate() {
-  const [, updateState] = React.useState();
-  return React.useCallback(() => updateState({}), []);
-}
-
-function useConst(func) {
-  const ref = React.useRef();
-
-  if (!ref.current) {
-    ref.current = func();
-  }
-
-  return ref.current;
-}

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

@@ -1,16 +1,35 @@
+import React from "react";
 import jsonpatch from "fast-json-patch";
 
-export function applyPatchInplace(doc, pathPrefix, patch) {
-  if (pathPrefix) {
-    patch = patch.map((op) =>
-      Object.assign({}, op, { path: pathPrefix + op.path })
-    );
-  }
-  jsonpatch.applyPatch(doc, patch, false, true);
+export function useJsonPatchCallback(initial) {
+  const model = React.useRef(initial);
+  const forceUpdate = useForceUpdate();
+
+  const applyPatch = React.useCallback(
+    (pathPrefix, patch) => {
+      if (pathPrefix) {
+        patch = patch.map((op) =>
+          Object.assign({}, op, { path: pathPrefix + op.path })
+        );
+      }
+      // Always return a newDocument because React checks some attributes of the model
+      // (e.g. model.attributes.style is checked for identity)
+      model.current = jsonpatch.applyPatch(
+        model.current,
+        patch,
+        false,
+        false,
+        true
+      ).newDocument;
+      forceUpdate();
+    },
+    [model]
+  );
+
+  return [model.current, applyPatch];
 }
 
-export function joinUrl(base, tail) {
-  return tail.startsWith("./")
-    ? (base.endsWith("/") ? base.slice(0, -1) : base) + tail.slice(1)
-    : tail;
+function useForceUpdate() {
+  const [, updateState] = React.useState();
+  return React.useCallback(() => updateState({}), []);
 }

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

@@ -43,3 +43,42 @@ def NewComponent():
         # check that we can resume normal operation
         set_state.current(1)
         driver.find_element_by_id("new-component-1")
+
+
+def test_style_can_be_changed(display, driver, driver_wait):
+    """This test was introduced to verify the client does not mutate the model
+
+    A bug was introduced where the client-side model was mutated and React was relying
+    on the model to have been copied in order to determine if something had changed.
+
+    See for more info: https://github.com/idom-team/idom/issues/480
+    """
+
+    @idom.component
+    def ButtonWithChangingColor():
+        color_toggle, set_color_toggle = idom.hooks.use_state(True)
+        color = "red" if color_toggle else "blue"
+        return idom.html.button(
+            {
+                "id": "my-button",
+                "onClick": lambda event: set_color_toggle(not color_toggle),
+                "style": {"backgroundColor": color, "color": "white"},
+            },
+            f"color: {color}",
+        )
+
+    display(ButtonWithChangingColor)
+
+    button = driver.find_element_by_id("my-button")
+
+    assert get_style(button)["background-color"] == "red"
+
+    for color in ["blue", "red"] * 2:
+        button.click()
+        driver_wait.until(lambda _: get_style(button)["background-color"] == color)
+
+
+def get_style(element):
+    items = element.get_attribute("style").split(";")
+    pairs = [item.split(":", 1) for item in map(str.strip, items) if item]
+    return {key.strip(): value.strip() for key, value in pairs}