I'm trying to create a basic keyboard shortcut system where I can register commands to execute when keys are pressed. Below is a simplified version of what I came up with. It suffers from a major problem, I believe due to stale closures. The keyboard shortcut trying to execute the logValueA function is the one that is causing trouble.

import { useState, useCallback, useEffect, useRef } from "react";

// A class to represent keyboard shortcuts.
// The goal is when you press the key, the stored action executes.
export class Shortcut {
    key: string;
    action: () => void;

    constructor(key: string, action: () => void) {
        this.key = key;
        this.action = action;
    }
}

// We store all shortcuts in an array so we can look them 
// up later by key and execute the associated action.
const shortcuts = new Array<Shortcut>();

export default function Test() {

    const [state, setState] = useState(false);
    const stateRef = useRef(state);
    useEffect(() => { stateRef.current = state}, [state]);

    function changeStateA() {
        setState(!state);
    }

    function changeStateB() {
        setState(state => !state);
    }

    function logValueA() {
        console.log('state: ' + state.toString());
    }

    function logValueB() {
        console.log('state: ' + stateRef.current.toString());
    }

    // I think this creates a closure on the initial 
    // "false" value of "state" so pressing 'a' any 
    // number of times results in state being true.
    shortcuts.push(new Shortcut('a', changeStateA));

    // Using "state => !state" in changeStateB fixes 
    // the issue and pressing 'b' changes state back 
    // and forth between true and false.
    shortcuts.push(new Shortcut('b', changeStateB));

    // Again, I think a closure is created so "state: false" 
    // is logged every time 'l' is pressed, and there is no 
    // easy and clean fix I'm aware of to see the current value.
    shortcuts.push(new Shortcut('l', logValueA));

    // I thought this meant "if state changes, execute the 
    // logValueA() function with the new value," but it doesn't 
    // fix anything. Again, "state: false" is logged every time.
    shortcuts.push(new Shortcut('c', useCallback(() => logValueA(), [state])));

    // By using a ref, logValueB always has access to the 
    // current value of state, but at the cost of having to 
    // create a separate ref variable, constantly update it 
    // in an effect, and access it via stateRef.current 
    // instead of just using state directly. It works but 
    // seems very weird and like it isn't a good solution.
    shortcuts.push(new Shortcut('r', logValueB));

    // When a key is pressed, find the matching shortcut 
    // and execute its "action" function.
    function handleKeyDown(event: KeyboardEvent) {
        const keyboardShortcut = 
            shortcuts.find(shortcut => shortcut.key === event.key);
        if (keyboardShortcut)
            keyboardShortcut.action();
    }

    // Register and unregister a handler for the "key down" event.
    useEffect(() => {
        document.addEventListener("keydown", handleKeyDown);
        return () => document.removeEventListener("keydown", handleKeyDown);
    }, []);

    // Display the current value of the state variable.
    return <div>state: {state.toString()}</div>;
}

I just want to use the "state" variable and not have to use a ref, update it in useEffect, and constantly access a ".current" property... is there any other way to create a keyboard shortcut system like this and avoid the stale closure problem?

I'm trying to create a basic keyboard shortcut system where I can register commands to execute when keys are pressed. Below is a simplified version of what I came up with. It suffers from a major problem, I believe due to stale closures. The keyboard shortcut trying to execute the logValueA function is the one that is causing trouble.

import { useState, useCallback, useEffect, useRef } from "react";

// A class to represent keyboard shortcuts.
// The goal is when you press the key, the stored action executes.
export class Shortcut {
    key: string;
    action: () => void;

    constructor(key: string, action: () => void) {
        this.key = key;
        this.action = action;
    }
}

// We store all shortcuts in an array so we can look them 
// up later by key and execute the associated action.
const shortcuts = new Array<Shortcut>();

