DEV Community

Cover image for React race condition bug
Sagiv ben giat
Sagiv ben giat

Posted on • Edited on • Originally published at debuggr.io

React race condition bug

Originally posted on my personal blog debugger.io

If your application is depended on a state that gets updated asynchronously, there is a good chance you have a bug. The bad news is that its hard or even almost impossible to reproduce in production. The good news is that you now know about it and we will learn how to reproduce it and fix it.

In this article I will use a demo application that I used in a previous article React state update on an unmounted component. Although reading it is not a prerequisite, I do think its a good idea to read it.

πŸ‘€ I've uploaded a starter repo to github so you won't have to copy paste the code.
You can clone and run it locally or use the import feature of codesandbox.io

This is how our application looks like:

Showing a drop down of pets and selecting a dog or cat

Basically we are selecting a pet and showing some info that we "fetch" from the server.

This is how the Pets component looks like:

function Pets() {
  const [pets, dispatch] = useReducer(petsReducer, initialState);

  const onChange = ({ target }) => {
    dispatch({ type: "PET_SELECTED", payload: target.value });
  };

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

  return (
    <div>
      <select value={pets.selectedPet} onChange={onChange}>
        <option value="">Select a pet</option>
        <option value="cats">Cats</option>
        <option value="dogs">Dogs</option>
      </select>
      {pets.loading && <div>Loading...</div>}
      {pets.petData && <Pet {...pets.petData} />}
    </div>
  );
}
Enter fullscreen mode Exit fullscreen mode

Our Pets component uses the useReducer hook to store some state.
Lets see the petsReducer and the initial state:

const initialState = { loading: false, selectedPet: "", petData: null }

function petsReducer(state, action) {
  switch (action.type) {
    case "PET_SELECTED": {
      return {
        ...state,
        selectedPet: action.payload
      };
    }
    case "FETCH_PET": {
      return {
        ...state,
        loading: true,
        petData: null
      };
    }
    case "FETCH_PET_SUCCESS": {
      return {
        ...state,
        loading: false,
        petData: action.payload
      };
    }

    case "RESET": {
      return initialState;
    }

    default:
      throw new Error( `Not supported action ${action.type}` );
  }
}
Enter fullscreen mode Exit fullscreen mode

As you can see theres nothing special here, a simple reducer that manage our state.

The Pets component also use the useEffect hook for some side effects like fetching the data of our selected pet, we invoke the getPet function which returns a Promise and we dispatch the FETCH_PET_SUCCESS action with the returned data as the payload to update our state.

Note that getPet is not really hitting a server end-point, its just a function that simulate a server call. This is how it looks like:

const petsDB = {
  dogs: { name: "Dogs", voice: "Woof!", avatar: "🐢" },
  cats: { name: "Cats", voice: "Miauuu", avatar: "🐱" }
};

export function getPet(type) {
  return new Promise(resolve => {
    // simulate a fetch call
    setTimeout(() => {
      resolve(petsDB[type]);
    }, 1000);
  });
}
Enter fullscreen mode Exit fullscreen mode

As you see, its nothing but a setTimeout inside a Promise.

The bug

So far everything looks great, we chose a pet type from the Drop-down and we get the info 1000ms later. Though when we are dealing with asynchronous operations we can't determine at what point of time exactly we are running our code, moreover we need to handle 2 or more operations simultaneously. What happens when the first operation is slower than the second operation? How are we dealing with the results?

Imagine this scenario:

  1. The user select the Cats option.
  2. We are fetching the Cats data from the server.
  3. The user now select the Dogs option.
  4. We are fetching the Dogs data from the server.
  5. for some reason, the Dogs data received before the Cats data (yeah it happens!).
  6. We display the Dogs data on the screen.
  7. Couple of milliseconds later, the Cats data is received.
  8. We display the Cats data on the screen, but the Drop-down still shows the Dogs as selected.

This is how it looks like on the screen:
showing the bug, selecting cats then dogs and displaying cats data

How did we managed to do it? just a hard-coded longer delay for the cats type:

export function getPet(type) {
  const delay = type === "cats" ? 3500 : 500;
  return new Promise(resolve => {
    // immulate fetch call
    setTimeout(() => {
      resolve(petsDB[type]);
    }, delay);
  });
}
Enter fullscreen mode Exit fullscreen mode

The problem

Why is that happening? lets revisit our data fetching logic in useEffect:

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

As you can see, our state update (using dispatch) is running inside the .then() function. It will run only when the Promise returned by getPet is resolved. When the user select a different option before the Promise is resolved, we trigger getPet again with its own .then() function. When the second (but faster) call is resolved, we run the function passed to .then() and updating the state with the passed in data object (Dogs data). When the first call is resolved, we run the function passed to it's .then() and updating the state with the passed in data object, a WRONG and none relevant data! Yes, the one with the cats πŸ™€πŸ™€πŸ™€

The solution

One possible solution is to cancel the first request, we can use the AbortController.abort() (⚠️ experimental technology) or we can implement a Cancelable promise.

If you can't or don't want to use these solutions, there is another solution. Basically our problem is that we store a key for the selected pet but we update the data object without checking that the data correspond to that key. If we will check that the key and the data correspond and only then we will trigger the update, we will not have this problem.

Lets see how can we do that.

Trial #1 (❌)

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

Here we are storing the selectedPet key in a different temp variable _previousKey and then inside the .then() function we check if the "current" selectedPet matches _previousKey.

This won't work! We just override the _previousKey variable each time the useEffect is running, so we end up matching the same value over and over. This is also true if we were to declare the _previousKey variable outside the useEffect at the function component level scope, because it will run on each render.

Trial #2 (❌)

