1
votes

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]);
4

4 Answers

1
votes

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.

1
votes

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?

1
votes

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.

1
votes

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.