Fix Hook Firing More Than Once (#6)

* Fix removeEventListener not working

* Rename

* Bump lint

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@buildinams/use-keydown",
   "description": "React hook for listening to custom keydown events.",
-  "version": "0.1.0",
+  "version": "0.1.1",
   "license": "MIT",
   "author": "Build in Amsterdam <[email protected]> (https://www.buildinamsterdam.com/)",
   "main": "dist/index.js",
@@ -45,7 +45,7 @@
   "devDependencies": {
     "@babel/preset-env": "^7.20.2",
     "@babel/preset-typescript": "^7.18.6",
-    "@buildinams/lint": "^0.0.2",
+    "@buildinams/lint": "^0.0.3",
     "@testing-library/react": "^13.4.0",
     "@types/jest": "^29.2.5",
     "@types/node": "^18.11.18",

src/types.ts

@@ -1,13 +1,8 @@
 export type OnChangeEvent = (
-  /** The original `keydown` event. */
+  /** The original keyboard event. */
   event: KeyboardEvent
 ) => void;
 
-export interface Listener {
-  targetKeyCode: string | string[];
-  onChange: OnChangeEvent;
-}
-
 export interface EventTargetRef {
   current: EventTarget | null;
 }
@@ -21,3 +16,15 @@ export interface Config {
    */
   target?: Target;
 }
+
+export type EventHandler = (baseEvent: Event) => void;
+
+export interface Listener {
+  keyCode: string | string[];
+  onChange: OnChangeEvent;
+}
+
+export interface Query {
+  eventHandler: EventHandler;
+  listeners: Set<Listener>;
+}

src/useKeydown.ts

@@ -1,68 +1,69 @@
-import { useEffect } from "react";
+import { useEffect, useRef } from "react";
 
-import { Config, Listener, OnChangeEvent } from "./types";
-import { getEventTargetFromConfig } from "./utils";
+import { Config, EventHandler, Listener, OnChangeEvent, Query } from "./types";
+import { getEventTargetFromTarget } from "./utils";
 
-const targets = new Map<EventTarget, Set<Listener>>();
+const queries = new Map<EventTarget, Query>();
 
 const addListener = (
   eventTarget: EventTarget,
   listener: Listener,
-  handleKeydown: (baseEvent: Event) => void
+  eventHandler: EventHandler
 ) => {
-  const listeners = targets.get(eventTarget);
+  const query = queries.get(eventTarget);
 
-  // If target already exists, add this listener to existing set of listeners
-  if (listeners) listeners.add(listener);
+  // If query already exists, add this listener to existing set
+  if (query?.listeners) query.listeners.add(listener);
   // Else, target is new, so create new event listener
   else {
-    targets.set(eventTarget, new Set([listener]));
-    eventTarget.addEventListener("keydown", handleKeydown);
+    queries.set(eventTarget, { eventHandler, listeners: new Set([listener]) });
+    eventTarget.addEventListener("keydown", eventHandler);
   }
 };
 
-const removeListener = (
-  eventTarget: EventTarget,
-  listener: Listener,
-  handleKeydown: (baseEvent: Event) => void
-) => {
-  const listeners = targets.get(eventTarget);
-  if (!listeners) return;
+const removeListener = (eventTarget: EventTarget, listener: Listener) => {
+  const query = queries.get(eventTarget);
+  if (!query) return;
+
+  const { eventHandler, listeners } = query;
 
   // Remove this listener from existing set of listeners
   listeners.delete(listener);
 
   // If there are no more listeners, remove the event listener
   if (listeners.size === 0) {
-    targets.delete(eventTarget);
-    eventTarget.removeEventListener("keydown", handleKeydown);
+    queries.delete(eventTarget);
+    eventTarget.removeEventListener("keydown", eventHandler);
   }
 };
 
-const handleTargetKeydown = (
+const handleEventTargetKeydown = (
   eventTarget: EventTarget,
   event: KeyboardEvent
 ) => {
-  const listeners = targets.get(eventTarget);
-  if (!listeners) return;
-
-  listeners.forEach((listener) => {
-    const { targetKeyCode, onChange } = listener;
-
-    // If the targetKeyCode is an array of keys, check if event.code matches any
-    if (Array.isArray(targetKeyCode)) {
-      if (targetKeyCode.includes(event.code)) onChange(event);
+  const query = queries.get(eventTarget);
+  if (!query) return;
+
+  // Note: While looping listeners here isn't ideal, it's still more
+  // performant than initializing a new event listener for each target
+  query.listeners.forEach((listener) => {
+    const { keyCode, onChange } = listener;
+
+    // If the event code matches the target key code, invoke the callback
+    if (
+      Array.isArray(keyCode)
+        ? keyCode.includes(event.code)
+        : event.code === keyCode
+    ) {
+      onChange(event);
     }
-
-    // Otherwise, just check if event.code matches the targetKeyCode
-    else if (event.code === targetKeyCode) onChange(event);
   });
 };
 
 /**
  * Hook for listening to window `keydown` events.
  *
- * @param targetKeyCode - The target key [code](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code) to listen for. Can also be an array of key codes.
+ * @param keyCode - The target key [code](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code) to listen for. Can also be an array of key codes.
  *
  * @param onChange - The callback to invoke when window `keydown` event is fired and the target key is pressed.
  *
@@ -84,27 +85,34 @@ const handleTargetKeydown = (
  * useKeydown("Escape", () => {}, { target: document });
  */
 const useKeydown = (
-  targetKeyCode: string | string[],
+  keyCode: string | string[],
   onChange: OnChangeEvent,
   config?: Config
 ) => {
+  const listenerRef = useRef<Listener>({ keyCode, onChange });
+
   useEffect(() => {
-    const eventTarget = getEventTargetFromConfig(config);
-    const listener: Listener = { targetKeyCode, onChange };
+    listenerRef.current = { keyCode, onChange };
+  }, [keyCode, onChange]);
+
+  useEffect(() => {
+    const eventTarget = getEventTargetFromTarget(config?.target);
+
+    const listener = listenerRef.current;
 
-    const handleKeydown = (baseEvent: Event) => {
+    const eventHandler = (baseEvent: Event) => {
       // As user can pass in a custom 'target', we need to check if the event is
-      // a KeyboardEvent before we can safely access the 'event.code' property
+      // a 'KeyboardEvent' before we can safely access the 'event.code' property
       const event = baseEvent as KeyboardEvent;
-      if (event.code) handleTargetKeydown(eventTarget, event);
+      if (event.code) handleEventTargetKeydown(eventTarget, event);
     };
 
-    addListener(eventTarget, listener, handleKeydown);
+    addListener(eventTarget, listener, eventHandler);
 
     return () => {
-      removeListener(eventTarget, listener, handleKeydown);
+      removeListener(eventTarget, listener);
     };
-  }, [onChange, config, targetKeyCode]);
+  }, [config?.target]);
 };
 
 export default useKeydown;

src/utils.ts

@@ -1,19 +1,5 @@
-import { Config } from "./types";
+import { Target } from "./types";
 
-export const getEventTargetFromConfig = (config?: Config) => {
-  if (config?.target) {
-    const { target } = config;
-
-    // If the target is a ref...
-    if ("current" in target) {
-      // ...and the ref has a value, return it
-      if (target.current) return target.current;
-    }
-
-    // Otherwise, return the target
-    else return target;
-  }
-
-  // If no target is specified, or it's null, return window
-  return window;
+export const getEventTargetFromTarget = (target?: Target) => {
+  return (target && "current" in target ? target.current : target) || window;
 };