fix bug with conditional returns in useValue (#38)

Bloomca · Jun 4, 2024 · fecc3f4 · fecc3f4
1 parent a3d3f7d
commit fecc3f4
Show file tree

Hide file tree

Showing 3 changed files with 217 additions and 66 deletions.
diff --git a/integration-tests/create-state/create-state.test.ts b/integration-tests/create-state/create-state.test.ts
@@ -446,4 +446,132 @@ describe("createState", () => {
     await user.click(btn);
     expect(screen.getByTestId("container").textContent).toBe("new title");
   });
+
+  test("supports state changes correctly when conditional is return true/false/true", async () => {
+    let newValue = "";
+    const user = userEvent.setup();
+    function StateComponent() {
+      const titleState = createState({ title: "title" });
+      return createElement("div", {
+        children: [
+          createElement("button", {
+            "data-testid": "button",
+            onClick: () => {
+              titleState.setValue({ title: newValue });
+            },
+          }),
+          createElement("div", {
+            "data-testid": "container",
+            children: titleState.useValueSelector(
+              (data) => data.title.length > 3,
+              (isLong) =>
+                isLong
+                  ? createElement(ConditionalComponent, { state: titleState })
+                  : null
+            ),
+          }),
+        ],
+      });
+    }
+
+    function ConditionalComponent({
+      state,
+    }: {
+      state: State<{ title: string }>;
+    }) {
+      return createElement("div", {
+        children: [
+          createElement("div", {
+            "data-testid": "text",
+            children: state.useValue(
+              (value) => `length is ${value.title.length}`
+            ),
+          }),
+        ],
+      });
+    }
+
+    cleanup = attachComponent({
+      htmlElement: document.body,
+      component: createElement(StateComponent),
+    });
+
+    expect(screen.getByTestId("text").textContent).toBe("length is 5");
+    const btn = screen.getByTestId("button");
+
+    newValue = "";
+    await user.click(btn);
+
+    newValue = "new title";
+    await user.click(btn);
+    expect(screen.getByTestId("text").textContent).toBe("length is 9");
+
+    newValue = "another new title";
+    await user.click(btn);
+    expect(screen.getByTestId("text").textContent).toBe("length is 17");
+  });
+
+  test("supports state changes correctly when conditional is not rendered initially", async () => {
+    let newValue = "";
+    const user = userEvent.setup();
+    function StateComponent() {
+      const titleState = createState({ title: "" });
+      return createElement("div", {
+        children: [
+          createElement("button", {
+            "data-testid": "button",
+            onClick: () => {
+              titleState.setValue({ title: newValue });
+            },
+          }),
+          createElement("div", {
+            "data-testid": "container",
+            children: titleState.useValueSelector(
+              (data) => data.title.length > 3,
+              (isLong) =>
+                isLong
+                  ? createElement(ConditionalComponent, { state: titleState })
+                  : null
+            ),
+          }),
+        ],
+      });
+    }
+
+    function ConditionalComponent({
+      state,
+    }: {
+      state: State<{ title: string }>;
+    }) {
+      return createElement("div", {
+        children: [
+          createElement("div", {
+            "data-testid": "text",
+            children: state.useValue(
+              (value) => `length is ${value.title.length}`
+            ),
+          }),
+        ],
+      });
+    }
+
+    cleanup = attachComponent({
+      htmlElement: document.body,
+      component: createElement(StateComponent),
+    });
+
+    const btn = screen.getByTestId("button");
+
+    newValue = "title";
+    await user.click(btn);
+    expect(screen.getByTestId("text").textContent).toBe("length is 5");
+
+    newValue = "new title";
+    await user.click(btn);
+    expect(screen.getByTestId("text").textContent).toBe("length is 9");
+
+    newValue = "another new title";
+    await user.click(btn);
+    expect(screen.getByTestId("text").textContent).toBe("length is 17");
+  });
 });
diff --git a/src/hooks/create-state.ts b/src/hooks/create-state.ts
@@ -1,4 +1,9 @@
-import { getComponentVelesNode, callMountHandlers, identity } from "../utils";
+import {
+  getComponentVelesNode,
+  callMountHandlers,
+  identity,
+  unique,
+} from "../utils";
 import { onUnmount, onMount } from "./lifecycle";
 import { createElement } from "../create-element/create-element";
 import { createTextElement } from "../create-element/create-text-element";
