ReactJS executes both if and else statement

Question

I'm trying to build this little project with React Leaflet in which by searching through the leaflet-geosearch search bar a certain city, after clicking on it, a form should pop up asking for some reviews about the place. I want to add a prevent duplicate clause in the handler of the geosearch bar using the array of already rated places that I'm passing to the SearchBar component as a prop. When i try to verify if the cities arr already has an object with the same id, it just doesn't early return, it ignores the early return completely.

CODE///

export default function SearchBar({
  onSubmitCityMapSearch,
  setCurrLocation,
  onSetFocus,
  cities,
}) {
  function handler(event) {
    const {
      location: {
        label,
        y,
        x,
        raw: { place_id },
      },
    } = event;

    
    const exists = cities.every((city) => city.location.id !== place_id);
    if (!exists) {
      console.log("It exists"); /*used to be an early return which wouldn't get executed. In this case the cl does get executed, same as the else block*/
    } else {
      onSubmitCityMapSearch();
      setCurrLocation({ label, location: [y, x], id: place_id });
      onSetFocus(place_id);
    }
  }

  const provider = new OpenStreetMapProvider();

  const searchControl = new GeoSearchControl({
    provider: provider,
    style: "bar",
    showMarker: false,
  });

  const map = useMap();
  useEffect(() => {
    map.addControl(searchControl);
    return () => map.removeControl(searchControl);
  }, []);

  map.on("geosearch/showlocation", handler);
  return null;
}

I tried with an if/else statement and in that case it executes both if and else statements. I tried doing some research on it, but to no avail. Anyone has any deeper understanding why this could be happening? The code is a little bit messy, I'm really new to React.

Answer 1

You're adding your handler every time your component is rendered, so you end up with multiple versions of your handler function attached, which close over different copies of cities. So when the event occurs, your handler is called multiple times, and reports the results that are valid for its copy of cities. If you're seeing both the if branch taken and the else branch taken, it's because one of the handlers is taking the if branch and another handler is taking the else branch.

You have at least two options:

1. Hook up your handler in a useEffect callback with cities as a dependency, and unhook it in the effect's cleanup (you should be doing that in any case), like this:

   useEffect(() => {
       function handler(event) {
           const {
               location: {
                   label,
                   y,
                   x,
                   raw: { place_id },
               },
           } = event;
   
           const exists = cities.every(
               (city) => city.location.id !== place_id
           );
           if (!exists) {
               console.log(
                   "It exists"
               ); /*used to be an early return which wouldn't get executed. In this case the cl does get executed, same as the else block*/
           } else {
               onSubmitCityMapSearch();
               setCurrLocation({ label, location: [y, x], id: place_id });
               onSetFocus(place_id);
           }
       }
       map.on("geosearch/showlocation", handler);
       return () => {
           // This function is called when cleaning up
           map.off("geosearch/showlocation", handler);
           //  ^^^−−−− or whatever the "unregister" function is
       }
   }, [cities]);
   

   Note that this means your handler will be removed and the the new one attached every time cities changes. In most cases, that's harmless.

2. If for some reason you want to only attach the handler once, you need a different way of accessing cities so that you're always accessing an up-to-date version of it. A common way to do that is to use a ref. You'd do that by declaring the ref and assigning cities to it in top-level code inside your component function, then hook up your event handler inside your useEffect(___, []) callback, which only runs on mount, and have the code use citiesRef.current instead of cities:

   const citiesRef = useRef(null);
   citiesRef.current = cities;
   
   useEffect(() => {
       function handler(event) {
           const {
               location: {
                   label,
                   y,
                   x,
                   raw: { place_id },
               },
           } = event;
   
           const cities = citiesRef.current; // <================================
           const exists = cities.every(
               (city) => city.location.id !== place_id
           );
           if (!exists) {
               console.log(
                   "It exists"
               ); /*used to be an early return which wouldn't get executed. In this case the cl does get executed, same as the else block*/
           } else {
               onSubmitCityMapSearch();
               setCurrLocation({ label, location: [y, x], id: place_id });
               onSetFocus(place_id);
           }
       }
   
       map.addControl(searchControl);
       map.on("geosearch/showlocation", handler);
       return () => {
           map.removeControl(searchControl);
           map.off("geosearch/showlocation", handler);
           //  ^^^−−−− or, again, whatever the "unregister" function is
       }
   }, []);
   

   That works because citiesRef is a stable reference to an object (it will always point to the same object, consistent across calls to your component function), and we update citiesRef.current on every call, so the code in the handler is always using an up-to-date version of the cities.

See also the answers to these questions (they talk about state members rather than props, but the underlying issue the same same — referencing stale copies of them:

• React useState hook event handler using initial state
• Wrong React hooks behaviour with event listener