gbowne1
commented
8 months ago I feel like there might be some minor issues with that.
here's one thought I had after looking at that code:
useEffect(() => {
const source = axios.CancelToken.source();
const updatedConfig = { ...axiosConfig }; // Create a copy of axiosConfig
updatedConfig.cancelToken = source.token;
fetchData();
// Cancel the request if component unmounts
return () => source.cancel('Axios request canceled.');
}, [fetchData, axiosConfig]);Also. the spread operator (...config) is used directly without any checks or modifications. This could potentially lead to issues if the config object is not properly structured or if it contains properties that Axios does not recognize.
In the catch block, you're setting the error message directly from err.message. This assumes that the error object always has a message property, which might not always be the case, especially with network errors. It's safer to check if the message property exists before setting it.
From what I can tell you're correctly using Axios's cancel token to cancel the request if the component unmounts. However, this setup is a bit redundant since you're already using useEffect to handle the side effect of fetching data. The cancellation logic can be simplified by directly using the cancel token in the fetchData function.
The dependency array of the useEffect hook includes fetchData and axiosConfig. This is correct since both are used inside the effect. However, it's worth noting that fetchData is already wrapped in useCallback with axiosConfig as a dependency, so it's effectively stable. You might not need to include it in the dependency array of useEffect unless you're expecting fetchData to change for some reason.
I could be wrong here but it might be better as:
import { useState, useEffect, useCallback, useMemo } from 'react';
import axios from 'axios';
const useAxios = (config, initialState = { response: null, error: null, loading: true }) => {
const [response, setResponse] = useState(initialState.response);
const [error, setError] = useState(initialState.error);
const [loading, setLoading] = useState(initialState.loading);
const axiosConfig = useMemo(() => ({
...config,
// Add any additional configuration here
}), [config]);
const fetchData = useCallback(async () => {
setLoading(true);
const source = axios.CancelToken.source();
try {
const result = await axios({ ...axiosConfig, cancelToken: source.token });
setResponse(result.data);
setError(null);
} catch (err) {
setError(err.message || 'An error occurred');
setResponse(null);
} finally {
setLoading(false);
}
}, [axiosConfig]);
useEffect(() => {
if (!initialState.loading) {
fetchData();
}
}, [fetchData]);
return { response, error, loading, refetch: fetchData };
};
export default useAxios;Other considerations for this: using a loading state indicator
This is also another way this could be done...
import axios from 'axios';
import { useState, useEffect, useCallback, useMemo } from 'react';
const useAxios = (config, initialState = { response: null, error: null, loading: true }) => {
const [response, setResponse] = useState(initialState.response);
const [error, setError] = useState(initialState.error);
const [loading, setLoading] = useState(initialState.loading);
// Use useMemo to create a memoized config object, preventing unnecessary reruns
const axiosConfig = useMemo(() => ({
...config,
cancelToken: new axios.CancelToken.source().token, // Add cancel token for cleanup
}), [config]);
// useCallback for memoizing fetchData, preventing unnecessary refetches
const fetchData = useCallback(async () => {
setLoading(true);
try {
const result = await axios(axiosConfig);
setResponse(result.data);
setError(null);
} catch (err) {
setError(err.message);
setResponse(null);
} finally {
setLoading(false);
}
}, [axiosConfig]);
// useEffect to fetch data and clean up on unmount
useEffect(() => {
fetchData();
return () => axiosConfig.cancelToken.cancel('Axios request canceled.');
}, [fetchData, axiosConfig]);
return { response, error, loading, refetch: fetchData };
};
export default useAxios;Something to think about.. Let me know what you think @dev-george-nikolaidis @LOGESH-B @BlackBond06
BlackBond06
dev-george-nikolaidis
@gbowne1
Overview
This PR introduces a new reusable custom Axios hook,
useAxios, aimed at simplifying HTTP requests across the application. It strives for a balance between simplicity and functionality, making HTTP calls more straightforward and maintainable.Changes
useAxios.jsimplementing the custom Axios hook.Details
The
useAxioshook provides an easy-to-use interface for making HTTP requests while handling loading states, errors, and data responses efficiently. It is designed to be flexible for various use cases, supporting different HTTP methods and configurations.Example Usage
Goal
The goal of this PR is to enhance the codebase with a tool that abstracts away the complexities of making HTTP requests, thereby improving code readability and reducing redundancy.
Request for Feedback
I welcome any feedback on the balance of simplicity and functionality, as well as the clarity of the included examples.
Related to issue #218