@@ -344,73 +349,76 @@ function createState<T>(
     // TODO: remove it from this object completely
     // and access it from closure
     _triggerUpdates: () => {
-      trackingSelectorElements = trackingSelectorElements.map(
-        (selectorTrackingElement) => {
-          const { selectedValue, selector, cb, node, comparator } =
-            selectorTrackingElement;
-          const newSelectedValue = selector ? selector(value) : value;
-
-          if (comparator(selectedValue, newSelectedValue)) {
-            return selectorTrackingElement;
-          }
+      const newTrackingSelectorElements: typeof trackingSelectorElements = [];
+      trackingSelectorElements.forEach((selectorTrackingElement) => {
+        const { selectedValue, selector, cb, node, comparator } =
+          selectorTrackingElement;
+        const newSelectedValue = selector ? selector(value) : value;
 
-          const returnednewNode = cb
-            ? cb(newSelectedValue)
-            : String(newSelectedValue);
-          const newNode =
-            !returnednewNode || typeof returnednewNode === "string"
-              ? createTextElement(returnednewNode as string)
-              : returnednewNode;
-
-          const { velesElementNode: oldVelesElementNode } =
-            getComponentVelesNode(node);
-          const { velesElementNode: newVelesElementNode } =
-            getComponentVelesNode(newNode);
-
-          const parentVelesElement = oldVelesElementNode.parentVelesElement;
-
-          const newTrackingSelectorElement = {
-            selector,
-            selectedValue: newSelectedValue,
-            cb,
-            node: newNode,
-            comparator,
-          };
-
-          if (parentVelesElement) {
-            newVelesElementNode.parentVelesElement = parentVelesElement;
-            parentVelesElement.html.replaceChild(
-              newVelesElementNode.html,
-              oldVelesElementNode.html
-            );
-            // we need to update `childComponents` so that after the update
-            // if the parent node is removed from DOM, it calls correct unmount
-            // callbacks
-            parentVelesElement.childComponents =
-              parentVelesElement.childComponents.map((childComponent) =>
-                childComponent === node ? newNode : node
-              );
-            // we call unmount handlers right after we replace it
-            node._privateMethods._callUnmountHandlers();
-            // at this point the new Node is mounted, childComponents are updated
-            // and unmount handlers for the old node are called
-            callMountHandlers(newNode);
-
-            // right after that, we add the callback back
-            // the top level node is guaranteed to be rendered again (at least right now)
-            // if there were children listening, they should be cleared
-            // and added back into their respective unmount listeners if it is still viable
-            newNode._privateMethods._addUnmountHandler(() => {
-              trackingSelectorElements = trackingSelectorElements.filter(
-                (el) => el !== newTrackingSelectorElement
-              );
-            });
-          } else {
-            console.log("parent node was not found");
-          }
+        if (comparator(selectedValue, newSelectedValue)) {
+          newTrackingSelectorElements.push(selectorTrackingElement);
+          return;
+        }
+
+        const returnednewNode = cb
+          ? cb(newSelectedValue)
+          : String(newSelectedValue);
+        const newNode =
+          !returnednewNode || typeof returnednewNode === "string"
+            ? createTextElement(returnednewNode as string)
+            : returnednewNode;
+
+        const { velesElementNode: oldVelesElementNode } =
+          getComponentVelesNode(node);
+        const { velesElementNode: newVelesElementNode } =
+          getComponentVelesNode(newNode);
+        const parentVelesElement = oldVelesElementNode.parentVelesElement;
+
+        const newTrackingSelectorElement = {
+          selector,
+          selectedValue: newSelectedValue,
+          cb,
+          node: newNode,
+          comparator,
+        };
 
-          return newTrackingSelectorElement;
+        if (parentVelesElement) {
+          newVelesElementNode.parentVelesElement = parentVelesElement;
+          parentVelesElement.html.replaceChild(
+            newVelesElementNode.html,
+            oldVelesElementNode.html
+          );
+          // we need to update `childComponents` so that after the update
+          // if the parent node is removed from DOM, it calls correct unmount
+          // callbacks
+          parentVelesElement.childComponents =
+            parentVelesElement.childComponents.map((childComponent) =>
+              childComponent === node ? newNode : node
+            );
+          // we call unmount handlers right after we replace it
+          node._privateMethods._callUnmountHandlers();
+          // at this point the new Node is mounted, childComponents are updated
+          // and unmount handlers for the old node are called
+          callMountHandlers(newNode);
+
+          // right after that, we add the callback back
+          // the top level node is guaranteed to be rendered again (at least right now)
+          // if there were children listening, they should be cleared
+          // and added back into their respective unmount listeners if it is still viable
+          newNode._privateMethods._addUnmountHandler(() => {
+            trackingSelectorElements = trackingSelectorElements.filter(
+              (el) => el !== newTrackingSelectorElement
+            );
+          });
+        } else {
+          console.log("parent node was not found");
         }
+
+        newTrackingSelectorElements.push(newTrackingSelectorElement);
+      });
+
+      trackingSelectorElements = unique(
+        trackingSelectorElements.concat(newTrackingSelectorElements)
       );
 
       // attributes

diff --git a/src/utils.ts b/src/utils.ts
@@ -57,4 +57,19 @@ function identity<T>(value1: T, value2: T) {
   return value1 === value2;
 }
 
-export { getComponentVelesNode, identity, callMountHandlers };
+// return an array with elements being there only one time total
+// the first encountered value will be preserved
+function unique<T>(arr: T[]): T[] {
+  const map = new Map<T, true>();
+  const resultArr: T[] = [];
+  arr.forEach((element) => {
+    if (map.has(element)) return;
+
+    map.set(element, true);
+    resultArr.push(element);
+  });
+
+  return resultArr;
+}
+
+export { getComponentVelesNode, identity, callMountHandlers, unique };