Discussion on: 8 Awesome React Hooks

[EECOLOR]

Great to see you are embracing the hook feature! And awesome you share your findings.

There are however a few small problems with your hooks.

useTimeout and useInterval

You are storing the callback into a ref which is great because you make sure you will allways call the latest version. You however set it using useEffect. This means if the callback changes, the change will be picked up in the next frame. Theoretically there is a chance that the callback has changed when the timeout (or interval) fires. You can fix it like this:

function useTimeout(callback, timeout) {
  const callbackRef = React.useRef(null)
  callbackRef.current = callback

  React.useEffect(
    () => {
      if (delay !== null) return
      const id = setTimeout(() => callbackRef.current(), delay)
      return () => clearTimeout(id)
    },
    [delay]
  )
}

usePrevious

This might actually be expected behavior, but this does not actually show the previous 'different' value. If you component rendered again with no value change usePrevious would return the same value. So the actual name should be useValueFromPreviousRender.

You could see this behavior when you add another state to the MoneyCount component that toggles a boolean when you click another button. The hook would be a bit more involved if you wanted to show the previous value after a change.

useClickInside and useClickOutside

This hook adds and removes the event listener on each render. It would be smart to use a similar strategy to useTimeout with a ref for the callback. On top of that, its a good idea to set the dependency array.

const callbackRef = React.useRef(null)
callbackRef.current = callback
React.useEffect(
  () => {
    window.addEventListener('click', handleClick)
    return () => window.removeEventListener('click', handleClick)
    function handleClick(e) {
      if (ref.current && ref.current.contains(e.target) callbackRef.current()
    }
  },
  []  
)

As a side note: in theory the ref you are given could be a function ref, if that is the case your code would fail. The easiest way to guard for this is by creating and returning the ref from the hook itself.

useFetch

I think this is already mentioned but it only executes once during the lifetime of the component.

In real world applications you often want to know the response status code. I think it would be a great addition to add that.

useComponentDidMount and useComponentDidUnmount

These hooks capture the callback that is passed in. This means that if the callback changes, those changes will not be picked up. For useComponentDidMount this will most likely never cause a problem, for useComponentDidUnmount I can see this causing problems more often.

If someone in my company would create these hooks I would say that these hooks do not have the correct name. Their names refer to the component lifecycle and that is something hooks helps us to move away from. We want to focus on the actual effect and its lifecycle. On top of that, for new developers (that have only been exposed to React with hooks) the concepts of mount and unmount are unknown.

---

All in all great examples. I hope my comments are of value to you. I strongly suggest you install the eslint plugin as this would warn you for most problems.

[Simon Holdorf]

Thank you very much for this detailed comment, this helps a lot!

[wangyouhua]

Nice comment! But I have a puzzle: why borther to use

const callbackRef = React.useRef(null);
callbackRef.current = callback

I think we can eliminate the use of useRef, just use the callback argument in the place callbackRef.current.

[EECOLOR]

@wangyouhua Great question: What is the difference between these versions?

function useTimeout1(callback, timeout) {
  const callbackRef = React.useRef(null)
  callbackRef.current = callback

  React.useEffect(
    () => {
      if (!timeout && timeout !== 0) return
      const id = setTimeout(() => callbackRef.current(), timeout)
      return () => clearTimeout(id)
    },
    [timeout]
  )
}

// vs

function useTimeout2(callback, timeout) {
  React.useEffect(
    () => {
      if (!timeout && timeout !== 0) return
      const id = setTimeout(() => { callback() }, timeout)
      return () => clearTimeout(id)
    },
    [timeout] // you will get a lint warning here about `callback`
  )
}

// vs

function useTimeout3(callback, timeout) {
  React.useEffect(
    () => {
      if (!timeout && timeout !== 0) return
      const id = setTimeout(() => { callback() }, timeout)
      return () => clearTimeout(id)
    },
    [timeout, callback]
  )
}

You can see the difference in the usage, in this example I want to log the value of someState after 5 seconds:

const [someState, setSomeState] = React.useState(0)
useTimeout1(
  () => { console.log(someState) },
  5000
) // will correctly log the value of `someState` after 5 seconds

// vs

const [someState, setSomeState] = React.useState(0)
const callback = React.useCallback(() => { console.log(someState) }, someState)
useTimeout2(
  callback,
  5000
) // this will always log 0, even if `someState` changes 

// vs

const [someState, setSomeState] = React.useState(0)
const callback = React.useCallback(() => { console.log(someState) }, someState)
useTimeout3(
  callback,
  5000
) // when `someState` changes, you need to wait another 5 seconds to see the results

As you can see version 2 and 3 have a bit different behavior. Version 2 is broken (and caught by the linting rules). Version 3 has unexpected behavior, it resets the timeout when someState is changed.

This has to with the 'capturing' of the callback. Once you pass a function to setTimeout that function can not be changed. Primitive values it references are captured. In order to get the expected result and don't put the burden of that correct handling on the user of the hook, we keep a reference to the 'current' callback.

So if you are using callbacks in hooks this is a great pattern:

const callbackRef = React.useRef(null)
callbackRef.current = callback

useSomeAsynchronousHook(
  () => { someAsynchrounousMethod(() => { callBackRef.current() }) },
  [] // there is no longer a dependency on the callback
)