let _previousKey;

function Pets() {
  //... 

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

  return (...);
}
Enter fullscreen mode Exit fullscreen mode

Here we are declaring the _previousKey outside the component's scope, this way we always get the latest value and not overriding it on each render or effect call.

Although it seems to be working fine and our problem is solved, we introduce a new bug. If we will have 2 different instances of Pets rendered, they will "share" this variable and will override it to each other.

Trial #3 (βœ”οΈ)

function Pets() {
  //...
  const _previousKeyRef = useRef(null);

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

  return (...);
}
Enter fullscreen mode Exit fullscreen mode

In trial #2 we made some progress but we ended up with kind of a "global" variable. What was missing is a variable attached to the instance of our component. In class components we would use the this key word to reference the instance -- this._previousKey. In function components the this key word doesn't reference the component's instance because there is no instance (you can read more about the this key word in JavaScript - The "this" key word in depth). React solved the lack of instance issue with the useRef hook. Think of it as a mutable state object for your component that doesn't trigger a re-render when you update it (unlike useState or useReducer).

This way we can safely store the _previousKey and compare it to the current selectedPet and only if they match, update our state with the relevant data object. If you run the code now you will see that we fixed our bug πŸ™Œ

Trial #3.5 (βœ”οΈ)

useEffect(() => {
  let abort = false;

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

  return () => abort = true;

}, [pets.selectedPet])
Enter fullscreen mode Exit fullscreen mode

This is another possible solution. Instead of keeping track on the matching values, we can just use a simple flag that indicates if we should keep with our update state operation. Every time the effect runs we are initialising the abort variable with false, inside the cleanup function of the effect we set it to true. the effect will only run at first render and every time one of the values passed to the dependencies array is changed. The cleanup function will run just before each cycle of the effect and when the component is unmounted.

This works great and probably the preferred solution for some people, but keep in mind that now your effect can't have other none related logic with none related dependencies in the array (and it shouldn't have!), because then the effect will re-run if those dependencies change and will trigger the cleanup function which will flip the abort flag.

Nothing is stopping you from having multiple useEffect functions, one for each logic operation.

Custom useEffect

If we want to get real crazy with our hooks, we can create our own custom useEffect (or useLayoutEffect) which will provide us the "current status" of the effect:

function useAbortableEffect(effect, dependencies) {
  const status = {}; // mutable status object
  useEffect(() => {
    status.aborted = false;
    // pass the mutable object to the effect callback
    // store the returned value for cleanup
    const cleanUpFn = effect(status);
    return () => {
      // mutate the object to signal the consumer
      // this effect is cleaning up
      status.aborted = true;
      if (typeof cleanUpFn === "function") {
        // run the cleanup function
        cleanUpFn();
      }
    };
  }, [...dependencies]);
}
Enter fullscreen mode Exit fullscreen mode

And we will use it in our Pet component like this:

  useAbortableEffect((status) => {
    if (pets.selectedPet) {
      dispatch({ type: "FETCH_PET" });
      getPet(pets.selectedPet).then(data => {
        if(!status.aborted){
          dispatch({ type: "FETCH_PET_SUCCESS", payload: data });
        }
      });
    } else {
      dispatch({ type: "RESET" });
    }
  }, [pets.selectedPet]);
Enter fullscreen mode Exit fullscreen mode

Note how our custom effect callback now accepts a status argument which is an object that contains an aborted boolean property. If it is set to true, that means our effect got cleaned and re-run (which means our dependencies are changed or the component was un-mounted).

I kind of like this pattern and i wish react useEffect would get us this behavior out of the box. I even created an RFC on the react repo for this if you want to comment or improve it.

Good news

Note that this is not a react specific problem, this is a challenge that most if not all of the UI libraries or framework are facing, due to the nature of asynchronous operations and state management. The good news is that the react team is working on a great feature called Concurrent Mode and one of its features is Suspense which should cover this issue out of the box.

Wrapping up

We saw that a simple component with a state and asynchronous operation can produce a nasty bug, we might not even know it's there until we face it in production. My conclusion is that whenever we update a state (can be local or in a state manager) inside an asynchronous callback, we must check if the arguments that we passed to the asynchronous function is corresponding to the data we received in the callback.

Hope you found this article helpful, if you have a different approach or any suggestions I would love to hear about them, you can tweet or DM me @sag1v. πŸ€“

For more articles you can visit debuggr.io

Top comments (9)

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])
Collapse
 
ap13p profile image
Afief S

Hi, great article! πŸ˜„

But, I think using useRef feels like magic to me, because I never know whether useEffect will update ref.current or not.
So, instead of using useRef, I use something like a skip method.
For example:

useEffect(() => {
  let canceled = false;

  getPet(pets.selectedPet)
    .then(data => {
      if (!canceled) dispatch({ type: "FETCH_PET_SUCCESS", payload: data });
  });

  return () => canceled = true;
}, [pets.selectedPet])

In the above snippet I know for sure (I have tried it before) that dispatch will not run if pets.selectedPet are changed, because useEffect will always call tear down function whenever it's dependencies array are changed.

Collapse
 
sag1v profile image
Sagiv ben giat • Edited

Thanks for the feedback 😊
I agree, this is another possible solution and it should work great.
The only thing to look for here, is that your effect doesn't have different logic with none relevant dependencies (which it shouldn't have!), though i did see some mixed logic inside effects in some code sources.

I think i will add this as another solution in the post. Thank you πŸ™

Edit
Forgot to mention, regarding the magic of ref.current in effects. Its actually just a mutable box handled by react, not different than state or reducers really. It will change every time the effect runs, its just attached to the "instance" of the component so you can access it outside the effect's scope.