1

I'm trying to display modal when no products have been selected by user before. I ended up having an infinite loop of useEffect() dependency. I'm not sure how to do it correctly in React.

import React, { useState, useEffect, useCallback } from 'react';

const MyComponent = ({ products }) => {
    const [modals, setModals] = useState({});
    const [currentModalName, setCurrentModalName] = useState('');

    const setCurrentModal = useCallback(
        (modalName, data = {}) => {
            if (modalName) {
                setModals({
                    ...modals,
                    [modalName]: {
                        ...modals[modalName],
                        ...data
                    }
                });
            }

            setCurrentModalName(modalName);
        },
        [modals]
    );

    useEffect(
        () => {
            if (!products.length) {
                setCurrentModal('chooseProduct')
            }
        },
        [products, setCurrentModal] // setCurrentModal causes infinite loop
    );

    return (
        <div>...</div>
    );
}

export default MyComponent;

I can just remove setCurrentModal from the dependencies, but I'm warned about it. If I add it, my React app freezes.

How can I organize my code to avoid freezing?

Robo Robok
  • 21,132
  • 17
  • 68
  • 126
  • Related: [How to fix missing dependency warning when using useEffect React Hook?](https://stackoverflow.com/a/55854902/11838196) – srk Dec 16 '20 at 18:31

2 Answers2

2

Why it loops?

The callback is always changing since it depends on the modals, which is always a different object even though it has the exact same properties as before, which always triggers the useEffect since it depends on the setCurrentModal callback value, which is always different since (() => {}) !== (() => {}).

Solution

Always use the functional update when the current state is needed to set the next state.

It'll prevent the need for the modals state as a dependency, which will limit the times when the callback is updated, fixing the infinite loop at the same time.

In addition to solving today's problem, functional update of the state is less prone to race-conditions, where multiple updates batched by React would overwrite each others.

const setCurrentModal = useCallback(
  (modalName, data = {}) => {
    if (!modalName) return; // precondition fail? early return.

    // Happy path here!

    // Note that I've used a different name to highlight that 
    // it's a different variable and to avoid shadowing the 
    // `modals` var from the outer scope.
    setModals((currentModals) => ({ // use functional update. 
      ...currentModals,
      [modalName]: {
        ...currentModals[modalName],
        ...data
      }
    }));

    setCurrentModalName(modalName);
  }, 
  // remove `modals` from the dependencies.
  // setter functions are stable anyway, so it should remove any warning.
  [setModals, setCurrentModalName]
);

useEffect(() => {
    if (!products.length) {
      setCurrentModal('chooseProduct')
    }
  },
  [products, setCurrentModal] 
);

Since the setCurrentModal callback is now stable (never ever changing), the useEffect will only be called when products value changes.

Missing dependencies warnings

The missing dependencies warnings come from the eslint-plugin-react-hooks, specifically, the react-hooks/exhaustive-deps rule. It's totally optional, but it helps keep the code clean and safe.

You could also choose to disable the warning just for this line:

const setCurrentModal = useCallback(
  (modalName, data = {}) => {
    // ...
    setModals(/* ... */);
    setCurrentModalName(modalName);
  }, 
  [] // eslint-disable-line react-hooks/exhaustive-deps
);
Emile Bergeron
  • 17,074
  • 5
  • 83
  • 129
  • The thing is, I'm still warned about missing `setCurrentModal` dependency in this case. I think it's because it's inside the functional component, so even though it's `const`, it will still change on the next cycle. I could move `setCurrentModal ` outside the component, except is uses the state. – Robo Robok Dec 16 '20 at 19:02
  • @RoboRobok I've updated my answer with all relevant dependencies which should remove any warnings and fix the loop problem as well. – Emile Bergeron Dec 16 '20 at 19:17
  • It says that `modals` is a required dependency too, unfortunately. – Robo Robok Dec 16 '20 at 20:05
  • @RoboRobok it means that `modals` is still used inside your callback or effect and that you didn't implement correctly what's in my answer (which removes completely the `modals` state from the callback). – Emile Bergeron Dec 16 '20 at 20:07
  • @RoboRobok, noticed the function in `setModals`? – Emile Bergeron Dec 16 '20 at 20:08
  • Sorry, I didn't notice it at first. I had no idea I can use a state setter as a function! So as a rule of thumb, I guess it's always better to do it that way whenever I need to use the previous value to compute the new one - am I correct? – Robo Robok Dec 16 '20 at 20:12
  • @RoboRobok yes, that's exactly the point of the solution I'm exposing above! I've linked to the documentation for more information on how and why functional update matters. – Emile Bergeron Dec 16 '20 at 20:14
  • 1
    Awesomeness, thank you! Sorry I ignored some parts of your answer at first, I felt overwhelmed. Thank you for a great answer. – Robo Robok Dec 16 '20 at 20:18
0

I think you can simplify it, without using useCallback.

(tested with Next.js and had no warnings, but if you still have some, you should use the answer of @Emile Bergeron)

import React, { useState, useEffect } from 'react'

const MyComponent = ({ products }) => {
  const [modals, setModals] = useState({})
  const [currentModalName, setCurrentModalName] = useState('')

  const setCurrentModal = (name, data) => {
    if (name) {
      setModals(prev => {
        return { ...prev, [name]: { ...prev[name], ...data }}
      })
      setCurrentModalName(name)
    }
  }

  useEffect(() => {
    if (!products || !products.length) {
      const modalName = 'chooseProduct'
      const data = { data: 'data' }

      setCurrentModal(modalName, data)
    }
  }, [products])

  const modalsJsx = modals ? Object.keys(modals).map((x, i) => {
    return <li key={`modal-${i}`}>{x}</li>
  }) : ''

  const addModal = () => {
    const name = 'test' + Math.floor(Math.random() * Math.floor(300))
    setCurrentModal(name, { data: 'Hey' })
  }

  return (
    <div>
      <p>Current Modal : {currentModalName}</p>
      <p>Modals : </p>
      <ul>
        {modalsJsx}
      </ul>

      <button onClick={addModal}>Test</button>
    </div>
  )
}

export default MyComponent

The function with useCallback to avoid warnings :

  const setCurrentModal = useCallback((name, data = {}) => {
    if (name) {
      setModals(prev => {
        return { ...prev, [name]: { ...prev[name], ...data }}
      })
      setCurrentModalName(name)
    }
  }, [setModals, setCurrentModalName])
Erwan
  • 102
  • 6