useEffect infinite loop WITH empty dependency array

Question

I am dispatching a thunk from my useEffect to populate a table when the page loads. The useEffect keeps running infintely, even though I am using an empty dependency array. Here is my code:

const Cart = (props) => {
  const [didFetch, setDidFetch] = useState(false);
  useEffect( () => {
    async function fetchCart(){
      await props.getCartItems();
      setDidFetch(true);
    }
    fetchCart()
    return setDidFetch(false)
  },[]);

  return (
    <TableContainer component={Paper}>
      <Table sx={{ minWidth: 650 }} aria-label="simple table">
        <TableHead>
          <TableRow>
            <TableCell>Product</TableCell>
            <TableCell align="right">Quantity</TableCell>
            <TableCell align="right">Total Cost</TableCell>
          </TableRow>
        </TableHead>
        <TableBody>
          {props.cartItems.map((item) => (
            <TableRow
              key={item.product.name}
              sx={{ "&:last-child td, &:last-child th": { border: 0 } }}
            >
              <TableCell component="th" scope="row">
                {item.product.name}
              </TableCell>
              <TableCell align="right">{item.quantity}</TableCell>
              <TableCell align="right">
                {(item.quantity * item.product.price).toFixed(2)}
              </TableCell>
              <TableCell>
                <Button onClick={() => props.removeCartItem(item.id)}>
                  Remove
                </Button>
              </TableCell>
            </TableRow>
          ))}
        </TableBody>
      </Table>
    </TableContainer>
  );
};

You are returning a function call in your effect. You should return a function which then calls your setDidFetch. — Dimitar
– Dimitar, Commented Sep 23, 2021 at 12:50

Sinan Yaman · Accepted Answer · 2021-09-23 12:49:02Z

2

The cleanup function of useEffect should be an anonymous function, and you shouldn't set state variables inside it, as @windmaomao pointed out, it is a memory leak:

Suggestion

If you want to store a didFetch flag, I would use a ref for this purpose:

import {useRef} from 'react'

const Cart = (props) => {
  const fetched = useRef()
  useEffect( () => {
    async function fetchCart(){
      const res = await props.getCartItems();
      if(res) fetched.current = true
    }
    fetchCart()
    return () => fetched.current = false
  },[]);

  return (
    <TableContainer component={Paper}>
      <Table sx={{ minWidth: 650 }} aria-label="simple table">
        <TableHead>
          <TableRow>
            <TableCell>Product</TableCell>
            <TableCell align="right">Quantity</TableCell>
            <TableCell align="right">Total Cost</TableCell>
          </TableRow>
        </TableHead>
        <TableBody>
          {props.cartItems.map((item) => (
            <TableRow
              key={item.product.name}
              sx={{ "&:last-child td, &:last-child th": { border: 0 } }}
            >
              <TableCell component="th" scope="row">
                {item.product.name}
              </TableCell>
              <TableCell align="right">{item.quantity}</TableCell>
              <TableCell align="right">
                {(item.quantity * item.product.price).toFixed(2)}
              </TableCell>
              <TableCell>
                <Button onClick={() => props.removeCartItem(item.id)}>
                  Remove
                </Button>
              </TableCell>
            </TableRow>
          ))}
        </TableBody>
      </Table>
    </TableContainer>
  );
};

edited Sep 23, 2021 at 12:49

answered Sep 23, 2021 at 12:45

Sinan Yaman

5,9562 gold badges21 silver badges40 bronze badges

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

6 Comments

windmaomao Over a year ago

you can't have a setDidFetch in the destroy of effect, it's a memory leak, just remove it. You don't need to set any state upon destroy.

Sinan Yaman Over a year ago

@windmaomao just noticed that, editing the answer. Thanks for pointing out

windmaomao Over a year ago

not a problem, your solution looks good now. Although there's no usage of the current in his code.

Sinan Yaman Over a year ago

OP doesn't use the didFetch in the question, I noticed that too. However, I figured OP would want to use it somewhere :)

charmingduchess Over a year ago

I was reading somewhere that there can be weird bugs if you don't return a cleanup function to undo any changes you made. However, I don't really believe its necessary here, so I'll just take it out. Just to clarify, was that return statement creating the infinite loop? Also @windmaomao thats her code ;) Thanks everybody!

|

Nicholas Bergesen · Accepted Answer · 2021-09-23 13:04:15Z

1

Using your existing code it would be either:

Anonymous function

  const [didFetch, setDidFetch] = useState(false);
  useEffect( () => {
    async function fetchCart(){
      await props.getCartItems();
      setDidFetch(true);
    }
    fetchCart()
    return () => { 
        setDidFetch(false); 
    };
  },[]);

Named Function

  const [didFetch, setDidFetch] = useState(false);
  useEffect( () => {
    async function fetchCart(){
      await props.getCartItems();
      setDidFetch(true);
    }
    fetchCart()
    return function cleanup() { 
        setDidFetch(false); 
    };
  },[]);

Here are the useEffect docs for reference: https://reactjs.org/docs/hooks-effect.html

answered Sep 23, 2021 at 13:04

Nicholas Bergesen

4023 silver badges14 bronze badges

Collectives™ on Stack Overflow

useEffect infinite loop WITH empty dependency array

2 Answers 2

Suggestion

6 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Suggestion

6 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related