DEV Community

Discussion on: React race condition bug

Collapse
 
tbroyer profile image
Thomas Broyer

How about sending the selected pet along with the pet data in the FETCH_PET_SUCCESS dispatch, and handing this in the reducer, depending on the selected pet stores in the current state?
That way at least you handle the state all in one place (I must I quite like the 3.5 option too)

Collapse
 
sag1v profile image
Sagiv ben giat

Not sure i follow, can you post a code snippet?

Collapse
 
tbroyer profile image
Thomas Broyer

Something like:

useEffect(() => {
  let selectedPet = pets.selectedPet;
  if (selectedPet) {
    dispatch({ type: "FETCH_PET" });
    getPet(selectedPet).then(data => {
      dispatch({ type: "FETCH_PET_SUCCESS", selectedPet, payload: data });
    });
  } else {
    dispatch({ type: "RESET" });
  }
}, [pets.selectedPet])

and

    case "FETCH_PET_SUCCESS": {
      if (state.selectedPet != action.selectedPet) {
        // out-of-order event, ignore
        return state;
      }
      return {
        ...state,
        loading: false,
        petData: action.payload
      };
    }
Thread Thread
 
sag1v profile image
Sagiv ben giat

This way the UI won't update untill we get back the data, which is fine for the info but may get a poor user experience for the drop down.
All in all this is just a verry simple example to show a possible bug, obviously the code could be a lot better, like validations etc 🙂

Thread Thread
 
tbroyer profile image
Thomas Broyer

Hmm, did you try it? codesandbox.io/s/new-e6gpr

Here's the diff:

  useEffect(() => {
-   if (pets.selectedPet) {
+   let selectedPet = pets.selectedPet;
+   if (selectedPet) {
      dispatch({ type: "FETCH_PET" });
-     getPet(pets.selectedPet).then(data => {
-       dispatch({ type: "FETCH_PET_SUCCESS", payload: data });
+     getPet(selectedPet).then(data => {
+       dispatch({ type: "FETCH_PET_SUCCESS", payload: data, selectedPet });
      });
    } else {
      dispatch({ type: "RESET" });
    }
  }, [pets.selectedPet])

and

    case "FETCH_PET_SUCCESS": {
+     if (state.selectedPet != action.selectedPet) {
+       // out-of-order event, ignore.
+       return state;
+     }
      return {
        ...state,
        loading: false,
        petData: action.payload
      };
    }
Thread Thread
 
sag1v profile image
Sagiv ben giat

Aw you kept the onChange handler. I thought you meant to omit it and instead "listen" to the "FETCH_PET_SUCCESS" in order to update the drop-down.

Thread Thread
 
tbroyer profile image
Thomas Broyer

Fwiw, to me, this feels cleaner than somehow duplicating the selectedPet state into a useRef.

Also, while the local selectedPet variable I used is not actually needed, I feel it reads better, but YMMV, using pets.selectedPet everywhere would work just as well. First diff would be reduced to:

  useEffect(() => {
    if (pets.selectedPet) {
      dispatch({ type: "FETCH_PET" });
      getPet(pets.selectedPet).then(data => {
-       dispatch({ type: "FETCH_PET_SUCCESS", payload: data });
+       dispatch({ type: "FETCH_PET_SUCCESS", payload: data, selectedPet: pets.selectedPet });
      });
    } else {
      dispatch({ type: "RESET" });
    }
  }, [pets.selectedPet])