export default function Test() {

    const [state, setState] = useState(false);
    const stateRef = useRef(state);
    useEffect(() => { stateRef.current = state}, [state]);

    function changeStateA() {
        setState(!state);
    }

    function changeStateB() {
        setState(state => !state);
    }

    function logValueA() {
        console.log('state: ' + state.toString());
    }

    function logValueB() {
        console.log('state: ' + stateRef.current.toString());
    }

    // I think this creates a closure on the initial 
    // "false" value of "state" so pressing 'a' any 
    // number of times results in state being true.
    shortcuts.push(new Shortcut('a', changeStateA));

    // Using "state => !state" in changeStateB fixes 
    // the issue and pressing 'b' changes state back 
    // and forth between true and false.
    shortcuts.push(new Shortcut('b', changeStateB));

    // Again, I think a closure is created so "state: false" 
    // is logged every time 'l' is pressed, and there is no 
    // easy and clean fix I'm aware of to see the current value.
    shortcuts.push(new Shortcut('l', logValueA));

    // I thought this meant "if state changes, execute the 
    // logValueA() function with the new value," but it doesn't 
    // fix anything. Again, "state: false" is logged every time.
    shortcuts.push(new Shortcut('c', useCallback(() => logValueA(), [state])));

    // By using a ref, logValueB always has access to the 
    // current value of state, but at the cost of having to 
    // create a separate ref variable, constantly update it 
    // in an effect, and access it via stateRef.current 
    // instead of just using state directly. It works but 
    // seems very weird and like it isn't a good solution.
    shortcuts.push(new Shortcut('r', logValueB));

    // When a key is pressed, find the matching shortcut 
    // and execute its "action" function.
    function handleKeyDown(event: KeyboardEvent) {
        const keyboardShortcut = 
            shortcuts.find(shortcut => shortcut.key === event.key);
        if (keyboardShortcut)
            keyboardShortcut.action();
    }

    // Register and unregister a handler for the "key down" event.
    useEffect(() => {
        document.addEventListener("keydown", handleKeyDown);
        return () => document.removeEventListener("keydown", handleKeyDown);
    }, []);

    // Display the current value of the state variable.
    return <div>state: {state.toString()}</div>;
}

I just want to use the "state" variable and not have to use a ref, update it in useEffect, and constantly access a ".current" property... is there any other way to create a keyboard shortcut system like this and avoid the stale closure problem?

• reactjs
• closures

2 Answers 2

After many hours and much reading, I have something that works. I'm posting in case it might help someone else... as well as to get any feedback people might have on the solution. It seems to work and I like it much better than what I had before because it let's me get rid of all the ref objects and effects to keep them up to date (as well as the need to constantly use the .current property). I still have to do those things but only one time in a single encapsulated place so the rest of the code base isn't littered with it.

I'll comment a little in the code below about how it works but since I still don't 100% get it, you are probably better off just reading the excellent article by Dan Abramov that informed the solution:

https://overreacted.io/making-setinterval-declarative-with-react-hooks/

import { useEffect, useRef, useState } from "react";

export class Shortcut {
    key: string;
    action: () => void;

    constructor(key: string, action: () => void) {
        this.key = key;
        this.action = action;
    }
}

const shortcuts = new Array<Shortcut>();

function useShortcut(key: string, action: () => void) {
    // This is the key to solving the issue. Basically we create a ref to the 
    // action and update on each render so it has access to up-to-date state. 
    const actionRef = useRef(action);

    // Here we keep it up to date with the latest state.
    useEffect(() => {
        actionRef.current = action;
    });

    // And here we create a wrapper that always calls the ref to the function 
    // which has the up-to-date state.
    function executeAction() {
        actionRef.current();
    }

    // And now create the shortcut that calls the above function.
    shortcuts.push(new Shortcut(key, executeAction));
}


export default function Test() {

    const [state, setState] = useState(false);

    function changeState() {
        setState(state => !state);
    }

    function logValue() {
        console.log('state: ' + state.toString());
    }

    useShortcut('s', changeState);
    useShortcut('l', logValue);

    function handleKeyDown(event: KeyboardEvent) {
        const keyboardShortcut =
            shortcuts.find(shortcut => shortcut.key === event.key);
        if (keyboardShortcut)
            keyboardShortcut.action();
    }

    // Register and unregister a handler for the "key down" event.
    useEffect(() => {
        document.addEventListener("keydown", handleKeyDown);
        return () => document.removeEventListener("keydown", handleKeyDown);
    }, []);

    // Display the current value of the state variable.
    return <div>state: {state.toString()}</div>;
}

Your code has a two main:

1. Calling shortcuts.push() at the top-level of your Test component will result in multiple Shortcuts being pushed to your array when Test re-renders. Each re-render of your component results in your function executing, so you end up pushing multiple actions with the same key.

2. Your use effect doesn't declare handleKeyDown as a dependency. Since your setup function uses this function in its body, you should be declaring it in the dependency array.

One way you can fix this is by placing your functions that rely on your state within a useEffect that is dependent on your state. That way, your function will be recreated and a new event listener will be added (with the old one will be removed) on each update of your state, allowing your event handler to call the new function and have visibility of the new state. You also don't need to list the functions are dependencies to your useEffect as they're now defined within the setup function itself. You can also move your shortcuts array into there also so that each shortcut gets associated with the new function reference that has visibility of the latest state:

const { useState, useCallback, useEffect } = React;

class Shortcut {
  constructor(key, action) {
      this.key = key;
      this.action = action;
  }
}

function Test() {
  const [state, setState] = useState(false);

  useEffect(() => {
    function changeStateA() {
      setState(!state);
    }

    function logValueA() {
      console.log('state: ' + state);
    }

    const shortcuts = [
      new Shortcut('a', changeStateA),
      new Shortcut('l', logValueA)
    ];

    function handleKeyDown(event) {
      const keyboardShortcut = shortcuts.find(shortcut => shortcut.key === event.key);
      if (keyboardShortcut)
          keyboardShortcut.action();
    }

    document.addEventListener("keydown", handleKeyDown);
    return () => document.removeEventListener("keydown", handleKeyDown);
  }, [state]);

  // Display the current value of the state variable.
  return <div>state: {state.toString()}</div>;
}

ReactDOM.createRoot(document.body).render(<Test />);

<script src="https://cdnjs.cloudflare/ajax/libs/react/18.3.1/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare/ajax/libs/react-dom/18.3.1/umd/react-dom.production.min.js"></script>

Alternatively, if you don't like putting everything in useEffect() and want to stick with a similar useEffect to what you currently have now, you can extract your functions and other dependencies as below (you could also do a mix the top example the below example if you wanted):

const { useState, useCallback, useEffect, useMemo } = React;

class Shortcut {
    constructor(key, action) {
        this.key = key;
        this.action = action;
    }
}

function Test() {
    const [state, setState] = useState(false);
    
    // Here we use useCallback to only recreate these functions on a new render when
    // `state` changes. If the component rerenders but the state hasn't change, the function reference
    // will remain as is, and the useEffect below won't execute again
    // Without the useCallback, the function definition would be treated as a new function reference
    // each render causing the useMemo below to trigger on every rerender.
    const changeStateA = useCallback(() => {
      setState(!state);
    }, [state]);

    const logValueA = useCallback(() => {
      console.log('state: ' + state);
    }, [state]);
    
    const shortcuts = useMemo(() => [
      new Shortcut('a', changeStateA),
      new Shortcut('l', logValueA)
    ], [changeStateA, logValueA]);
    
    const handleKeyDown = useCallback((event) => {
      const keyboardShortcut = shortcuts.find(shortcut => shortcut.key === event.key);
      if (keyboardShortcut)
          keyboardShortcut.action();
    }, [shortcuts]);
    
    useEffect(() => {
      document.addEventListener("keydown", handleKeyDown);
      return () => document.removeEventListener("keydown", handleKeyDown);
    }, [handleKeyDown]);

    // Display the current value of the state variable.
    return <div>state: {state.toString()}</div>;
}

ReactDOM.createRoot(document.body).render(<Test />);

<script src="https://cdnjs.cloudflare/ajax/libs/react/18.3.1/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare/ajax/libs/react-dom/18.3.1/umd/react-dom.production.min.js"></script>

Why does this happen?

I think it should be fairly clear that if you have code like below, both functions would log different values:

const Example = (s) => {
  const state = s;
  function foo() {
    console.log(state);
  }
  foo();
}

Example(1); // first render (with initial state)
Example(2); // next render (with new state)

Even though there is only one foo function in the code above, it's created twice for both function executions, similarly, the state is also created twice, once for the first function execution's scope and again for the second function execution's scope. Each foo function that is created access the different state variables based on the scope that the foo function was created in (the first invocation accessing the value 1, and the second accessing the value 2). Here we say that the foo function "closes over" the state variable as it can access the state variable from its surrounding scope.

In React, things aren't too dissimilar:

A state update causes a rerender, the rerender results in a function call to Test and this results in a new scope with new variables and functions (so a state update causes a function execution like in the above example).

That means that in your code, new handleKeyDown and state variables are created on each render, with the state holding the new updated state for that render, and the handleKeyDown holding the function object created as part of that render (which can only "see" the variables values as they are currently in the current scope).

When you have a useEffect() with an empty dependency array [], it runs after the initial render, so the setup function ends up adding your event listener using the first version of the handleKeyDown that was created in the initial render's scope. That means on future renders, your event listener will continue to invoke the handleKeyDown function reference that was created in the original scope, so it doesn't have visibility of the new variables and values that are created (such as the updated state values). By changing your useEffect dependencies (for example to use hanldeKeyDown), your useEffect will be able to remove the old listener (ie: it will stop invoking the old function reference), and will add a new listener to invoke the "newer" handleKeyDown function which has access to the current render's variables.