Misleading memoization in useDebouncedCallback

Heya, thanks for the lib!

I have conceptual disagreement with how it works wrt the returned callback though. My expectation, given the name useCallback and memoization being mentioned in the docs, that it behaves similar to useCallback. Or more specifically returns a new callback when the given callback changes. I saw this issue and agree with the assessment that a deps array wouldn't be the right approach.

Instead I'd propose changing callback from being a ref, to be part of the debounced useCallback deps here:

use-debounce/src/useDebouncedCallback.ts

Line 74 in dbaaef8

[maxWait, delay, cancelDebouncedCallback, leading, trailing]

(if that's feasible, you know the code better than me)

Here's some example code showing how it surprised me. There's a commented-out fix on line 17, but I don't think it's super elegant.
https://codesandbox.io/s/quirky-monad-hedgy?file=/src/App.js

Happy to create a PR if you agree with my assessment!

Hi @Gregoor

I'm so sorry for a such long response.

The solution with using ref for a callback was made to prevent unnecessary functions call and potential bugs for developers who're using this custom hook.

The best practice for your example is to move activePage from useCallback dependencies array to the argument so that your useCallaback would be:

  const fetchPage = useCallback(activePage => {
    setContent({ title: activePage, body: "Lorem Ipsum" });
  }, []);

Next step is to add activePage as a dependency to your useEffect:

  useEffect(() => {
    fetchPageDebounced(activePage);
  }, [fetchPageDebounced, activePage]);

All together:

import React, { useCallback, useState, useEffect } from "react";
import { useDebouncedCallback } from "use-debounce";

export default function App() {
  const [activePage, setActivePage] = useState("profile");
  const [content, setContent] = useState(null);

  const fetchPage = useCallback(activePage => {
    setContent({ title: activePage, body: "Lorem Ipsum" });
  }, []);

  const [fetchPageDebounced] = useDebouncedCallback(fetchPage, 500);

  useEffect(() => {
    fetchPageDebounced(activePage);
  }, [fetchPageDebounced, activePage]);

  return (
    <>
      <button onClick={() => setActivePage("profile")}>Profile</button>
      <button onClick={() => setActivePage("feed")}>Feed</button>
      {content && (
        <>
          <h3>{content.title}</h3>
          <p>{content.body}</p>
        </>
      )}
    </>
  );
}

Also, I've forked your example and changed it: https://codesandbox.io/s/xenodochial-hodgkin-1ypjo?file=/src/App.js:267-276

This solution has several pros:

You do not need to change a fetchPage function any more
You have explicit dependencies for your useEffect in one place

If we change useDebouncedCallback from ref to part of the state (or another caching mechanism like useCallback), it could affect excess user function calls.

That makes a ton of sense. Thank you very much for taking the time to write it out! Hooks apparently still haven't completely clicked for me yet. Anyway, much appreciated 🙂