window.localStorage.setItem not executing in onkeydown callback

Viewed 87

I am trying to implement an onkeydown event handler into my react app to close the settings screen by pressing ESC:

  useEffect(() => {
    document.addEventListener("keydown", handleKeyPress, false);

    return () => {
      document.removeEventListener("keydown", handleKeyPress, false);
    };
  }, [])

  const saveSettings = () => {
    window.localStorage.setItem("storage.ls.version", CURRENT_LOCALSTORAGE_SCHEMA_VERSION);

    window.localStorage.setItem("auth.token", authInput);
    window.localStorage.setItem("filter.class", classInput);
    window.localStorage.setItem(
      "filter.subjects",
      JSON.stringify(subjectsInput.filter((v: string) => v.trim() !== ""))
    );

    props.dismiss();
  };

  const handleKeyPress = (event: KeyboardEvent) => {
    if (event.key === "Escape") {
      saveSettings();
    }
  };

The callback does execute, props.dismiss() (in saveSettings) runs just fine. But the changes don't seem to be saved to localStorage. However, when I use the same saveSettings function on a press of a button (which I have been doing already before), it works and saves as expected.

I wasn't able to find something, but is there a restriction that prevents usage of localStorage in event callbacks? Or is there another reason it doesn't work as expected?


See the comment thread under the question; here's the code that reads the localStorage:

const [authInput, setAuthInput] = useState(window.localStorage.getItem("auth.token") ?? "");

const [classInput, setClassInput] = useState(window.localStorage.getItem("filter.class") ?? "");
const [subjectsInput, setSubjectsInput] = useState(
  JSON.parse(window.localStorage.getItem("filter.subjects") ?? "[]")
);
2 Answers

Couple of issues here that are contributing to your issue:

  1. The handleKeyPress function is bound as the event handler for the keydown event in the first useEffect, and so it holds the context at that point in time when it runs. So say you have values in local storage already like authInput = apple, classInput = english and subjectsInput = '', and you change the value to authInput = banana, then you hit ESC, the value that will be stored in authInput will be apple again because that was the initial value of authInput when you attached handleKeyPress to the keydown event in the DOM, and will always be apple! So it 'looks' as if the value isn't changing no matter what you do. To fix this, you need to add handleKeyPress as a dependency of that first useEffect(). This function context changes with every state change, because the saveSettings() function has different values for authInput, classInput and all the other values it's referencing, whenever the state changes. So the first change you need to make is the following:
  useEffect(() => {
    document.addEventListener("keydown", handleKeyPress, false);

    return () => {
      document.removeEventListener("keydown", handleKeyPress, false);
    };
  }, [handleKeyPress]); // add the function as a dependency
  1. handleKeyPress doesn't need to change if the values / functions it's referencing inside doesn't change. So use React's useCallback() to specify this relationship:
  const handleKeyPress = useCallback((event) => {
    if (event.key === "Escape") {
      saveSettings();
    }
  }, [saveSettings]);

See https://reactjs.org/docs/hooks-reference.html#usecallback for more on this.

  1. Lastly, apply the same treatment to saveSettings() because that function depends on three values:
  const saveSettings = useCallback(() => {
    window.localStorage.setItem("storage.ls.version", CURRENT_LOCALSTORAGE_SCHEMA_VERSION);

    window.localStorage.setItem("auth.token", authInput);
    window.localStorage.setItem("filter.class", classInput);
    window.localStorage.setItem(
      "filter.subjects",
      JSON.stringify(subjectsInput.filter((v: string) => v.trim() !== ""))
    );

    props.dismiss();
  }, [authInput, classInput, subjectsInput]);

Whenever the state of authInput, classInput or subjectsInput changes, React will run the cleanup that you specified in the useEffect() and re-run the effect with the new handler that has the correct state values in its run-time context.

If you still don't understand why this is happening, that's ok! Don't fret - I highly encourage you to read how Closures work in Javascript: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Closures or https://javascript.info/closure

Lastly, I want to mention some code smells for you that you might want to cleanup:

  1. When setting the initial states for all of your state variables, wrap the expensive operation of reading from localStorage in a function so React only runs it once, instead of every re-render.
const [authInput, setAuthInput] = useState(window.localStorage.getItem("auth.token") ?? "");

// instead do this
const [authInput, setAuthInput] = useState(() => window.localStorage.getItem("auth.token") ?? "");
  1. Is subjectsInput guaranteed to be an array when you run filter() on it in saveSettings()? You might want to account for this.

I have a react hook that might help you.

useKeyboardEvents.ts

import { useEffect } from "react";

export type UseKeyboardEventsProps = {
    [key in keyof WindowEventMap]?: EventListenerOrEventListenerObject;
};

export const useKeyboardEvents = (value: UseKeyboardEventsProps) => {
    useEffect(() => {
        Object.entries(value).forEach((item) => {
            const [key, funct] = item;
            window.addEventListener(
                key,
                funct as EventListenerOrEventListenerObject,
                false
            );
        });

        return () => {
            Object.entries(value).forEach((item) => {
                const [key, funct] = item;
                window.removeEventListener(
                    key,
                    funct as EventListenerOrEventListenerObject,
                    false
                );
            });
        };
    }, [value]);
};

Use it like follows. Note: I used keydown like in your code, but I think it would be better to listen to keyup. (replace your useEffect with the following)

  useKeyboardEvents({
    keydown: (event: any) => {
      if (event.key === 'Escape') {
        saveSettings();  
      }
    }
  });
Related