Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix useApi to not return a new axios instance every render #20

Merged
merged 1 commit into from Sep 1, 2022

Conversation

ko-lem
Copy link
Contributor

@ko-lem ko-lem commented Jul 29, 2022

Problem

useApi creates and returns a new axios instance every render. This makes it unusable as a useEffect dependency which is unfortunate when working in a project that uses the recommended exhaustive-deps lint rule.

For example:

function Component () {
  const api = useApi();
  const [response, setResponse] = useState(false);

  useEffect(() => {
    api.get("/api/verify-token")
    .then((res) => {
      setResponse(res.data);
    });
  }, []);

  ...
}

The above code works fine. But in a project that uses exhaustive-deps, you will be forced to add in api in the array. That is a problem since it triggers never ending API requests.

function Component () {
  const api = useApi();
  const [response, setResponse] = useState(false);

  useEffect(() => { // This will be executed every render while also triggering a re-render.
    api.get("/api/verify-token")
    .then((res) => {
      setResponse(res.data);
    });
  }, [api]);

  ...
}

The way we deal with this is to just leave out api from the dependency array and add // eslint-disable-next-line react-hooks/exhaustive-deps above. This does work but is not ideal in my opinion, especially when there are other dependencies that we have to remember to include.

Solution

useApi can just maintain one instance of axios and return that one non-changing instance.

@ctrlaltdylan ctrlaltdylan merged commit 0ff540f into ctrlaltdylan:master Sep 1, 2022
@ctrlaltdylan
Copy link
Owner

Thanks for the contribution @ko-lem !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants