1

I have a map which has an overlay. I have an effect which, when the underlying data changes, deletes the old overlay and re-draws a new one.

Of note, I'm using "react-hooks/exhaustive-deps" and it seems from everything I read that just removing the dependency overlay is not the right answer (but it does work).

Of note, data is a prop passed in.

useEffect(() => {
    // remove the overlay if it is there,
    // I should have overlay as a dependency per exhaustive-reps react eslint rule
    if (overlay) map.removeOverlays(overlay);    

    // Generate the new overlay based on the new data (useCallback function)
    const newOverlay = generateNewOverlay(data)

    // Store the new overlay so I can remove it later if the data changes
    // Doesn't need to be done right away though, just before next render
    setOverlay(newOverlay);

    // Show the new overlay on my map
    map.addOverlays(newOverlay);
  }, [map, overlay, data, generateNewOverlay]);

This of course will be an infinite loop because I'm setting overlay and making it a dependency. I also don't like using setState immediately in an effect as it causes another render. What am I missing? How do I achieve this logic?

I did read about 5 similarly titled questions, but they did not answer my question.

Similar question, but not asking while keep exhaustive-reps deps lint rule which isn't a factor for them because they aren't reading the state before changing it.


Edit:

I still have the problem where useState and a reducer should be pure. The purpose of the dependency I'm having an issue with (overlay) is that I have to check if overlay exists. If it does, I need to do a non-pure thing which is remove that overlay from the map before setting my new overlay. This line of code is what makes it not pure:

if (overlay) map.removeOverlays(overlay); 

Without that line, I'd never need overlay in my dependency list and this entire thing wouldn't be a factor. As you can see the accepted answer is not pure because of that line.

The useReducer answer has this line outside the reducer so overlay should be a dependency or it needs to go in the reducer. The first option is what I'm trying to avoid by the "react-hooks/exhaustive-deps" and the second answer is not pure.

Appreciate any help.


Final Edit:

I found the best way to approach this was to use the cleanup in useEffect properly. I was requiring state so that I could remove things from a map later.

useEffect(() => {
    // remove the overlay if it is there,
    // I originally needed this to store the overlay so I could remove it later, but if I use cleanup properly this isn't needed
    // if (overlay) map.removeOverlays(overlay);    

    // Generate the new overlay based on the new data (useCallback function)
    const newOverlay = generateNewOverlay(data)

    // Store the new overlay so I can remove it later if the data changes
    // Doesn't need to be done right away though, just before next render (This is what a cleanup is!)
    // setOverlay(newOverlay);

    // Show the new overlay on my map
    map.addOverlays(newOverlay);

    // New code below
    return () => {map.removeOverlays(newOverlay)};
  }, [map, data, generateNewOverlay]);
Diesel
  • 5,099
  • 7
  • 43
  • 81

5 Answers5

1

You can use a function inside of setOverlay:

useEffect(() => {
  setOverlay(prev => {
    // remove the overlay if it is there,
    // I should have overlay as a dependency per exhaustive-reps react eslint rule
    if (prev) map.removeOverlays(prev);    

    // Generate the new overlay based on the new data (useCallback function)
    const newOverlay = generateNewOverlay(data)

    // Show the new overlay on my map
    map.addOverlays(newOverlay);
    return newOverlay;
  });

}, [map, data, generateNewOverlay]);

In the prev parameter you get the current state of overlay. This way you don't have to add the overlay state variable to the dependency array. Hence, the effect won't trigger when overlay updates.

Damien Flury
  • 769
  • 10
  • 23
1

I suggest to simply remove it and add the ignore rule with comment. This is what I have my team do and is what is suggested here by Gaearon for specific use-cases.

Note: I do like @DamienFlury's solution, if it works (and I suspect it does).

useEffect(() => {
  // remove the overlay if it is there,
  if (overlay) map.removeOverlays(overlay);    

  // Generate the new overlay based on the new data (useCallback function)
  const newOverlay = generateNewOverlay(data)

  // Store the new overlay so I can remove it later if the data changes
  // Doesn't need to be done right away though, just before next render
  setOverlay(newOverlay);

  // Show the new overlay on my map
  map.addOverlays(newOverlay);
  // value of overlay changing should not trigger effect
  // eslint-disable-next-line react-hooks/exhaustive-deps
}, [map, data, generateNewOverlay]);

This will stifle the warning, but will also mask any future dependency changes should the API of the effect change, so you're basically on your own to ensure the effect still triggers when you expect it to.

Does overlay ever change outside this effect? I.E. does your component ever call setOverlay outside the effect?

Drew Reese
  • 165,259
  • 14
  • 153
  • 181
  • It does not change outside of this effect. I'm currently doing what you said, but I may shift to one of the other answers. Thanks! – Diesel Feb 10 '20 at 16:49
1

To add onto @DamienFlury's solution, Another technique you can use to remove a constantly changing dependency from the dependency array is to use the useReducer hook. This allows you to decouple your state variables from your effects

function reducer(state, action){
  if(action.type === 'UPDATE_OVERLAY'){
    return {
     ...state,
     overlay: action.newOverlay,  //update overlay on your state object
    }
  } else {
    return state;
  }
}

function YourComponent(yourProps){
  const [ state, dispatch ] = useReducer(reducer, initialState);

  // Other functions

  useEffect(() => {
    // remove the overlay if it is there,
    if (overlay) map.removeOverlays(overlay);    

    // Generate the new overlay based on the new data (useCallback function)
    const newOverlay = generateNewOverlay(data)

    // this also allows you to decouple your state changes from your effects!
    dispatch({ 
      type: 'UPDATE_OVERLAY', 
      newOverlay,
    });

    // Show the new overlay on my map
    map.addOverlays(newOverlay);

  }, [dispatch, map, data, generateNewOverlay]);

}

Note that I have added dispatch to the dependency array just as a good practice, You should never lie about your dependencies!. React guarantees the dispatch function to be constant throughout the component lifetime.

JLRishe
  • 99,490
  • 19
  • 131
  • 169
Chitova263
  • 719
  • 4
  • 13
  • Ah I see this works because the old value is referenced in the reducer (much like the other answer where it's referenced in the function) and so it isn't required as a dependency, right? – Diesel Feb 10 '20 at 16:50
  • It would appear this doesn't fix the dependency problem. You're still using `overlay` within the `useEffect` callback. – JLRishe Feb 21 '20 at 02:07
1

I think the issue here is that your approach to accommodating this linter rule is too passive. You are running all the code in the useEffect callback any time it runs, but you really only want to execute that stuff when the data changes.

So you can reflect that in your code:

const [shouldUpdateOverlay, setShouldUpdateOverlay] = useState(false);

useEffect(() => {
    if (shouldUpdateOverlay) {
        // remove the overlay if it is there,
        // I should have overlay as a dependency per exhaustive-reps react eslint rule
        if (overlay) map.removeOverlays(overlay);    

        // Generate the new overlay based on the new data (useCallback function)
        const newOverlay = generateNewOverlay(data);

        // Show the new overlay on my map
        map.addOverlays(newOverlay);

        // Store the new overlay so I can remove it later if the data changes
        // Doesn't need to be done right away though, just before next render
        setOverlay(newOverlay);
        setShouldUpdateOverlay(false);
    }
}, [map, overlay, shouldUpdateOverlay data, generateNewOverlay]);

// use from event handlers, etc.
const updateData = (newData) => {
    setData(newData);
    setShouldUpdateOverlay(true);
};

Realistically, this is an overwrought approach to accomplishing the same thing you could achieve by having [data] as the only dependency in useEffect, since that's really the only dependency that matters there. However, I believe the above should allow you to achieve what you are trying to do without violating the linter rule.

JLRishe
  • 99,490
  • 19
  • 131
  • 169
0

If you want to keep the old overlay value in a state just to remove it later, you don't need to use a state. It can be saved on a ref, then it won't trigger the re-render when changed, avoiding the infinite loop.

The refs can be used for this kind of purpose (when you just want to save something without triggering a re-render): https://reactjs.org/docs/hooks-faq.html#is-there-something-like-instance-variables