How to Avoid a useEffect and useCallback Dependency Loop in React.js

Question

I am working on converting a React component from a class component to a functional component with hooks.

I want to get a list of group members from my database. I hold the array in component state.

const [members, setMembers] = useState([]);

Once the members are downloaded, I want to get each member's profile picture asynchronously.

1) The component is mounted, and the following useEffect() is called. Note the dependency to getMembers.

useEffect(() => {
    getMembers();
}, [getMembers]);

2) The useEffect callback calls the function getMembers(). Note the dependency to getMembersProfilePictures.

const getMembers = useCallback(() => {
    fetchMembersFromDatabase()
        .then((data) => {
            setMembers(data);

            getMembersProfilePictures();
        })
}, [getMembersProfilePictures]);

3) Once the members are retrieved from the database, the members state is updated and getMembersProfilePictures() is called. Note the dependency to members.

const getMembersProfilePictures = useCallback(() => {
    for (let i = 0; i < members.length; i++) {
        const member = { ...members[i] };

        if (member.has_picture) {
            firebase
                .storage()
                .ref()
                .child("<childUrl>")
                .getDownloadURL()
                .then((url) => {
                    member.picture_url = url;
                    const membersCopy = [...members];
                    membersCopy[i] = member;
                    setMembers(membersCopy);
                });
        }
    }
}, [members]);

Because the useEffect() depends on getMembers(), getMembers() depends on getMembersProfilePictures(), and getMembersProfilePictures() depends on members, as soon as the members state is updated, the chain of useCallback()s is re-created, and the useEffect() is called. This becomes an infinite loop of data fetching.

My current thought is to pass the data retrieved from fetchMembersFromDatabase() directly to getMembersProfilePictures() as an argument. This removes the dependency of members from getMembersProfilePictures(), and therefore removes the infinite loop.

Ignoring listening to changes in the members list in the database and ignoring caching of members and their respective profile picture, it appears that there are no drawbacks to this solution. I am wondering what other's thoughts are to this solution. Thanks!

getMembers and getMembersProfilePictures are part of the same callback, you've separated them unnecessarily and created a dependency loop. — Adam Jenkins
– Adam Jenkins, Commented May 15, 2020 at 1:02
@Adam What do you mean by separated? Should both functions be combined into one function? If that is the case, I believe that I would still have a dependency loop. — Dan Levy
– Dan Levy, Commented May 15, 2020 at 1:29
No there wouldn't be because getMembersProfilePictures would no longer rely on members because members comes directly from the fetchMembersFromDatabase call. — Adam Jenkins
– Adam Jenkins, Commented May 15, 2020 at 1:34
I've got a better solution for you - create a "Member" component and let each Member component be responsible for downloading it's own picture, your "top" component should only be concerned with getting the list of members. — Adam Jenkins
– Adam Jenkins, Commented May 15, 2020 at 1:45
99% of the devs try to create mini applications out of their components instead of just creating teeny-tiny, almost silly tiny components. My favourite types of components are so small they are laughable, like OptionalThemeComponent = ({theme,children}) => theme ? <ThemeProvider theme={theme}>{children}</ThemeProvider> : children This is what component based development should look like - components so small that you understand everything they do in under 1 minute. — Adam Jenkins
– Adam Jenkins, Commented May 15, 2020 at 1:53

Community · Accepted Answer · 2020-06-20 09:12:55Z

2

You are calling fetchMembersFromDatabase() in the useCallback but not using it as a dependency.
You are setting members inside getMembersProfilePictures() which is using members as dependency. This is causing infinite loop in my opinion.

Proposed solution

useEffect without any dependency
useEffect for member and call the get member images function. maintain another object for member thumbnails.

edited Jun 20, 2020 at 9:12

CommunityBot

11 silver badge

answered May 15, 2020 at 1:01

Ehsan Mahmud

7434 silver badges11 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Dan Levy Over a year ago

I agree that setting members inside of getMembersProfilePictures() is what is causing the infinite loop. The fetchMembersFromDatabase() function declaration is outside of this component function, and therefore not a dependency. Clever solution - thanks!

Adam Jenkins · Accepted Answer · 2020-05-15 01:55:50Z

1

EDIT: It's become clear you're trying to do too much in one component.

What you really want to do is

Get your list of members and render Member components and then
Have your Member components download their own pictures.

The below is semi-pseudo code, take the async/await stuff with a grain of salt and do it the proper way that works for you:

const MemberList = ({maybeWithAProp}) => {

   const [members,setMembers] = useState([]);
   useEffect(async () => {
       setMembers((await someCallToGetMembers(maybeWithAProp));
   },[maybeWithAProp]);

   return members.map(m => <Member {...m}/>

}

const Member = ({memberId}) => {

  const [pic,setPic] = useState();

  useEffect(async () => {
     setPic(null); // you're about to download a new one, probably get rid of the old one first
     setPic((await fetchPicForMember(memberId));
  },[memberId]);

  return (...)

}

Original answer:

Ever consider separating your state?

const [members, setMembers] = useState([]);
const [membersPics,setMembersPics] = useState([]);

// run on mount only
useEffect(async () => {
  const data = await fetchMembersFromDatabase();
  setMembers(data);
,[]);

useEffect(async () => {
  const pics = await Promise.all(members.map(m => fetchProfilePic(m)));
  setMembersPics(pics);
},[members]);


return <SomeComponent members={useMemo(members.map((m,i) => ({...m,url:membersPics[i]})),[members,memberPics])}/>

The major issue with your original code is that you were calling setMembers twice, which is a big indicator that what you think is one piece of state, is probably better served as being two pieces of state.

edited May 15, 2020 at 1:55

answered May 15, 2020 at 1:38

Adam Jenkins

56.3k13 gold badges85 silver badges113 bronze badges

2 Comments

Dan Levy Over a year ago

My only concern with this approach is that I must create a mapping/pointer to map a picture to a member. Not all members will have a picture, so I would either have to hold the member's UID in the member state and the memberPic state or I would have to leave empty indices in the memberPics array in order to map by array index. This is not a memory problem with 30 members, but it would be a waste of memory or duplication of data when dealing with, say, 500 + members. Is there a more efficient way to map a picture to a member? There is always the time/space/readability tradeoff with any code...

Dan Levy Over a year ago

I like the edit! It separates responsibilities, removes the re-rendering of all members when just one picture comes in, and removes the infinite loop. Thanks so much!

Collectives™ on Stack Overflow

How to Avoid a useEffect and useCallback Dependency Loop in React.js

2 Answers 2

Proposed solution

1 Comment

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Proposed solution

1 Comment

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related