[BUG] useDebounceCallback doesn't seem to memoize the callback function

[marcebdev]

Describe the bug

Debounce is called multiple times if you use a non-memoized function, like an inline arrow function or something you'd typically put into a useCallback (so not passing a setState or similar directly).

Using a custom arrow function like you'd use in useCallback or something similar is not uncommon nor unresonable. Many other libraries show better examples (than passing a useState directly) and use something like this as an example e.g. https://mantine.dev/hooks/use-debounced-callback/

So the issues are many fold:

1. a stronger example is needed, as we can see useState is basically cheating
2. improve tests as critical test cases are missed
3. fix bug (it is not a useCallback variant hook without this fix)

To Reproduce

const Test = () => {
  const [value, setValue] = useState<string>('')
  const [, setDebouncedValue] = useState('')

  const debounce = useDebounceCallback((val: string) => {
    console.log('debounced', val)
    if (value) setDebouncedValue(val)
  }, 1500)

  return (
    <input
      value={value}
      onChange={({ target: { value } }) => {
        setValue(value)
        debounce(value)
      }}
    />
  )
}

Expected behavior

debounced functions should be called once

Right now that console will print the following if "123" is typed

debounced 1
debounced 12
debounced 123

Additional context

No response

[marcebdev]

I don't mean to be bitter or rude and can def appreciate the contributions to this library, but given such a critical/misdocumented issue with this hook and seeing other debounce hook related issues I think I'll step away from using this library for now.

FWIW this library has one of my fav set of functions but if they don't work reliably, it's hard to confidently rely on them

[bestickley]

This is critical. @marcebdev, thanks for documenting.

[Aleuck]

Maybe this hook does intend to memoize the function. For example if you use it to debounce a state setter, the setter is stable by default and memoizing it would be inefficient.

Maybe you should wrap it over useCallback to memoize it before passing it over to this hook.

[dmythro]

Yeah, I've spent a bit of time on this and then switched to my regular useCallback + debounce approach. So weird.

[nabilfatih]

you have to wrap your function with useCallback before you pass to the useDebounceCallback. then it will works as you expected... i think thats why the hook called useDebounceCallback

[dmythro]

you have to wrap your function with useCallback before you pass to the useDebounceCallback. then it will works as you expected... i think thats why the hook called useDebounceCallback

No, if it doesn't memo it's not a hook, it's just a debounce function.

[Aleuck]

you have to wrap your function with useCallback before you pass to the useDebounceCallback. then it will works as you expected... i think thats why the hook called useDebounceCallback

No, if it doesn't memo it's not a hook, it's just a debounce function.

this is not the definition of what a hook is. Not all hooks are for memoization.
maybe you want to debounce a function that is already stable so there is no need to memoize. The implementation is good AS IS because you can choose whether you need to memoize or not.

[dmythro]

you have to wrap your function with useCallback before you pass to the useDebounceCallback. then it will works as you expected... i think thats why the hook called useDebounceCallback

No, if it doesn't memo it's not a hook, it's just a debounce function.

this is not the definition of what a hook is. Not all hooks are for memoization. maybe you want to debounce a function that is already stable so there is no need to memoize. The implementation is good AS IS because you can choose whether you need to memoize or not.

I think you're getting it wrong. The catch is not the hook itself, but the naming which confuses a lot. React's useCallback returns a memoized callback, which doesn't re-create each render, so everything ending with "Callback" suggests this is a memoized function and is safe to pass deeper to props.

Otherwise, what's the point in useDebounceCallback naming if it just does debounce? Or at maybe useDebounce.

In general you can name anything however you like. But I hope it explains why this causes confusion.

[marcebdev]

The implementation is good AS IS because you can choose whether you need to memoize or not.

It's not, for one as originally pointed out in the issue, the way this was found was noticing debouncing does not work correctly. In order for debouncing to work, the input must be memoized. The hook is not written in a way that is stable.

Secondly, even if the above is true, there needs to be documentation of this. There's no mention of this in the docs, which is aside the fact that the example needs to be stronger to clearly demonstrate proper/improper use. Though for a hook in a lib, this is not at all an uncommon use-case and a lib should likely be made with flexibility and safety in mind imo. Not with the possibility for someone to accidentally create a "fake" debounce.

Additionally, regarding memoization specifically, for a lib it's likely ideal to have proper memoization built-in by default, as after all, the whole point of a hook lib is to have a strong, performant, ideally tested version of a hook that has eyes on it from the broader community vs implementing your own. It should be batteries included imo. But I believe this is besides the point, as I'm not sure there's a way to memoize an inputted func from inside the hook, it's just that the hook itself does not seem to be stable/safe if the input func is not memoized

[lolmaus]

I have decided to use this function and I wrapped my callback with useCallback in the first place. And it still didn't debounce!

I spent a couple hours of hair pulling to find out that useDebounceCallback resets when the opts object has changed.

This works:

  const search = useCallback(() => {/*...*/}, []);
  const debounceOpts = useMemo(() => ({ leading: true }), []);
  const searchDebounced = useDebounceCallback(search, 500, debounceOpts);

This won't debounce (inside a React component, obviously):

  const search = useCallback(() => {/*...*/}, []);
  const searchDebounced = useDebounceCallback(search, 500, { leading: true });

I'm really frustrated with the requirement to memoize the options hash. Is it just me or is it bad design of the library?

[Pasalietis]

I'm really frustrated with the requirement to memoize the options hash. Is it just me or is it bad design of the library?

Had exactly the same problem. And it's annoying to memorize in all the places. I could understand why it may not memorize callback, as you may want to reset the debounce on callback change. But for options, it doesn't make sense

[lolmaus]

I've created a separate issues regarding the options object in useDebounceCallback:
#703

And here's a fix:
#704

Please upvote the issue and the PR. 😇