54

We have an action that fetches an object async, let's call it getPostDetails, that takes a parameter of which post to fetch by an id. The user is presented with a list of posts and can click on one to get some details.

If a user clicks on "Post #1", we dispatch a GET_POST action which might look something like this.

const getPostDetails = (id) => ({
  type: c.GET_POST_DETAILS,
  promise: (http) => http.get(`http://example.com/posts/#${id}`),
  returnKey: 'facebookData'
})

This is picked up by a middleware, which adds a success handler to the promise, which will call an action like GET_POST__OK with the deserialized JSON object. The reducer sees this object and applies it to a store. A typical __OK reducer looks like this.

[c.GET_ALL__OK]: (state, response) => assign(state, {
  currentPost: response.postDetails
})

Later down the line we have a component that looks at currentPost and displays the details for the current post.

However, we have a race condition. If a user submits two GET_POST_DETAILS actions one right after the other, there is no guarantee what order we recieve the __OK actions in, if the second http request finishes before the first, the state will become incorrect.

    Action                   => Result
    ---------------------------------------------------------------------------------
|T| User Clicks Post #1      => GET_POST for #1 dispatched => Http Request #1 pending
|i| User Clicks Post #2      => GET_POST for #2 dispatched => Http Request #2 pending
|m| Http Request #2 Resolves => Results for #2 added to state
|e| Http Request #1 Resolves => Results for #1 added to state
 V

How can we make sure the last item the user clicked always will take priority?

merours
  • 4,076
  • 7
  • 37
  • 69
Luís Deschamps Rudge
  • 1,106
  • 1
  • 10
  • 12

2 Answers2

102

The problem is due to suboptimal state organization.

In a Redux app, state keys like currentPost are usually an anti-pattern. If you have to “reset” the state every time you navigate to another page, you'll lose one of the main benefits of Redux (or Flux): caching. For example, you can no longer navigate back instantly if any navigation resets the state and refetches the data.

A better way to store this information would be to separate postsById and currentPostId:

{
  currentPostId: 1,
  postsById: {
    1: { ... },
    2: { ... },
    3: { ... }
  }
}

Now you can fetch as many posts at the same time as you like, and independently merge them into the postsById cache without worrying whether the fetched post is the current one.

Inside your component, you would always read state.postsById[state.currentPostId], or better, export getCurrentPost(state) selector from the reducer file so that the component doesn’t depend on specific state shape.

Now there are no race conditions and you have a cache of posts so you don’t need to refetch when you go back. Later if you want the current post to be controlled from the URL bar, you can remove currentPostId from Redux state completely, and instead read it from your router—the rest of the logic would stay the same.


While this isn’t strictly the same, I happen to have another example with a similar problem. Check out the code before and the code after. It’s not quite the same as your question, but hopefully it shows how state organization can help avoid race conditions and inconsistent props.

I also recorded a free video series that explains these topics so you might want to check it out.

Kostas Minaidis
  • 4,681
  • 3
  • 17
  • 25
Dan Abramov
  • 264,556
  • 84
  • 409
  • 511
  • 1
    I have a bit different situation. What should I do if I also have `currentPostId` and `postById`, but I have to fetch data about all posts at time (time when created, preview picture, meta, etc) and then fetch the content of only one selected post and cache this content with everything else about this post in `postById`? – denysdovhan Jul 27 '16 at 11:10
  • 1
    @DanAbramov Hi I know this is a pretty old question, but since any action creator doesn't get access to the state, how can the action creator decides whether it was cached or it needs to fetch the remote object? – Vinz243 Apr 01 '17 at 14:37
  • 1
    What if you have `currentListOfSearchResults` which is an array, instead of just one `currentPost`? I usually end up skipping Redux for these results and just putting them in local state for my page component. – Wayne Bloss Oct 15 '17 at 03:37
  • 1
    I appreciate this solution. However, I cannot find a good explanation of what to do in the following scenario: 1. You have parents that can be created dynamically. 2. Each parent has 5 children that all make fetch requests. These children each have their own reducer to facilitate this. 3. If I create a new parent in the app, I either need to create new children reducers OR cancel all inflight requests of previous active parent & initiate new requests for currently active parent. Anyone encountered a similar scenario? I've read and tried Dan's reply for code splitting, but doesn't work. – prufrofro Mar 09 '18 at 20:35
  • 1
    @Dan Abramov, this is great. I have one question regarding this pattern. How often are we going to clean states from store? – Samuel May 28 '18 at 05:09
5

Dan's solution is probably a better one, but an alternative solution is to abort the first request when the second one begins.

You can do this by splitting your action creator into an async one which can read from the store and dispatch other actions, which redux-thunk allows you to do.

The first thing your async action creator should do is check the store for an existing promise, and abort it if there is one. If not, it can make the request, and dispatch a 'request begins' action, which contains the promise object, which is stored for next time.

That way, only the most recently created promise will resolve. When one does, you can dispatch a success action with the received data. You can also dispatch an error action if the promise is rejected for some reason.

Cam Jackson
  • 11,860
  • 8
  • 45
  • 78
  • 6
    Note that we don’t recommend storing promises in the state. Ideally it should be serializable. I do, however, describe the approach you suggested in [this lesson](https://egghead.io/lessons/javascript-redux-avoiding-race-conditions-with-thunks?course=building-react-applications-with-idiomatic-redux). It’s a good approach but doesn’t replace normalizing state. (Combining both approaches is a good idea.) – Dan Abramov Jun 10 '16 at 01:37
  • 2
    This solution doesn't seem to me like it does what it is supposed to. When a second request comes in, you want to abort the first one. However, the promise in the store seems to act as a guard which prevents any other requests being made whilst it is in flight: 'The first thing your async action creator should do is check the store for an existing promise, and abort it if there is one.' – Richard Hunter Apr 24 '18 at 19:22
  • 1
    @Richardinho By "and abort it if there is one", I mean to abort that existing request, not to abort the new one being made. Does that make more sense? – Cam Jackson Apr 27 '18 at 07:05
  • 1
    Yes, it did sound like you were saying that if there is an existing request then you just abort and don't do anything else. – Richard Hunter Apr 28 '18 at 10:49