Fix behavior of useHotkey when returned ref is re-attached to a diferent element.

[vilemj-Viclick]

Motivation

This pull request fixes issue #1116.

A brief look at the code

First of all: A test!

I added a test that fails if the issue #1116 is not fixed. This allows for any other solution to be adopted, should you not like my solution.

An auxiliary type

I added a type alias for KeyboardEventHandler (of the native kind - as opposed to the React.KeyboardEventHandler). It helps make the code more concise in the hook.

The hook (useHotkeys)

The most important change here is that the returned Ref is a RefCallback. The rest of the changes are driven by this crucial step.

What the callback

The returned callback leverages refs as callbacks behavior documented here. As stated in the next section, refCallbacks should be stable, hence the useCallback around it.

Why so many (use)refs?

To make the returned refCallback as stable as possible a few steps had to be made. As having any sort of refObject.current among the dependencies of any hook is generally a bad practice (because it won't work as expected), we need a way to update the attached handler reliably. This is simply achieved by making the attached handler stable for the life of the component using this hook. This means we can swap the active code of the handler (handleKeyUpRef.current) without using the DOM API and we can simply add and remove the listeners (stableHandleKeyUpRef.current) based on simpler conditions or events.

Other comments

The code could be further improved, by splitting the giant useEffect into smaller chunks and, honestly the dependencies could also be dropped with smart usage of refObjects. Updating the value of a refObject.currect when the user of your hook provides a new callback is virtually free. And if they want to make it stable or provide a new instance every time is their call.

If we manage to get this merged into your package in this form or a refined one, I'm willing to help you further improve this package, should you be interested. 😉

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-hotkeys-hook	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 9, 2024 3:10pm

[zeorin]

@vilemj-Viclick You might be interested in the useEventCallback pattern:

const useEventCallback = <F extends (...args: any[]) => any>(fn: F) => {
  const ref = useRef(fn);
  useLayoutEffect(() => {
    ref.current = fn;
  });

  return useCallback(
    (...args: Parameters<F>) => (0, ref.current)(...args) as ReturnType<F>,
    []
  );
};

It's often used when one wants a referentially stable function that should nevertheless close over values that might change from one render to the next (and those values change frequently, or one is creating a hook that accepts a callback, since your custom hook wouldn't get a dependency array flagged by eslint-plugin-react-hooks; this way you don't need a dependency array). It allows one to avoid having to do a lot of wiring up of refs.

It's worth noting that once it's considered stable, using React's built-in useEffectEvent would be a better option.

Having said that, in this particular case, the easiest way to avoid all of the ref objects is to store the DOM instance in state, use useState's updater function as the ref callback function directly, and simply react to the state when it changes as one normally does in idiomatic React: #1132.

[vilemj-Viclick]

No need for this PR anymore. A better version is proposed in #1132 .