Instantly share code, notes, and snippets.

Embed
What would you like to do?
import { useRef, useState, useEffect } from 'react';
// Usage
function App() {
const [hoverRef, isHovered] = useHover();
return (
<div ref={hoverRef}>
{isHovered ? '😁' : '☹️'}
</div>
);
}
// Hook
function useHover() {
const [value, setValue] = useState(false);
const ref = useRef(null);
const handleMouseOver = () => setValue(true);
const handleMouseOut = () => setValue(false);
useEffect(
() => {
const node = ref.current;
if (node) {
node.addEventListener('mouseover', handleMouseOver);
node.addEventListener('mouseout', handleMouseOut);
return () => {
node.removeEventListener('mouseover', handleMouseOver);
node.removeEventListener('mouseout', handleMouseOut);
};
}
},
[ref.current] // Recall only if ref changes
);
return [ref, value];
}
@gragland

This comment has been minimized.

Copy link
Owner

gragland commented Oct 30, 2018

@Guria Good call on moving useRef inside the hook and just returning it. Updated the code example.

@AndyBarron

This comment has been minimized.

Copy link

AndyBarron commented Oct 30, 2018

Now this is a useful hook. Some feedback:

  • I don't think ref can ever be null, so we can probably just check ref.current in useEffect
  • This won't update correctly if ref changes to a different DOM element. I think passing [ref.current] as the last argument to useEffect will fix this.
  • We can simplify useEffect to only check for ref.current once, since there's no unsubscribe work to do if the ref is empty.
  • Bind ref.current to local variable node via closure, so we are totally sure to call removeEventListener on the right thing (i.e. if ref.current is changed out of order, though this shouldn't happen in normal usage).
  • Minor change: ref -> hoverRef in App just to make it crystal clear what that ref is for.
  • Minor change: Importing React since that's usually necessary to use JSX.

Suggested code changes (NB: GitHub wouldn't let me leave a comment with emoji in it. Weird.):

import React, { useRef, useState, useEffect } from 'react';

// Usage
function App() {
  const [hoverRef, isHovered] = useHover();

  return (
    <div ref={hoverRef}>
      {isHovered ? 'Hovered! :D' : 'Hover me please :('}
    </div>
  );
}

// Hook
function useHover() {
  const [value, setValue] = useState(false);

  const ref = useRef(null);

  const handleMouseOver = () => setValue(true);
  const handleMouseOut = () => setValue(false);

  useEffect(() => {
    const node = ref.current;
    if (node) {
      node.addEventListener('mouseover', handleMouseOver);
      node.addEventListener('mouseout', handleMouseOut);

      return () => {
        node.removeEventListener('mouseover', handleMouseOver);
        node.removeEventListener('mouseout', handleMouseOut);
      };
    }

    // If we didn't set up any listeners, we won't need to unsubscribe from anything.
    return () => {};
  }, [ref.current]); // Ensure we remove and re-add the listeners if and only if the ref changes.

  return [ref, value];
}
@Grmiade

This comment has been minimized.

Copy link

Grmiade commented Oct 30, 2018

I tend to agree with the @AndyBarron 👍

  • No need to return empty function when unsubscription is useless
  • We can create handle functions directly in the useEffect function I think, to avoid create new handle functions at every render for nothing 😉
export default function useHover() {
  const [value, setValue] = useState(false);

  const ref = useRef(null);

  useEffect(
    () => {
      const handleMouseOver = () => setValue(true);
      const handleMouseOut = () => setValue(false);
      const element = ref && ref.current;

      if (element) {
        element.addEventListener("mouseover", handleMouseOver);
        element.addEventListener("mouseout", handleMouseOut);
        return () => {
          element.removeEventListener("mouseover", handleMouseOver);
          element.removeEventListener("mouseout", handleMouseOut);
        };
      }
    },
    [ref]
  );

  return [ref, value];
}
@AndyBarron

This comment has been minimized.

Copy link

AndyBarron commented Oct 31, 2018

Ah, good catch! Creating the functions inside the useEffect callback is way better.

I still think we can just do

const element = ref.current;

because useRef should never return null.

@Guria

This comment has been minimized.

Copy link

Guria commented Oct 31, 2018

@gragland did a bit more thoughts on explicit passing ref to hook. It looks nice to make hook to manage ref inside hook itself in this simple scenario. But it doesn't scale if we need another hook with ref for the same element. So it is better to revert it back I suppose.

@Guria

This comment has been minimized.

Copy link

Guria commented Oct 31, 2018

So, for instance, if we add useFocus into the same example we getting in a trouble:

function App() {
  const [hoverRef, isHovered] = useHover();
  const [focusRef, isFocused] = useFocus();

  return (
    <div ref={hoverRef /* we can't pass multiple refs here :( */}>
      {isHovered ? 'hovered' : 'not hovered'}
      {isFocused ? 'focused' : 'not focused'}
    </div>
  );
}
@Guria

This comment has been minimized.

Copy link

Guria commented Oct 31, 2018

@gragland I guess we should make multiple hooks reusing same ref example in a next issue of usehooks.com to show off why creating ref inside a hook doesn't scale. And probably we will get more ideas from community.

@AndyBarron

This comment has been minimized.

Copy link

AndyBarron commented Oct 31, 2018

That's a great point. I guess we should invert control and require hoverRef as a parameter to useHover.

@raunofreiberg

This comment has been minimized.

Copy link

raunofreiberg commented Oct 31, 2018

Why even rely on refs?

function useFocus() {
  const [focused, set] = useState(false);
  const binder = {
    onFocus: () => set(true),
    onBlur: () => set(false)
  };
  return [focused, binder];
}

function useHover() {
  const [hovered, set] = useState(false);
  const binder = {
    onMouseEnter: () => set(true),
    onMouseLeave: () => set(false)
  };
  return [hovered, binder];
}

function HoverableAndFocusable() {
  const [hovered, bindHover] = useHover();
  const [focused, bindFocus] = useFocus();
  return (
    <div>
      <input {...bindHover} {...bindFocus} />
      <h2>{hovered ? "Hovered" : "Not hovered"}</h2>
      <h2>{focused ? "Focused" : "Not focused"}</h2>
    </div>
  );
}
@gragland

This comment has been minimized.

Copy link
Owner

gragland commented Nov 2, 2018

@AndyBarron Thanks, added your suggestions! I think we can avoid returning at all in useEffect if no ref.current right?

@Guria Good point about it not scaling well if we need another hook with same ref. I like the idea of just passing in a hoverRef or even not using refs at all like @raunofreiberg suggests.

I'm trying to find the right balance between keeping these code recipes simple and accounting for the various edge cases that come up. Rather than refining the original recipe past a certain point, I'm wondering if it might be more informative to include multiple code variations on the site (could be a row of links above the code block that user can toggle between). This way they can learn things like "oh, for super simple situations I could just have a hook return a ref, but if that's too limiting I can also accept a ref as an argument". Basically a more user friendly way to learn things discussed in this gist. Anyway, thinking on this and open to feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment