Hacker News new | past | comments | ask | show | jobs | submit login
A better way to represent loading state in ReactJS/Redux apps (nikolay.rocks)
34 points by NikolayN on June 18, 2017 | hide | past | favorite | 30 comments



> Option 1: Use a string or null/false data [...] If you're using a type checker like #flow, it will yell at you and for a good reason: data type changes are hard to reason about and very error prone in practice.

This isn't true. Flow is great at enforcing that all potential types/values are handled in an if statement. If the following type was used, the code missing the failure case would have been picked up by Flow:

    type Pics = 'fetching' | 'failed' | Array<Pic>
> Option 2: use an object with two props [...] someone accidentally changes the status without changing the data. Or the other way around. This will result in nasty and hard to debug issues where the state and data don't match visually.

Again, this can be avoided by using the correct union in Flow, which will pick up right away if the two properties have become inconsistent:

    type PicsState =
      | { status: 'fetching', list: null }
      | { status: 'failed', list: null }
      | { status: 'fetched', list: Array<Pic> }


Hadn't seen the union approach in #2 before, I like that a lot. The 2 named properties are intuitive to other developers reading it, but now we also get compile-time safety that the combinations they use are valid. Thanks!

For anyone else that this was new to, Flow has documented these as "disjoint unions": https://flow.org/en/docs/types/unions/#toc-disjoint-unions


I would propose a fourth option, use two reducers to decouple loading of data with actual data. Both reducers listen to the same events and update independently. Your data ends up looking like { picturesLoading: false/true, pictures: [] }, which everyone can understand. Your reducers look like:

    const picsLoadingReducer = (state = false, action) => {
      switch (action.type) {
        case PICS_REQUEST: return true;
        case PICS_FAILURE: return false;
        case PICS_SUCCESS: return false;
        default: return state;
      }
    };

    const picsReducer = (state = [], action) => {
      switch (action.type) {
        case PICS_FAILURE: return action.payload;
        case PICS_SUCCESS: return action.payload;
        default: return state;
      }
    };
IMO this is a bit more simple since we don't mix two concepts (am I loading data, what is the data to be rendered).


This is effectively the same solution as the authors second option.


Yep! However the reason why I've written this was not to demonstrate my own creativity it was more to show that the concerns that the author had with option 2 disappear when you approach the problem from a different perspective. Your codebase doesn't become more complex as you add more actions since the loading reducer doesn't need to change. You are also less likely to run into bugs since a boolean is a better primitive for the behavior he is describing.


This doesn't avoid the split brain concern completely though: your suggestion has 4 states, and one of them is invalid (picturesLoading: true, pictures: non-empty).

If a refactoring or some other edit of the source leads to that state, the pictures may not get rendered (if the pictures are right but picturesLoading is wrong), or the wrong pictures may get rendered (if picturesLoading is right and pictures is wrong).


I'm confused by why you think that there is an "invalid" state in this situation.

The canonical example for that state is pagination. I have thousands of pictures loaded on a server but due to resource constraints the server only responds with 25 pictures. When I scroll to a specific point we make a request to load more pictures. Therefore I end up in the state of pictures being non empty and picturesLoading being true. If I were in charge the UI for that application, I might change the opacity of the container to signal that we are loading in new data. When we get a response from the server we can either append the data to the pictures array or we can replace it with the new data. Depending on how you want to build your product, either choice is valid.


The article talked about representing 3 states, and you suggested encoding those 3 states in a way that has the same split-brain issue that the article discussed.

You can tell that 4 states doesn't match up with the article based on how the loading state is rendered - the pictures are ignored in those examples.

Now sure, if you decide you really do need 4 states, then that's a different story and your suggestion works just fine. But that's not a solution to the article's problem.


This is actually pretty bad. In redux dev tools you will not see if data is being fetched or failure occured. Also, if you serialize and deserialize state object - say save it in local storage - empty array will be neither fetching nor failure one and strange bugs will occur.


I'm only passingly familiar with redux (read the tutorial + parts of the docs a few times, never actually used it), where is the problem? As I understand it the status of the request is encoded in the store's state. The (de)serialization can be handled as part of the fetch:

    const fetchPics = () => dispatch => {
      dispatch({ type: PICS_REQUEST });
      const cached = deserialize(localStorage.get('pics'));
      if (cached && cached.length > 0) return dispatch({ type: PICS_SUCCESS, ...});

      return api.get('/pics')
        ...
    }


The problem is if you try to serialize and deserialize the entire state tree itself, including the loading/failed state. LOADING_STATE, FAILURE_STATE, and an empty array all convert to the same JSON, so if you take your entire state tree, save to JSON somewhere, then load it back, you won't get what you want.

The ability to take your whole application state, save it somewhere (e.g. localstorage), then load it back later is one of the cool things enabled by redux, and it also makes it easier to implement things like time-travel debugging.


I can see that. Thanks!


Good point! I'm confused by the articles suggestion as it already talks about Flow which has support for string literal types. I.e. undefined | "loading" | "failed" | ActualData


Just because it has a union type, it doesn't mean it's optimal to use it.


Use ADTs, described on a high level with something like this:

    data Pics
        = PicsLoading
        | PicsFailed
        | PicsLoaded (Array Pic)
Now you only need to figure out how you want to map it into assembler (eh, JavaScript). But that's largely a mechanical task. Like for example a compiler does.

Use a object with a tag field (like advocated by redux and well-supported in flow/typescript), or an array where the first element is a numeric tag and the rest payload (that's roughly the bytecode that a Haskell compiler would produce), or, god forbid, classes with instanceof checks or abstract class methods (hello Java).


We've extracted something like this logic into a higher order function that we use to generate reducers++.

Fetching data with Redux is kinda broken, though. If a dispatch is done in componentWillMount, a synchronous update like setting a loading flag is not visible in the first render, so most spinners have subtle bugs. Kinda crazy that this common use case is not supported.


Why use componentWillMount and not componentDidMount? From the docs:

> componentWillMount() is invoked immediately before mounting occurs. It is called before render(), therefore setting state synchronously in this method will not trigger a re-rendering. Avoid introducing any side-effects or subscriptions in this method.

> componentDidMount() is invoked immediately after a component is mounted. Initialization that requires DOM nodes should go here. If you need to load data from a remote endpoint, this is a good place to instantiate the network request. Setting state in this method will trigger a re-rendering.


My point isn't so much loading data from a remote endpoint, but setting state. As the doc you quote says: componentWillMount should not trigger rerendering, and should have the newest state in the render function.

But when you use Redux this isn't true! The first render will see the old state of the store even if you did changes to it right befofe. And then you immediately get a new rerender with the correct state. This cannot be fixed with how react-redux is currently implemented, and leads to subtle bugs, especially when unmounting and remounting stuff that tries to clear some state.

Edit: more details https://github.com/reactjs/react-redux/issues/210


As Dan said when closing that issue, this seems to me how I would expect rendering in React to work... it's async by nature.

If you need the component's state/props to be a certain way for the very first render, use `this.state = {...}` in the contructor, or wait until the Redux store is in the correct shape before mounting the component. (I'm guessing this is what you've done in your HOC, so this sounds like the right solution to me).

Calls like `dispatch` and `setState` shouldn't be expected to reflect in a component's rendering synchronously. And as far as I know, as of Fiber, componentWillMount is even going to become a method that could be called multiple times before the component actually mounts... its usage is pretty widely discouraged for most cases: https://github.com/facebook/react/issues/7671


With algebraic data type like in Haskell, the state could be typed like this:

    data State a = 
      Loading
      | Failed
      | Loaded [a]
Compared to a representation like `{ loading:true, error: false, pics:[] }`, the above model has the benefit of "making illegal state not representable".

One library in js I like to use to model this type of tagged union / ADT values is single-key[1]. Each variant in a tagged union type is represented by an plain object with just one key, for example:

    const state1 = { Loading: null };
    const state2 = { Failed : <reason> };
    const state3 = { Loaded : [...pics] };
The library offers pattern matching syntax on top of this data representation:

    // render
    const reactElement = match(state, {
      Loading : () => <Loading />,
      Failed  : (reason) => <Error {...reason}/>,
      Loaded  : (pics) => <PicsList pics={pics} />
    });

    // reducer
    const reducer = (state, action) => match(state, {
        Loading : () => {
          switch (action.type) {
            case PICS_FAILURE:
              return { Failure: action.payload };
            case PICS_SUCCESS:
              return { Loaded : action.payload };
            default:
              return state
          }
        },
        Failed : () => {
          switch (action.type) {
            case PICS_REQUEST:
              return { Loading: true };
            default:
              return state;
          }
        },
        Loaded : () => {
          switch (action.type) {
            case PICS_REQUEST:
              return { Loading: true };
            default:
              return state;
          }
        }
      });
[1] https://www.npmjs.com/package/single-key


This is exactly the kind of issue that go away with elm.

http://blog.jenkster.com/2016/06/how-elm-slays-a-ui-antipatt...


I would say the nice thing here is the type system.

I always use `null` to differentiate between an empty list or string and nothing.

I like when I have a type system that ensures I check for null etc. But I only work on very small projects so this works well in js for me as well.


I think that this issue is nicely solved by https://github.com/arunoda/react-komposer


Relying on JS illogical behaviour:

  [] === [] // false
It's more confusing and error prone than other options mentioned in the article.


I understand it's popular to heap abuse on any and all JS features, but please don't jump on that bandwagon.

There's nothing whatsoever illogical about two mutable objects that are not identical being reported as unequal, and it's hard to think of any language that has mutable values at all where there isn't an equivalent feature.


I don't think that particular example is illogical. it's basically the same as writing something like

    new String[1] == new String[1]
in Java, which would also be false.


I will just add that even though I disagree with proposition of that article, understanding this JS behavior is key in writing big redux apps, since it is main source of wasted renders optimizations. If you do it wrong, whole app might rerender on smallest changes.


These solutions only have one status for the entire list, not per item.

Its common to only get a partial list (e.g. a search, or paginated) or read/update just one item. I expected this "better way" to accomodate for that. The oldest "flux" examples already did.


One drawback is you can't render the list and the loader at the same time. Which means whenever doing pagination or refresh, it will always render a empty list with a loading spinner instead of rendering the current list and a loading spinner of top.


A little related to redux data fetching - have any of you tried https://github.com/amplitude/redux-query ?




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: