React useApi Hook results in endless loop when used in useEffect

Question

I created a react hook to perform api calls inspired by this article.

The hook looks as followed:

function useApi<T>(apiFunc: (...args: Array<any>) => Promise<AxiosResponse<T>>) {
    const [data, setData] = useState<T | null>(null);
    const [error, setError] = useState("");
    const [loading, setLoading] = useState(false);
    const controllerRef = useRef(new AbortController());

    const request = async (...args: Array<any>) => {
        setLoading(true);
        try {
            const response = await apiFunc(...args);
            setData(response.data);
        } catch (error: any) {
            setError(error.message || 'Unknown Error');
        } finally {
            setLoading(false);
        }
    };

    return { request, cancel, data, error, loading };
};

Now I am using the hook in a component in the useEffect function:

const vacancyApi = useApi(getVacancy);

useEffect(() => {
    if (!!id) {
        vacancyApi.request(id);
    }
}, [id, vacancyApi]);

The problem is that when I add vacancyApi to the dependency array it results in an endless loop because vacancyApi is updated by the hook which then calls useEffect again.

I could just not add vacancyApi to the dependency array and it works but that seems ugly to me and not the right thing to do.

I also can't move the function out of the component because it's a hook and has to be created inside the component.

In the articles comments I read that it could be a solution to wrap the request function into a useCallback hook. But I haven't figured out how to do it properly, I still had the same problem.

const request = useCallback(async (...args: Array<any>) => {
    ...
}, [apiFunc]);

Is there a solution where I can call the api only if the id changes without removing the dependency of vacancyApi from the useEffect dependencies?

Answer 1

no more any

@JLRishe helps you solve your infinite loop but there's other issues with your code that are less straightforward. The biggest one being (...args: Array<any>) => ... effectively disabling TypeScript wherever this hook is used. Let's fix that using a more generic hook -

// hooks.ts

import { DependencyList, useState, useEffect } from "react"

type UseAsyncHookState<T> = {
    loading: boolean,
    error: null  | Error,
    data: null | T
}

function useAsync<T>(func:() => Promise<T>, deps: DependencyList) {
  const [state, setState] = useState<UseAsyncHookState<T>>({ loading: true, error: null, data: null })
  useEffect(
    () => {
      let mounted = true
      func()
        .then(data => mounted && setState({ loading: false, error: null, data }))
        .catch(error => mounted && setState({ loading: false, error, data: null }))
      return () => { mounted = false }
    },
    deps,
  )
  return state
}

export { useAsync }

Why that mounted var? We need to make sure we don't attempt to set state on an unmounted component. See the React guide for Fetching Data.

Now separate axios logic from your components with a reusable api module -

// api.js

import axios from "axios"

type User = {
  email: string,
  bio: string,
}

function getUser(id: number) {
  return axios.get<User>(`/users/${id}`)
}

// your other api functions ...

export { User, getUser, ... }

Type-safety restored. Now your components simply glue the pieces together without involving complex logic -

// UserProfile.tsx

import { useAsync } from "./hooks"
import { getUser } from "./api"

function UserProfile(props: { userId: number }) {
  const user = useAsync(          // ✅ inferred type: AsyncHookState<User>
    () => getUser(props.userId),  // async call
    [props.userId],               // dependencies
  )
  if (user.loading)
    return <Loading />
  if (user.error)
    return <Error message={user.error.message} />
  if (user.data)
    return <UserData user={user.data} />
}

no more nulls

The UseAsyncHookState type has nullable fields which means we have to be mindful in the way we attempt to access the result. Always checking loading first, then for the presence of error before accessing data. We can take advantage of TypeScript 5's new exhaustive switch..case completions by adjusting our hook's type -

type UseAsyncHookState<T> =
  | { kind: "loading" }
  | { kind: "error", error: Error }
  | { kind: "result", data: T }

function useAsync<T>(func:() => Promise<T>, deps: DependencyList) {
  const [state, setState] = useState<UseAsyncHookState<T>>({ kind: "loading" })
  useEffect(
    () => {
      let mounted = true
      func()
        .then(data => mounted && setState({ kind: "result", data }))
        .catch(error => mounted && setState({ kind: "error", error }))
      return () => { mounted = false }
    },
    deps,
  )
  return state
}

Now we can write these in any order without worry and TypeScript will warn us if we forget to handle all kinds -

function UserProfile(props: { userId: number }) {
  const user = useAsync(
    () => getUser(props.userId),  // async call
    [props.userId],               // dependencies
  )
  switch (user.kind) { // ✅ exhaustive switch..case
    case "loading":
      return <Loading />
    case "result":
      return <UserData user={user.data} />
    case "error":
      return <Error message={user.error.message} />
  }
}

multiple uses of useAsync

You can see a more complex use case where multiple useAsync calls are needed in this Q&A.

Answer 2

As you have it now, your useApi hook is going to create a new request function every time it's called, so you need to put that in a useCallback:

function useApi<T>(apiFunc: (...args: Array<any>) => Promise<AxiosResponse<T>>) {
    const [data, setData] = useState<T | null>(null);
    const [error, setError] = useState("");
    const [loading, setLoading] = useState(false);
    const controllerRef = useRef(new AbortController());

    const request = useCallback(async (...args: Array<any>) => {
        setLoading(true);
        try {
            const response = await apiFunc(...args);
            setData(response.data);
        } catch (error: any) {
            setError(error.message || 'Unknown Error');
        } finally {
            setLoading(false);
        }
    }, [apiFunc]);

    return { request, cancel, data, error, loading };
};

In order this to work, this also requires that apiFunc is a constant value from one render to the next.

Once you've ensured that request isn't constantly changing, you need to use that in the dependency array for your useEffect not the whole object that useApi returns:

const vacancyApi = useApi(getVacancy);

const { request: vacancyRequest } = vacancyApi;

useEffect(() => {
    if (id) {
        vacancyRequest(id);
    }
}, [id, vacancyRequest]);

I believe that will do the trick.