CODEWITHSUNDEEP
reactjsreact-hooks
Pike
Pike

Reputation: 39

State Keeps Refreshing Hundreds of Times, Even With useEffect

So I am following Brad Traversy's React course, but it's a little old and I am using hooks and modern React for my own project, but I have a problem. When I enter the Detail page for a user, it calls a function to get the information of that user from an api. However, it turns out I am calling this function hundreds of times, even with useEffect. I will share my code, but I am not sure what I am doing wrong! If anyone can advise me on why this is re-rendering thousands of times, it would definitely be helpful. Thank you!

This is the function that is being called by Detail (setLoading is a useState function, as well as setSingleUser.

const getUser = username => {
    setLoading(true);

    axios
        .get(
            `https://api.github.com/users/${username}?client_id=
    ${BLAHBLAH}&client_secret=
    ${BLAHBLAH}`,
        )
        .then(res => {
            axios
                .get(
                    `https://api.github.com/users/${username}/repos?per_page=5&sort=created:asc&
        client_id=${BLAHBLAH}&client_secret=
        ${BLAHBLAH}`,
                )
                .then(repores =>
                    setSingleUser({ ...res.data, repos: repores.data }),
                );
        });
    setLoading(false);
};

This is where I call it in Detail

const Detail = ({ getUser, user, loading }) => {
const { username } = useParams();
useEffect(() => {
    getUser(username);
}, [username, getUser]);

I have also tried useEffect with an empty dependency array and no dependency array, but it does not like any of these options.

Upvotes: 4

Views: 1813

Answers (3)

Tom Bombadil
Tom Bombadil

Reputation: 3975

This is happening because of the second parameter to useEffect [username, getUser]. The array of dependencies for which the useEffect must be called is being called within your useEffect directly, which then triggers the useEffect again. So, The getUser is called again. Which then calls useEffect again. And therefore its stuck in a loop. You are pretty much calling a function (getUser) again inside of a function (useEffect) that you want to call each time (getUser) is called.

You need to add an if condition in the useEffect which makes sure the getUser() function gets called only when username is false or null or undefined, or you can also change the array that you are passing to the useEffect. You can even take advantage of the setSingleUser() that you have in the .then(). And can add the values of those in the if check or else in the array.

Upvotes: 2

Chirag Jain
Chirag Jain

Reputation: 221

Just include the username in the dependency array. Remove the function getUser since when the username is changed, the return is executed, again when getUser is called, username changes and again return is executed.

Try using this code:

useEffect(() => {
    getUser(username);
}, [username]);

Upvotes: 1

Shubham Khatri
Shubham Khatri

Reputation: 281686

As per your question, you seem to be defining getUser directly within the Parent component and passing it to child component.

Since you update state within the getUser function, the parent component will re-render and a new reference of getUser will be created. Now that you are passing getUser as a dependency to useEffect the useEffect runs again as getUser function has changed.

To solve this, you must memoize the getUser function in parent and you can do that using the useCallback hook

const getUser = useCallback(username => {
    setLoading(true);

    axios
        .get(
            `https://api.github.com/users/${username}?client_id=
    ${BLAHBLAH}&client_secret=
    ${BLAHBLAH}`,
        )
        .then(res => {
            axios
                .get(
                    `https://api.github.com/users/${username}/repos?per_page=5&sort=created:asc&
        client_id=${BLAHBLAH}&client_secret=
        ${BLAHBLAH}`,
                )
                .then(repores =>
                    setSingleUser({ ...res.data, repos: repores.data }),
                );
        });
    setLoading(false);
}, []);

Once you do that your code should work fine

Upvotes: 1

Related Questions