1
votes

I'm not quite understand how useEffect cleanup function work. Because whatever I do I always get warning:

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.

Here is my code:

useEffect(() => {
        setLoading(true) 

        // Get position list
       const getPositionList = db.collection('lists').doc('positions').get()
            .then( res => {
                let data = JSON.stringify(res.data())
                data = JSON.parse(data)
                setPositionsList(data.list)
                setLoading(false)
            })
        return () => getPositionList
    }, [])
3
You changing state on an unmounted component which doesn't make sense and you get a warning for it.Dennis Vash
Could you explain more?Romanas

3 Answers

2
votes

useEffect expects as a return value an unsubscribe/cleanup function, or a null. Your .get() is NOT a subscription, so there's nothing to clean up

BUT useEffect is NOT asynchronous, and the .get() definitively returns a promise that needs a delay, even with the .then(). Your dependency array is empty, so useEffect is only called once - but I suspect your component unmounted before the .get() ever returned.

Your code will need to be closer to:

useEffect(() => {
        setLoading(true) 
       
        (async () => {
        // Get position list
           await db.collection('lists').doc('positions').get()
            .then( res => {
                let data = JSON.stringify(res.data())
                data = JSON.parse(data)
                setPositionsList(data.list)
                setLoading(false)
            });
        )();
        return null
    }, [])

the ( async () => {})() creates an anonymous asynchronous function, then executes it. You do want to try to structure your system such that the enclosing component does not unmount before loading is set to false - we don't see that here, so we have to assume you do that part right.

The important take-aways:

  • useEffect is NOT asynchronous
  • .get() IS asynchronous
  • ( async () => {}) creates an anonymous async function, and ( async () => {})() creates a self-executing anonymous asynchronous function
1
votes
useEffect(() => {
        setLoading(true) 

       // below firebase api return promise. hence no need to unsubscribe.
       db.collection('lists').doc('positions').get()
            .then( res => {
                let data = JSON.stringify(res.data())
                data = JSON.parse(data)
                setPositionsList(data.list)
                setLoading(false)
            })
        return () => {
          // any clean up code, unsubscription goes here. unmounting phase
        }
    }, [])
0
votes

According @Dennis Vash answer I figure out, that before setting state I have to check is component mounted or not, so I add variable in useEffect and before set state I check I add if statement:

useEffect(() => {
        setLoading(true) 
        let mounted = false // <- add variable
        // Get position list
        db.collection('lists').doc('positions').get()
            .then( res => {
                let data = JSON.stringify(res.data())
                data = JSON.parse(data)
                if(!mounted){ // <- check is it false
                    setPositionsList(data.list)
                    setLoading(false)
                }
            })
        return () => {
            mounted = true // <- change to true 
        }
    }, [])