allow dynamic adding and removing event listeners (#52)

Bloomca · Jun 9, 2024 · 98b9346 · 98b9346
1 parent 8551d06
commit 98b9346
Show file tree

Hide file tree

Showing 7 changed files with 881 additions and 217 deletions.
diff --git a/integration-tests/assign-attributes.test.ts b/integration-tests/assign-attributes.test.ts
@@ -43,7 +43,8 @@ describe("createState", () => {
             value: nameState.useAttribute((name) => name),
             onFocus: focusFn,
             onBlur: blurFn,
-            onInput: (e) => nameState.setValue(() => e.target.value),
+            onInput: (e) =>
+              nameState.setValue((e.target as HTMLInputElement).value),
           }),
           nameState.useValue((value) =>
             createElement("div", {
@@ -101,7 +102,8 @@ describe("createState", () => {
             value: nameState.useAttribute(),
             onFocus: focusFn,
             onBlur: blurFn,
-            onInput: (e) => nameState.setValue(e.target.value),
+            onInput: (e) =>
+              nameState.setValue((e.target as HTMLInputElement).value),
           }),
           nameState.useValue((value) =>
             createElement("div", {
@@ -216,4 +218,91 @@ describe("createState", () => {
     expect(testBtn.getAttribute("disabled")).toBe(null);
     expect((testBtn as HTMLButtonElement).disabled).toBe(false);
   });
+
+  it("allows to assign and remove event listeners dynamically", async () => {
+    const user = userEvent.setup();
+    const state = createState(0);
+    const spyFn = jest.fn();
+    function App() {
+      return createElement("div", {
+        children: [
+          createElement("button", {
+            "data-testid": "button",
+            onClick: state.useAttribute((value) =>
+              value !== 0 && value < 4
+                ? () => {
+                    spyFn();
+                    state.setValue((currentValue) => currentValue + 1);
+                  }
+                : undefined
+            ),
+          }),
+        ],
+      });
+    }
+
+    cleanup = attachComponent({
+      htmlElement: document.body,
+      component: createElement(App),
+    });
+
+    const btn = screen.getByTestId("button");
+
+    await user.click(btn);
+    await user.click(btn);
+
+    state.setValue(1);
+
+    await user.click(btn);
+    await user.click(btn);
+    await user.click(btn);
+    await user.click(btn);
+    await user.click(btn);
+
+    expect(spyFn).toHaveBeenCalledTimes(3);
+    expect(state.getValue()).toBe(4);
+  });
+
+  it("allows to assign and remove event listeners dynamically passing the same callback", async () => {
+    const user = userEvent.setup();
+    const state = createState(0);
+    const spyFn = jest.fn();
+    function App() {
+      const handler = () => {
+        spyFn();
+        state.setValue((currentValue) => currentValue + 1);
+      };
+      return createElement("div", {
+        children: [
+          createElement("button", {
+            "data-testid": "button",
+            onClick: state.useAttribute((value) =>
+              value !== 0 && value < 4 ? handler : undefined
+            ),
+          }),
+        ],
+      });
+    }
+
+    cleanup = attachComponent({
+      htmlElement: document.body,
+      component: createElement(App),
+    });
+
+    const btn = screen.getByTestId("button");
+
+    await user.click(btn);
+    await user.click(btn);
+
+    state.setValue(1);
+
+    await user.click(btn);
+    await user.click(btn);
+    await user.click(btn);
+    await user.click(btn);
+    await user.click(btn);
+
+    expect(spyFn).toHaveBeenCalledTimes(3);
+    expect(state.getValue()).toBe(4);
+  });
 });
diff --git a/integration-tests/create-state/create-state.test.ts b/integration-tests/create-state/create-state.test.ts
@@ -64,7 +64,7 @@ describe("createState", () => {
   test("supports direct state updates", async () => {
     const user = userEvent.setup();
     function StateComponent() {
-      const nameState = createState(0);
+      const nameState = createState("");
       return createElement("div", {
         children: [
           createElement("input", {

diff --git a/integration-tests/create-state/track-value.test.ts b/integration-tests/create-state/track-value.test.ts
@@ -178,7 +178,7 @@ describe("createState", () => {
             onInput: (e) =>
               userState.setValue((currentUser) => ({
                 ...currentUser,
-                name: e.target.value,
+                name: (e.target as HTMLInputElement).value,
               })),
           }),
           createElement("input", {
@@ -190,7 +190,7 @@ describe("createState", () => {
             onInput: (e) =>
               userState.setValue((currentUser) => ({
                 ...currentUser,
-                email: e.target.value,
+                email: (e.target as HTMLInputElement).value,
               })),
           }),
         ],
@@ -244,7 +244,7 @@ describe("createState", () => {
             onInput: (e) =>
               userState.setValue((currentUser) => ({
                 ...currentUser,
-                name: e.target.value,
+                name: (e.target as HTMLInputElement).value,
               })),
           }),
           createElement("input", {
@@ -256,7 +256,7 @@ describe("createState", () => {
             onInput: (e) =>
               userState.setValue((currentUser) => ({
                 ...currentUser,
-                email: e.target.value,
+                email: (e.target as HTMLInputElement).value,
               })),
           }),
         ],

diff --git a/src/create-element/assign-attributes.ts b/src/create-element/assign-attributes.ts
@@ -13,32 +13,42 @@ function assignAttributes({
     const isFunction = typeof value === "function";
     if (isFunction && value.velesAttribute === true) {
       const attributeValue = value(htmlElement, key, velesNode);
-      if (typeof attributeValue === "boolean") {
-        // according to the spec, boolean values should just get either an empty string
-        // or duplicated key. I don't see a reason to duplicate the key.
-        // If the value is `false`, no need to set it, the correct behaviour is to remove it.
-        if (value) htmlElement.setAttribute(key, "");
-      } else {
-        htmlElement.setAttribute(key, attributeValue);
-      }
-    } else if (
-      // basically, any form of `on` handlers, like `onClick`, `onCopy`, etc
-      isFunction &&
-      key.startsWith("on")
-    ) {
-      // TODO: think if this is robust enough
-      htmlElement.addEventListener(
-        key[2].toLocaleLowerCase() + key.slice(3),
-        value
-      );
+      assignAttribute({ key, value: attributeValue, htmlElement });
     } else {
-      if (typeof value === "boolean") {
-        if (value) htmlElement.setAttribute(key, "");
-      } else {
-        htmlElement.setAttribute(key, value);
-      }
+      assignAttribute({ key, value, htmlElement });
     }
   });
 }
 
+function assignAttribute({
+  key,
+  value,
+  htmlElement,
+}: {
+  key: string;
+  value: any;
+  htmlElement: HTMLElement;
+}) {
+  if (
+    // basically, any form of `on` handlers, like `onClick`, `onCopy`, etc
+    typeof value === "function" &&
+    key.startsWith("on")
+  ) {
+    // TODO: think if this is robust enough
+    htmlElement.addEventListener(
+      key[2].toLocaleLowerCase() + key.slice(3),
+      value
+    );
+  } else {
+    if (typeof value === "boolean") {
+      // according to the spec, boolean values should just get either an empty string
+      // or duplicated key. I don't see a reason to duplicate the key.
+      // If the value is `false`, no need to set it, the correct behaviour is to remove it.
+      if (value) htmlElement.setAttribute(key, "");
+    } else {
+      htmlElement.setAttribute(key, value);
+    }
+  }
+}
+
 export { assignAttributes };
diff --git a/src/hooks/create-state.ts b/src/hooks/create-state.ts
@@ -118,6 +118,7 @@ function createState<T>(
     cb?: Function;
     htmlElement: HTMLElement;
     attributeName: string;
+    attributeValue: any;
   }[] = [];
 
   let trackingIterators: TrackingParams[] = [];
@@ -299,7 +300,8 @@ function createState<T>(
 
     // attributes
     // the HTML node does not change, so we don't need to modify the array
-    trackingAttributes.forEach(({ cb, htmlElement, attributeName }) => {
+    trackingAttributes.forEach((element) => {
+      const { cb, htmlElement, attributeName, attributeValue } = element;
       const newAttributeValue = cb ? cb(value) : value;
 
       if (typeof newAttributeValue === "boolean") {
@@ -308,6 +310,24 @@ function createState<T>(
         } else {
           htmlElement.removeAttribute(attributeName);
         }
+      } else if (attributeName.startsWith("on")) {
+        // if the value is the same, it is either not set
+        // or we received the same event handler
+        // either way, no need to do anything
+        if (attributeValue === newAttributeValue) {
+          return;
+        }
+
+        const eventName =
+          attributeName[2].toLocaleLowerCase() + attributeName.slice(3);
+        if (attributeValue) {
+          htmlElement.removeEventListener(eventName, attributeValue);
+        }
+        if (newAttributeValue && typeof newAttributeValue === "function") {
+          htmlElement.addEventListener(eventName, newAttributeValue);
+        }
+        // not the best approach, but it should work as expected
+        element.attributeValue = newAttributeValue;
       } else {
         htmlElement.setAttribute(attributeName, newAttributeValue);
       }
@@ -776,7 +796,12 @@ function createState<T>(
         // read that array on `_triggerUpdates`
         // and change inline
         // we need to save the HTML element and the name of the attribute
-        const trackingElement = { cb, htmlElement, attributeName };
+        const trackingElement = {
+          cb,
+          htmlElement,
+          attributeName,
+          attributeValue,
+        };
         trackingAttributes.push(trackingElement);
 
         node._privateMethods._addUnmountHandler(() => {