Memoized callbacks from hooks

[dartess]

Hi!

Functions that return from the hook must be memoized because they can be used in effects, and the linter will require you to specify them depending. Now I had to memoize the functions myself, so as not to disable the linter. But after updating the linter, I still had to turn it off. Time to raise the problem.

Simple example (broken code):

    const {
        seconds,
        minutes,
        pause,
        restart,
    } = useTimer({
        expiryTimestamp: Date.now() + RE_SEND_AFTER_SECONDS * 1000,
        onExpire: () => setIsCanReSendSms(true),
    });

    useEffect(
        () => {
            if (screen === 'phoneConfirm') {
                restart(Date.now() + (RE_SEND_AFTER_SECONDS * 1000));
            } else {
                pause();
            }
        },
        [screen, pauseMemoized, restartMemoized],
    );

This will restart the effect with each renderer, because the functions are not memoized. Correct behavior - restarting the effect only when the variable "screen" is changed.

This can be added by removing callbacks from the dependencies. But this will force the developer to disable the linter, which is a bad decision in 99.9%.

Another solution is to add memoization yourself, but this is impractical and is now the reason for a new linter error. Now it looks like this.

    const {
        seconds,
        minutes,
        pause,
        restart,
    } = useTimer({
        expiryTimestamp: Date.now() + RE_SEND_AFTER_SECONDS * 1000,
        onExpire: () => setIsCanReSendSms(true),
    });

    // eslint-disable-next-line react-hooks/exhaustive-deps
    const pauseMemoized = useCallback(pause, []);
    // eslint-disable-next-line react-hooks/exhaustive-deps
    const restartMemoized = useCallback(restart, []);

    useEffect(
        () => {
            if (screen === 'phoneConfirm') {
                restartMemoized(Date.now() + (RE_SEND_AFTER_SECONDS * 1000));
            } else {
                pauseMemoized();
            }
        },
        [screen, pauseMemoized, restartMemoized],
    );

I think the problem needs to be solved radically. That is, in the source.

I was solving a similar problem nearby: pbeshai/use-query-params#46

[dylanmrowe]

Running into the same problem. Specifying a timer callback as a dependency infinitely triggers useEffect if any state is set inside the useEffect.

  const {
    seconds,
    minutes,
    pause,
    resume,
    restart
  } = useTimer({
    expiryTimestamp: convertDurationToExpiryTimestamp(durationInSeconds ? durationInSeconds : 60),
    onExpire: () => {}});

useEffect(() => {
  console.log("rerunning");
  if(activityState === "someValue") {
    restart();
    pause();
    setActivityState(newState);
  }
}, [activityState, pause, restart]);

In this example, the pause/restart callback changes with every render, which triggers the useEffect, which causes another render (via setActivityState), which changes the pause/restart callback, which calls the useEffect, etc...

[dylanmrowe]

Should be solved (for useTimer at least) by #44

[chybisov]

@amrlabib could you please look into this?

[amrlabib]

Done in v3.0.6