From d28305bf31ca86b161972824ff178a450094b0ca Mon Sep 17 00:00:00 2001 From: Paulo Date: Tue, 24 Jan 2023 16:49:52 +0000 Subject: [PATCH] Fix Hook Firing With Stale Values (#7) * Fix hook firing with old states * Tweak * Tweak * Tweak --- README.md | 2 +- package-lock.json | 4 ++-- package.json | 2 +- src/types.ts | 6 +++++- src/useKeydown.ts | 45 ++++++++++++++++++++++++++++----------------- 5 files changed, 37 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index ffe9927..3332f38 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ By default, the hook will listen to the `window` object. You can however listen import useKeydown from "@buildinams/use-keydown"; const elementRef = useRef(null); -useKeydown("Enter", () => {}, { target: elementRef }); // Do something on "Enter"... +useKeydown("Enter", () => {}, { target: elementRef }); ``` ## Requirements diff --git a/package-lock.json b/package-lock.json index 9a636c0..5f59a3c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@buildinams/use-keydown", - "version": "0.1.1", + "version": "0.1.2", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@buildinams/use-keydown", - "version": "0.1.1", + "version": "0.1.2", "license": "MIT", "devDependencies": { "@babel/preset-env": "^7.20.2", diff --git a/package.json b/package.json index 3b5476d..674b9be 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@buildinams/use-keydown", "description": "React hook for listening to custom keydown events.", - "version": "0.1.1", + "version": "0.1.2", "license": "MIT", "author": "Build in Amsterdam (https://www.buildinamsterdam.com/)", "main": "dist/index.js", diff --git a/src/types.ts b/src/types.ts index 879baf7..ae46342 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,3 +1,5 @@ +import { MutableRefObject } from "react"; + export type OnChangeEvent = ( /** The original keyboard event. */ event: KeyboardEvent @@ -24,7 +26,9 @@ export interface Listener { onChange: OnChangeEvent; } +export type ListenerRef = MutableRefObject; + export interface Query { eventHandler: EventHandler; - listeners: Set; + listenerRefs: Set; } diff --git a/src/useKeydown.ts b/src/useKeydown.ts index 99b9f0b..a008a91 100644 --- a/src/useKeydown.ts +++ b/src/useKeydown.ts @@ -1,37 +1,47 @@ import { useEffect, useRef } from "react"; -import { Config, EventHandler, Listener, OnChangeEvent, Query } from "./types"; +import { + Config, + EventHandler, + Listener, + ListenerRef, + OnChangeEvent, + Query, +} from "./types"; import { getEventTargetFromTarget } from "./utils"; const queries = new Map(); const addListener = ( eventTarget: EventTarget, - listener: Listener, + listenerRef: ListenerRef, eventHandler: EventHandler ) => { const query = queries.get(eventTarget); // If query already exists, add this listener to existing set - if (query?.listeners) query.listeners.add(listener); + if (query?.listenerRefs) query.listenerRefs.add(listenerRef); // Else, target is new, so create new event listener else { - queries.set(eventTarget, { eventHandler, listeners: new Set([listener]) }); + queries.set(eventTarget, { + eventHandler, + listenerRefs: new Set([listenerRef]), + }); eventTarget.addEventListener("keydown", eventHandler); } }; -const removeListener = (eventTarget: EventTarget, listener: Listener) => { +const removeListener = (eventTarget: EventTarget, listenerRef: ListenerRef) => { const query = queries.get(eventTarget); if (!query) return; - const { eventHandler, listeners } = query; + const { eventHandler, listenerRefs } = query; - // Remove this listener from existing set of listeners - listeners.delete(listener); + // Remove this listener from existing set of listenerRefs + listenerRefs.delete(listenerRef); - // If there are no more listeners, remove the event listener - if (listeners.size === 0) { + // If there are no more listenerRefs, remove the event listener + if (listenerRefs.size === 0) { queries.delete(eventTarget); eventTarget.removeEventListener("keydown", eventHandler); } @@ -44,10 +54,10 @@ const handleEventTargetKeydown = ( const query = queries.get(eventTarget); if (!query) return; - // Note: While looping listeners here isn't ideal, it's still more + // Note: While looping listenerRefs 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; + query.listenerRefs.forEach((listenerRef) => { + const { keyCode, onChange } = listenerRef.current; // If the event code matches the target key code, invoke the callback if ( @@ -91,6 +101,9 @@ const useKeydown = ( ) => { const listenerRef = useRef({ keyCode, onChange }); + // Note: We store these values as refs so that we don't re-run the effect + // when they change. Also, as we're using a ref, whenever 'eventHandler' + // fires it will always have access to the latest values useEffect(() => { listenerRef.current = { keyCode, onChange }; }, [keyCode, onChange]); @@ -98,8 +111,6 @@ const useKeydown = ( useEffect(() => { const eventTarget = getEventTargetFromTarget(config?.target); - const listener = listenerRef.current; - 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 @@ -107,10 +118,10 @@ const useKeydown = ( if (event.code) handleEventTargetKeydown(eventTarget, event); }; - addListener(eventTarget, listener, eventHandler); + addListener(eventTarget, listenerRef, eventHandler); return () => { - removeListener(eventTarget, listener); + removeListener(eventTarget, listenerRef); }; }, [config?.target]); };