Object.is equality check in react hook is causing multiple versions of the same function

Question

I'm not sure what the correct fix is to stop the following scenario. I've created this codesandbox to highlight the problem.

I have this hook and here is a scaled down version:

export const useAbortable = <T, R, N>(
  fn: () => Generator<Promise<T>, R, N>,
  options: Partial<UseAbortableOptions<N>> = {}
) => {
  const resolvedOptions = {
    ...DefaultAbortableOptions,
    ...options
  } as UseAbortableOptions<N>;
  const { initialData, onAbort } = resolvedOptions;
  const initialState = initialStateCreator<N>(initialData);
  const abortController = useRef<AbortController>(new AbortController());
  const counter = useRef(0);

  const [state, dispatch] = useReducer(reducer, initialState);

  const runnable = useMemo(
    () =>
      makeRunnable({
        fn,
        options: { ...resolvedOptions, controller: abortController.current }
      }),
    // eslint-disable-next-line react-hooks/exhaustive-deps
    [counter.current]
  );

  const runner = useCallback(
    (...args: UnknownArgs) => {
      console.log(counter.current);

      dispatch(loading);

      runnable(...args)
        .then(result => {
          dispatch(success<N>(result));
        })
        .finally(() => {
          console.log("heree");
          counter.current++;
        });
    },
    [runnable]
  );

  return runner;
};

The hook takes a function and options object and as they are recreated on each render, and the hooks use Object.is comparison, it was creating a new version of the returned function no matter what I do.

So I have hacked it like this, to use a counter:

  const runnable = useMemo(
    () =>
      makeRunnable({
        fn,
        options: { ...resolvedOptions, controller: abortController.current }
      }),
    // eslint-disable-next-line react-hooks/exhaustive-deps
    [counter.current]
  );

I have had to silence the linter to make this possible.

What the linter suggests is this:

  const runnable = useMemo(
    () => makeRunnable({ fn, options: { ...resolvedOptions, controller: abortController.current } }),
    [fn, resolvedOptions],
  );

But fn and resolvedOptions are causing a new runnable function to be created each time.

It is a real pain having to wrap everything in useCallback, useMemo, and friends.

I've had a look at other fetch libraries and they are doing other things like this, like JSON.stringify dependency arrays to get around this problem.

I like hooks but the Object.is equality check is killing the whole paradigm.

What would be the correct way for me to use the dependency array correctly so I don't get a new function each time and also keep the linter happy? The 2 requirements seem to add odds with each other.

Answer 1

You must note that you need not provide any hack to get around the callback problem

ESLint warnings about missing dependency are there to help users avoid mistakes make unknowingly and not to be enforced.

Now in your case

If you look at your useAbortable function, you are passing a generator function and an options object, now both of them are created on every re-render.

You can memoize the options and function passed to useAbortable to avoid issues with dependencies

• If you use callback pattern for setMessages, you can create onAbort once only by providing [] dependency to useCallback

• The generator function depends on a time delay from state so you can create it with useCallback and provide delay as a dependency

      const onAbort = useCallback(() => {
        setMessages(prevMessage => (["We have aborted", ...prevMessage]));
      }, []); // 


      const generator = useCallback(
        function*() {
          const outsideLoop = yield makeFetchRequest(delay, "outside");

          processResult(outsideLoop);

          try {
            for (const request of requests) {
              const result = yield makeFetchRequest(delay, `${request.toString()}`);

              processResult(result);
            }
          } catch (err) {
            if (err instanceof AbortError) {
              setMessages(["Aborted"]);
              return;
            }
            setMessages(["oh no we received an error", err.message]);
          }
        },
        [delay, processResult]
      );

      const options = useMemo(() => ({ onAbort }), [onAbort]);
      const { run, state, abortController, reset, counter, ...rest } = useAbortable<
        Expected,
        void,
        Expected
      >(generator, options);

Now inside useAbortable you need not worry about fn or options changing as they will only change when they absolutely have too if we implement it like above

So you your runnable instance in useAbortable can be clearly created with the correct dependencies

const resolvedOptions = useMemo(() => ({
    ...DefaultAbortableOptions,
    ...options
  }), [options]);

  const runnable = useMemo(
    () =>
      makeRunnable({
        fn,
        options: { ...resolvedOptions, controller: abortController.current }
      }),
    [fn, resolvedOptions]
  );

Working demo

Answer 2

The problem is with resolvedOptions and fn and not with runnable. You should rewrite runnable dependencies as the linter suggests and modify resolvedOptions to also be a memoized.

// I assume that `DefaultAbortableOptions` is defined outside of the `useAbortable` hook
const resolvedOptions = useMemo(() => ({
  ...DefaultAbortableOptions,
  ...options
}), [options]};

The hook takes a function and options object and as they are recreated on each render, and the hooks use Object.is comparison, it was creating a new version of the returned function no matter what I do.

When you consume your hook you should also pay attention to not recreate stuff unnecessarily and use useMemo and useCallback for data and functions defined inside your components.

function MyComponent () {
  const runnable = useCallback(() => {}, [/*let the linter auto-fill this*/])
  const options = useMemo(() => ({}), [/*let the linter auto-fill this*/])
  const runner = useAbortable(runnable, options)
}

This way the runner function will only be recreated when it really needs to be (when the dynamic dependencies of runnable and options change).

You have to go all-in with hooks if you use them and really wrap everything in them to make things work without shady bugs. I personally dislike them because of these traits.

Extra note: as the React docs point out, useMemo is not guaranteed to run only when its dependencies change, it may run on any render and cause runner to be recreated. In the current version of React this does not happen but it may happen in the future.