Search code examples
reactjstypescriptreact-hooks

setInterval in useEffect triggers twice - React


I am trying to solve a cool problem in React from this video.

In short, we need have 5 checkout lines in a shop, and input and a button. The input is a number only, and when we click "add" then we basically add a person with x number of groceries (x = the number from the input field) to the checkout line with the fewest amount of total groceries it needs to process, basically a queue but we need to find the checkout line with the least amount of total groceries.

Each second the first person in each checkout line is decreased by 1 until it reaches zero and then we process the next person in line.

I tried to solved it in react and split it into 3 components:

  • Shop: holds "k" checkout lines in a useState and a checkoutlineToShoppers which is map/object that maps each checkout line index to its shoppers.
  • Shopper: holds amount of groceries and nothing more.
  • CheckoutLine: holds an index and a list of numbers (the first number is the first shopper's amout, the second number is the second's etc.)

I won't go into details here but the main logic happens in the Shop component, here it is fully but I'll copy some snippets for context here:

The states:

const [numberOfCheckoutLines, setNumberOfCheckoutLines] = useState<number>(5);
const [checkoutlineToShoppers, setCheckoutlineToShoppers] = useState<CheckoutlineData>(initializeDictionary(numberOfCheckoutLines));

The function that initializes the dictionary that maps between the index of a checkout line to an empty array of numbers

function findShopWithMinimumShoppers(checkoutlines: CheckoutlineData) {
    let minShopId: number = 0;
    let minSum = Infinity;
    console.log(checkoutlines)
    for (const shopId in checkoutlines) {
        const shoppersSum = checkoutlines[shopId].shoppers.reduce((acc, curr) => acc + curr, 0);
    
        if (shoppersSum < minSum) {
        minSum = shoppersSum;
        minShopId = Number(shopId);
        }
    }
    
    return minShopId;
    }

The logic that finds the checkout line with the least amount of total groceries.

The problematic useEffect:

useEffect(() => {
        const intervalId = setInterval(() => {
          setCheckoutlineToShoppers(prevCheckoutlines => {
            const updatedCheckoutlines = { ...prevCheckoutlines };
    
            for (const shopId in updatedCheckoutlines) {
              if (updatedCheckoutlines[shopId].shoppers.length > 0) {
                // Decrement the first shopper by 1
                const firstShopper = updatedCheckoutlines[shopId].shoppers[0];
    
                // Check if the first shopper should be removed
                if (firstShopper <= 0) {
                  // Remove the first shopper
                  updatedCheckoutlines[shopId].shoppers.shift();
                } else {
                  // Update the first shopper's value
                  updatedCheckoutlines[shopId].shoppers[0] = firstShopper - 1;
                }
              }
            }
    
            return updatedCheckoutlines;
          });
        }, 1000); // Every second
    
        return () => clearInterval(intervalId); // Clean up the interval when component unmounts
      }, []);

Each second I iterate over each checkout line, find its first element and set it to its value -1, but unfortunately the problem is that it decreases each of the first person in each checkout line by 2 and not 1... I entered 5 for all and after 1 second they went down to 3 and after 2 seconds they all went down to 1.

enter image description here

What is the problem here? Why does it happen twice? I don't even have a clue on where to start with the problem because all I did is when the component mounts it just uses the useEffect hook to start a setInterval of one second, and then clears it.


Solution

  • You have an accidental state mutation in your interval callback logic. The double-calling of the React.useEffect hook callback in React.StrictMode is surfacing this logical bug for you to see.

    useEffect(() => {
      const intervalId = setInterval(() => {
        setCheckoutlineToShoppers(prevCheckoutlines => {
          const updatedCheckoutlines = { ...prevCheckoutlines };
    
          for (const shopId in updatedCheckoutlines) {
            if (updatedCheckoutlines[shopId].shoppers.length > 0) {
              // Decrement the first shopper by 1
              const firstShopper = updatedCheckoutlines[shopId].shoppers[0];
    
              // Check if the first shopper should be removed
              if (firstShopper <= 0) {
                // Remove the first shopper
                updatedCheckoutlines[shopId].shoppers.shift(); // <-- mutation of updatedCheckoutlines[shopId].shoppers array
              } else {
                // Update the first shopper's value
                updatedCheckoutlines[shopId].shoppers[0] = firstShopper - 1; // <-- mutation of updatedCheckoutlines[shopId] object
              }
            }
          }
    
          return updatedCheckoutlines;
        });
      }, 1000);
    
      return () => clearInterval(intervalId);
    }, []);
    

    The shallow copy of prevCheckoutlines into updatedCheckoutlines only copies the properties by reference into a new outer object reference, but all the properties still point to the current state.

    All React state, and nested state, necessarily needs to be shallow copied then the parts that are being updated should be overwritten.

    Example:

    useEffect(() => {
      const intervalId = setInterval(() => {
        setCheckoutlineToShoppers(prevCheckoutlines => {
          const updatedCheckoutlines = { ...prevCheckoutlines };
    
          for (const shopId in updatedCheckoutlines) {
            if (updatedCheckoutlines[shopId].shoppers.length > 0) {
              const firstShopper = updatedCheckoutlines[shopId].shoppers[0];
    
              // Check if the first shopper should be removed
              if (firstShopper <= 0) {
                // Remove the first shopper
                updatedCheckoutlines[shopId] = {
                  // Shallow copy shop into new object reference
                  ...updatedCheckoutlines[shopId],
    
                  // Use slice instead of shift to create new array reference
                  // and keep all but last element
                  shoppers: updatedCheckoutlines[shopId].shoppers.slice(0, -1),
                };
              } else {
                // Update the first shopper's value
                updatedCheckoutlines[shopId] = {
                  // Shallow copy shop into new object reference
                  ...updatedCheckoutlines[shopId],
    
                  // Use map to create new array reference
                  shoppers: updatedCheckoutlines[shopId].shoppers
                    .map((shopper, index) => index === 0 ? shopper - 1 : shopper
                  ),
                };
              }
            }
          }
    
          return updatedCheckoutlines;
        });
      }, 1000);
    
      return () => clearInterval(intervalId);
    }, []);