dispatching in componentWillMount

[jedborovik]

Do we expect dispatching in componentWillMount to take effect before the component is rendered?

For example, this test fails:

it('should handle dispatches before componentDidMount', () => {
  const store = createStore(stringBuilder)

  @connect(state => ({ string: state }) )
  class Container extends Component {
    componentWillMount() {
      store.dispatch({ type: 'APPEND', body: 'a' })
    }

    render() {
      expect(this.props.string).toBe('a'); // => Error: Expected '' to be 'a'
      return <Passthrough {...this.props}/>
    }
  }

  const tree = TestUtils.renderIntoDocument(
    <ProviderMock store={store}>
      <Container />
    </ProviderMock>
  )
})

[gaearon]

Would you like to add this test and see if you can fix it? Might also be related to #196.

[lsapan]

@jedborovik did you find a solution to this? Adding something like this to render functions is a bit ugly: if (!this.props.xyz) return

[gaearon]

As I said in the previous comment it might be related to another bug report. Please help investigate and fix it.

[lsapan]

Sorry, I'd taken a look at #196 and from a glance didn't think it was related. This is more an issue of deliberately changing the state after redux has already set the props. In any case, I tried applying #196 locally and the issue still persists.

The react docs mention that if you change a component's state during componentWillMount, render will only be called once (with the updated state). With that in mind, I'm guessing the issue is actually that redux is updating the props, which react doesn't expect to happen in componentWillMount. As such, react goes ahead and calls render a second time.

I haven't had time to look into the source for react-redux too much, but I'll take a look around. I'm guessing any solution to this may be a bit dirty.

[lsapan]

@gaearon alright after doing some digging, I found the problem. connect doesn't call trySubscribe until componentDidMount. As such, it doesn't see the dispatch/state change that happens in componentWillMount. I'll add a componentWillMount and some logic to catch this scenario.

[lsapan]

Alright I guess I spoke too soon. I'm really not sure how we can work around this problem. I made the mistake of confusing componentWill/DidMount on the Connect class with the one on the wrapped class. The problem is that componentWillMount and componentDidMount will both run on the Connect class before render (obviously), but that also means they happen before componentWillMount on the wrapped class.

In other words, we can't stop ourselves from calling render the first time because the dispatch hasn't happened yet. I'm at a loss here.

Here's a test case, I didn't want to open a PR for it because I don't have a solution:

it('should only call render once (with the updated data) when a state change occurs during componentWillMount', () => {
  const store = createStore(stringBuilder)

  @connect(state => ({str: state}))
  class Container extends Component {
    componentWillMount() {
      store.dispatch({ type: 'APPEND', body: 'a' })
    }

    render() {
      expect(this.props.str).toBe('a')
      return (<div></div>)
    }
  }

  TestUtils.renderIntoDocument(
      <ProviderMock store={store}>
        <Container />
      </ProviderMock>
  )
})

[gaearon]

Please do open a PR with a failing case, it makes it easier for others to try to fix it.

[jedborovik]

Roger that. Just opened PR https://github.com/rackt/react-redux/pull/222.

[gaearon]

I looked at it and I think React works as intended here. By the time componentWillMount fires on the child, it is too late to change the props child receives. Since store dispatch changes child's props, this must happen on the next render.

[evandrix]

@jedborovik did you find a solution to this? Adding something like this to render functions is a bit ugly: if (!this.props.xyz) return

@lsapan: can't do return in render, because Uncaught Invariant Violation: <X>.render(): A valid ReactComponent must be returned. You may have returned undefined, an array or some other invalid object.

[lsapan]

@evandrix you can return an empty div, I just didn't write it out.

[Matsemann]

if (!this.props.xyz) ... doesn't solve all the issues.

We have a component that loads some data in componentWillMount by calling an action. The action makes a reducer set a "loading"-property synchronously (with the default value of true). Inside the render function, we can then do if (this.props.loading) return <Spinner />. The action is a thunk that later (async) makes a reducer set loading to false when it has received data. This works.

However, when the component is unmounted and later mounted (for instance going to another page in the react-app and back), this doesn't work as expected. The data is again fetched by an action inside componentWillMount and the "loading"-property in the store is set to true. But in the first render, this.props.loading is still false, seeing the old value from before the store was updated in componentWillMount. So instead of seeing the spinner, whatever was done when not loading happens for a brief instance, until it's rerendered again, this time with loading set to true as it should have been.

In our case, this has the unwanted side-effect of generating a rather expensive chart for the old data, that is immediately thrown away anyway. How can this be avoided in a nice way when we inside the render function only sees the old store data, not the data being set in willMount?

[kristian-puccio]

Use a higher order component to dispatch the action and to not render the child component until the required props are present.

[Matsemann]

Don't think that will work? As stated, this is an issue the second time the component is mounted, as the old data is still present. So the props are there for a brief moment, just with the old values, so checking that they are present wont solve it.

[kristian-puccio]

You check for the presence of the required props in a higher order component. And that component that wraps the component that cares about certain props is prevented from rendering by doing a return

in the higher order component.

This sort of talks about what I'm trying to say http://natpryce.com/articles/000814.html

[Matsemann]

But that higher order component will still see the wrong data, and thus not really know if to render the wrapped component or not. We actually have a component similar to the one you link. The problem is that when it uses redux instead of setState, data set in a store in componentWillMount is not present in render.

[kristian-puccio]

Correct you just need to render something else in the HOC until the correct props arrive. The HOC shouldn't care if it has the props that the inner component require.

On 31 March 2016 at 23:08, Mats Krüger Svensson notifications@github.com wrote:

But that higher order component will still see the wrong data, and thus not really know if to render the wrapped component or not. We actually have a component similar to the one you link. The problem is that when it uses redux instead of setState, data set in a store in componentWillMount is not present in render.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/reactjs/react-redux/issues/210#issuecomment-203902337

[chrishoage]

@gaearon would something like Alt.js componentDidConnect be applicable to react-redux? This seems like a valid case that would be nice to have support for.

https://github.com/altjs/connect-to-stores/issues/6

https://github.com/altjs/utils/blob/master/src/connectToStores.js#L80-L88

[gaearon]

1) Adding custom lifecycle hooks isn’t a great API moving forward as React is trying to get away from classes. This wouldn’t work for functional stateless components or (future) functional stateful components. 2) I’m not sure what use case this would solve? The changes will be “picked up” in any case, it’s just that they will get picked up on the next render. If not, please submit a failing test case.

[chrishoage]

That is a good point.

After more thought, the cases I was thinking of could be solved with firing actions in React Router event handlers.

Thanks for explaining!

[yn5]

@chrishoage Did you manage to dispatch actions in the react-router event handlers and use the changed data in the component? I'm getting the same results as I would when dispatching in componentWillMount.

@gaearon I'm having the issue where I need the changes in my first render because it being on the server side and if it would be picked up later it causes my server side rendered HTML would differ from the HTML on the client. So the changes being "picked up" on the next render is not good enough, or am I overseeing a solution to this?

[gaearon]

@gaearon I'm having the issue where I need the changes in my first render because it being on the server side and if it would be picked up later it causes my server side rendered HTML would differ from the HTML on the client.

Usually the solution is to hydrate data first and then render. So rather than render in a lifecycle hook, you would get the matched components from the router, call a static method on them which dispatches an action, and wait for the promise to resolve. Does this make sense? https://github.com/markdalgleish/redial is one way of doing it.

[yn5]

Usually the solution is to hydrate data first and then render. So rather than render in a lifecycle hook, you would get the matched components from the router, call a static method on them which dispatches an action, and wait for the promise to resolve. Does this make sense? https://github.com/markdalgleish/redial is one way of doing it.

Thanks for your response @gaearon! I do hydrate my store before rendering. If I understand correctly that is not the problem. To be a little more specific: I have a parameter to specify the language in the url. I would like to call an action to set the language with that value and use the newly selectedLanguage from the store in my component on the first render in order to render the app with the correct language. So what I tried first is calling the action setLanguage in the componentWillMount of the highest order component and came to understand the props (which are connected to the redux state_ can not be updated before the component renders. So I went with @chrishoage's suggestion to dispatch the selectLanguage action inside the onEnter hook of my highest order route but throughout the first render of the component this newly selectedLanguage does not get picked up.

[gaearon]

I have a parameter to specify the language in the url. I would like to call an action to set the language with that value and use the newly selectedLanguage from the store in my component on the first render in order to render the app with the correct language.

I think the problem is you’re effectively duplicating the data between the store and the router. Why not just use the router params for this? Is there any particular reason you prefer it coming from the store?

[yn5]

I think the problem is you’re effectively duplicating the data between the store and the router. Why not just use the router params for this? Is there any particular reason you prefer it coming from the store?

Since there are other ways to change the language than just with the URL I would have to then have to duplicate the state between the store and the router if I understand correctly.

Edit: I think you're right though, I should not try to do this with redux, I'll try to only use the params only. Thanks a lot for your help @gaearon.

[gaearon]

Since there are other ways to change the language than just with the URL I would have to then have to duplicate the state between the store and the router if I understand correctly.

You would want to change URL anyway, at which point it’s easier to treat router as the source of truth for the URL.

[mihirsoni]

@gaearon I have sync action which fill up data in store while dispatching action using higher order component componentWillMount , but I am facing issue that the values are not defined with the WrappedComponent on the server side rendering. But client side it renders two times and it works. Am I doing something wrong ?

import React, { PropTypes, Component } from 'react';
import { connect } from 'react-redux';

export default function decoratedComponent(options = {}) {
  return (WrappedComponent) => {
    class HOC extends Component {
      constructor(props) {
        super(props);
         //Binding other methods
      }
      componentWillMount() {
        this.props.actions.initialize(this.props); // This initialize state from the props
      }
       render() {
       console.log(this.props.form) // This is undefined or {} which initial state.
        return(
          <WrappedComponent
          {...this.props}
          />);
      }
    }
    function mapStateToProps(state) {
      return {
        form: state.forms
      };
    }
    return connect(mapStateToProps)(HOC);
  };
}

[Strato]

For the curious, I encountered this problem and ended up doing this:

public constructor(props) {
    super(props);

    this.state = {
        isFirstRender: true
    };
}

private componentWillMount() {
    // Initial data load
    this.props.getData();
}

private componentDidMount() {
    this.setState({ isFirstRender: false });
}

public render() {
    const isLoading = this.props.isFetching || this.state.isFirstRender;

    return (
        <div className={classNames({
            'loading': isLoading
        }) }>
            REDACTED
        </div>
    );
}

I'm not superfan of this solution but that's the best I could pull off...

[rafaeltikva]

Any new recommendations how to deal with this situation? This is a real pain in the ass... I'm dispatching an action from componentWillMount that changes isFetching to true, but this doesn't get picked up by the render() method... I'm using react-router, so the componentWillMount is ideally the best option to change the application state when entering a new page (or using the onEnter hook). I ended up using @Strato's suggestion, but this seems like a hacky solution...

[chriswu14]

I like to know if there is an update on this as well

[jimbolla]

There's a passing test in connect.spec.js that says this should work: 'should handle dispatches before componentDidMount'. Please provide a failing test if it's not working for your use case.

[Matsemann]

@jimbolla a case was provided as a PR further up, #222. The test in the top as well should be good enough to understand the issue. The current test is basically wrong, it doesn't assert what it's supposed to do.

However, this was closed as working as intended. I'd argue it's not as intended, just a weakness in how connect works and more of a wont-fix.

[jordanmkoncz]

I'm also looking for a good solution to this problem. I have a component that dispatches an action (to fetch data from an API) in its componentWillMount which will update a slice of my state to set isLoading: true. The initial state for this slice is {isLoading: false, data: []}, which means that a component I connect to this state will initially render with the value of isLoading being false, then a brief moment later the state is updated as a result of the dispatched action and the component will render again, this time with the value of isLoading being true. And of course, shortly after this a success/failure action will be dispatched that sets isLoading: false and triggers another render.

The issue this causes is that in the component I connect to this state, I can't do the following:

render() {
    const {isLoading, data} = this.props; // Injected via connect

    if (isLoading) {
        return <Spinner />;
    }

    if (!data.length) {
        return <Message>No results.</Message>;
    }

    return <SomeComponent list={data} />;
}

The intention here is of course to render <Spinner/> until we have received the response from the API, and then to render <Message/> or <SomeComponent/> based on the response. But on the first render isLoading will actually be false so <Message/> will be rendered for a brief moment, then <Spinner/> for 1-2 seconds, and then finally the correct component based on the API response.

Anyone have a solution to get the intended behaviour here without having to do hacky 'first render' checks such as @Strato's solution?

[naw]

@jordanmkoncz

At a high level, I believe the issue is you're trying to squish 3 distinct states into only 2.

Here are your three states:

1. don't have data
2. fetching data
3. have data (even if it's empty data)

You cannot adequately represent this as 2 states:

1. fetching data
2. have data

This issue is intrinsic to fetching initial data asynchronously; it's not specific to redux or react. There is no escaping it, although it's feasible to build abstractions around components and data fetching so you can hide these details from individual components.

One way to handle this for an individual component is to introduce another boolean in your state slice like haveFetchedData that is initialized as false and set to true once you have receive data.

Another way is to use null as your initial value for data, and change it to an array (including an empty one) once you have received data.

Your component can display the spinner until haveFetchedData is true or until data is not null.

[naw]

@jordanmkoncz

Also, depending on your exact needs, the simplest change to get the correct spinner behavior is to make the initialState for isLoading be true, even though you haven't actually started fetching yet. This will result in correct spinner behavior, at the risk of not being able to dinstinguish between "not fetching yet" and "currently fetching".

Or, for more clarity, you could rename isLoading to notFetchedOrIsLoading and set its initial value to true.

[jordanmkoncz]

@naw At one point I was actually trying to solve this using one of those suggestions; for every isFetching (initialised as false) I also had an isFetched (initialised as false), and then in mapStateToProps I'd have logic like const isLoading = isFetching || !isFetched; and pass this isLoading variable to my component in order to get the behaviour I wanted in the component.

However I ran into an issue with this solution as well - it works when rendering a component for the first time, but if I switch to a different component (e.g. via route change) and then back again, the isLoading logic will not have the same effect, because when the component is rendered isFetching will be false but isFetched will actually be true, which will cause the same issue as before except instead of empty data being rendered for a brief moment, the old data from the previous call is what will be rendered for a brief moment. This problem would also happen with your solution to initally set data to null.

[naw]

@jordanmkoncz

Yes, the issue you point out is certainly a very real issue, and one that I've experienced myself.

There are various challenges that arise when you build a single page application using a store that persists across different URL's. In a traditional server-rendered application, every time you land on a new URL, all of your data is thrown away and fetched from scratch synchronously, and then passed to your template. You don't have to worry about a template getting the wrong data from an old URL

In a single page application with a store (i.e. redux/react with react-router), all of your data is just sitting there, and nothing automatically marks it as "stale" when you visit a new URL. There might be abstractions you can build on top of redux/react that will help with this, but vanilla redux/react doesn't solve this problem for you.

Suppose you have a <BlogPost> component that displays the content of a blog post, based on an ID in the URL (e.g. example.com/blog/5 and example.com/blog/27). If you have a slice in your redux store responsible for holding the "current" blog post content, merely having isFetched and isFetching booleans will be inadequate just as you said.

The solution to this problem is similar to the solution I mentioned previously ---- you need to identify all of the distinct states you might find yourself in, and make sure your store slice has adequate information to help you distinguish these states, or at least distinguish the ones that matter (i.e. should I show a spinner or not). You actually have 4 distinct states:

1. data not fetched
2. data fetched, but data is for the wrong blog post id
3. fetching data
4. fetched data, and data is for the correct blog post id

One way to implement this is adding a blogPostId field to your store slice:

{
  data: "Content for a blog post",
  blogPostId: 5
  isFetching: false,
  isFetched: true
}

Then, if your component connects to this slice and sees that the blogPostId (5) is different than the blogPostId provided in the URL (27, via react-router params), it knows to display the spinner. Once the fetch for post 27 is received, you update the slice, and the component re-renders without the spinner since the ids match.

Another way to organize your state is to have a slice that holds all fetched blog posts in a hash by id. Your component knows the desired blog post id (e.g. from react-router params), and reaches into that slice to find the correct blog post --- if the key for that id is missing, you display the spinner and wait for the correct blog post to be received.

There really is no getting around this, unless you use a higher-level abstraction on top of react/redux. Personally I'm still brainstorming on building such an abstraction for my own projects. Until then, I believe the aforementioned techniques are adequate, albeit a little painful. The reality is that redux is a low-level tool, not a high-level tool, so you have to do more from scratch unless you're using other tools on top of it.

Finally, just a a warning in case you haven't run into this yet -- if you're using react-router and link directly from blog post 5 to blog post 27, the <BlogPost> component does not get re-mounted (i.e. componentDidMount is not called), so if you're fetching data for a component only in componentDidMount, you probably need to consider also fetching in componentWillReceiveProps. This is just a ramification of how react-router and react work.

You have some great questions, and I'm just responding because I've run into the same issues and spent a lot of time thinking about it. If anyone knows of a simpler way to solve this with vanilla redux, I'd be interesting in hearing it.

[markerikson]

@naw : this deserves to be turned into a blog post of its own, really. We could use more publicly available info on how to think in terms of app state.

[Matsemann]

@jordanmkoncz Yeah we faced the same problem, as written far up here somewhere. One can make it work for the first render, but that only postpones the problem to when the component is hidden and shown again. It becomes a lot of boilerplate and pit falls to make sure a simple spinner can be shown.

@naw, great post

At a high level, I believe the issue is you're trying to squish 3 distinct states into only 2.

I disagree with this. The code declares an invariant that should hold, but then Redux goes ahead and breaks it.

This issue is intrinsic to fetching initial data asynchronously; it's not specific to redux or react.

and

There really is no getting around this, unless you use a higher-level abstraction on top of react/redux.

It's specific to Redux. If one had used setState from React instead of dispatching an action in componentWillMount, the correct props would be available on first render. This is a promise made in React's API, so no wonder people get confused when this doesn't prove to be true when switching state handling to Redux.

[markerikson]

@Matsemann : Not sure what "invariant" you're referring to. I also don't see this as anything specific to Redux. It looks like this applies to any use of the "container component" pattern, where a presentational component is asking a parent component to fetch data. That means that the data is coming in via props instead of being applied internally via setState. Really, the "odd" part about this is that React tries to optimize the "setState during componentWillMount" case. Otherwise, you'd expect that to cause a second render as well.

[naw]

@Matsemann Yes, I think you make a great point!

You are correct that the React API for componentWillMount specifies that state changes will take place before the first render. It's easy to expect a synchronous dispatch to a Redux store from within componentWillMount to behave the same way.

I agree this is confusing, and I think it's worth discussing how (or if) its feasible to improve it.

To be clear, this is not a Redux issue, it's a ramification of how the react-redux bindings are implemented.

As @markerikson said, react-redux pushes everything from the store down to connected components via props. This means at the time of mounting, the props have already been pushed down, and there is no way for the component to intercept those props with an immediate state change like you can do with setState.

Back to the isFetching stuff: I hope you can agree that there are three distinct states --- the question is which of those states render needs to be capable of handling? In vanilla React, you can ensure that render never "sees" one of those states, by running your setState({ isFetching: true}) in componentWillMount (I believe this is the invariant that @Matsemann is talking about).

The problem in react-redux is there is no way from within the component itself to change props; by the time you're inside componentWillMount, it's too late.

As far as I know, there are only three ways to remedy this issue:

1. Modify react-redux to subscribe to the Redux store from within your component (rather than from within a wrapper around your component). This likely has many ramifications that could lead to different problems, although perhaps it's worth exploring.
2. Modify your application so that your component never sees certain states (i.e. find a different way to ensure that { isFetching: true} is the first state seen by your component). Ultimately this means putting your fetch dispatch somewhere outside of your component (personally, this is what I do)
3. Modify your application so that render can handle all 3 states (which is what I proposed earlier to @jordanmkoncz ).

Ultimately (and perhaps unfortunately), it's not as simple as blindly swapping out setState for dispatch.

I agree with @markerikson that this is intrinsic to a parent-child props relationship. However, a potential problem is that it's not conceptually obvious that react-redux is using such a relationship in its implementation of connect.

In other words, we are not encouraged to think of connected components as presentational "children" receiving props from a connected wrapper --- instead, we tend to think of the component being connected and the resulting decorated component as one-and-the-same. At least, that's my perception. It's pretty common to see connected components that have data fetching inside of them, which can lead to problems.

Perhaps react-redux or Redux needs to clearly delineate suggestions for how you might need to modify your application to handle these subtleties?

One conceptual way to handle these subtleties is to treat your components as presentation only as @markerikson suggested. In other words, force yourself to think of your components as simply receiving state with no ability to change state (as opposed to what you might traditionally do in componentWillMount)

Personally I take presentation only to an extreme -- I use a modified version of connect that accepts a componentWillMount argument that runs in the context of the connected wrapper instead of the underlying wrapped component. This means the component knows nothing about fetching data. It also means that my dispatch occurs before my component is mounted, which means I actually get the behavior you desire (i.e. my render function doesn't have to deal with the "not fetching yet" state). However, this is not a simple library tweak we could code into react-redux -- it's a fundamental shift in how I (or you) think about components.

TL;DR

In vanilla React, components are meant to be a mix of state management and presentation.

In vanilla Redux, if you try to mix state management and presentation, you can run into non-obvious problems.

Ultimately, the switch from state to props is subtle, but significant, and you cannot blindly change setState to dispatch.

Perhaps react-redux docs could do a better job at helping people navigate these subtleties?

Perhaps react-redux could be rewritten to use state instead of props? (various difficulties in doing that, I believe).

I appreciate the discussion and would welcome your thoughts.

[markerikson]

In other words, we are not encouraged to think of connected components as presentational "children" receiving props from a connected wrapper --- instead, we tend to think of the component being connected and the resulting decorated component as one-and-the-same. At least, that's my perception.

Hmm. That's exactly how I view things - wrapper container and presentational component. Admittedly, I'm a really bad example case - I'm obviously intimately familiar with how connect is implemented (at least at the conceptual level, and partially at the implementation level), and my own app doesn't do any component-driven fetching anyway. But yeah, it seems pretty straightforward to me that if you do some kind of data fetching in componentWillMount, that will effectively always render twice: once without the data, and a second time when that data comes back. React's optimization for the setState case is almost confusing here, especially given that setState is usually asynchronous.

However, a potential problem is that it's not conceptually obvious that react-redux is using such a relationship in its implementation of connect.

I love the simplified version of connect that Dan wrote to clarify the conceptual behavior: https://gist.github.com/gaearon/1d19088790e70ac32ea636c025ba424e . Beyond that, I'd have to read through the docs and various other articles to see how well the "wrapper" aspect is emphasized.

[naw]

Thanks for your perspective and also the link, @markerikson

For context, let me explain my personal philosophy (just an opinion):

I believe a truly presentational component should:

1. Never fetch data
2. Never use dispatch

A presentational component receives props. Those props are generally either a piece of data to render, or a function to call when something interesting (like a click) happens. These pieces of data generally are slices of Redux state, or at least derived from Redux state. The functions are often bound-action-creators. (I know you know this, @markerikson, just trying to be very clear within the discussion)

I don't think a presentational component should ever dispatch something like dispatch({type: FETCH_RECORDS}) or call a function like this.props.fetchRecords() --- if it did that, I wouldn't call it a presentational component.

I suppose you could consider "I am mounting" as something interesting, in which case the parent could expose a function through props likethis.props.iAmMounting(), which could fetch data indirectly. In this case, it would be very clear that you have to render() with your existing props, and patiently wait for your parent to react to your iAmMounting() call by delivering new props to you.

If we did think about our components like this, which I believe was your point farther up, then the subtleties discussed in this issue could be avoided. I think we're on the same page in this regard. :heart:

So, one question is whether people generally "get" that they need to construct their components differently when they use Redux than when they use vanilla React? Or, is it trivial to convert an application from vanilla React to Redux? Can (or should) we make it more trivial to do so?

There are two extremely different conceptual ways to approach this:

1. Encourage a high-level of separation between the wrapped presentational component and the connected wrapper.
2. Encourage React-like hybrids where state management and presentation are mixed.

My opinion is that the current react-redux implementation falls in the middle somewhere, doing a reasonably good job at both, but not a spectacularly good job at either.

A better version of (1) would put things like componentWillMount inside the wrapper, and explicitly discourage anything but presentation in the wrapped component.

A better version of (2) would subscribe to the Redux store directly inside the component (i.e. no wrapper), so that the component's state can be manipulated directly (e.g. mapReduxStateToComponentState instead of mapStateToProps)

To be clear, I'm not trying to rag on react-redux --- It's great --- I'm just brainstorming some ideas for improvement --- I hope nobody's toes get crunched.

[jimbolla]

Coincidentally, FB is maybe considering depreciating componentWillMount

[markerikson]

@jimbolla : dude, you beat me to it by 13 seconds. :(

[jimbolla]

@markerikson

[jordanmkoncz]

Great posts @naw, you've definitely helped clarify the subtle implications of how connect is implemented and the fact that the component created by connect represents a parent-child props relationship between the parent component and the wrapped component. Your <BlogPost> example is spot on and I agree that there are actually 4 different states we can be in.

After taking this into consideration, I've come up with a solution to this problem. I'll extend your <BlogPost> example to explain my solution.

I'm using normalizr for my project, which manages an entities slice of my store's state that holds all fetched blog posts in a hash by id (as you suggested). I also have a separate slice of my store's state, visibleBlogPosts, which has the following initial state:

{
    id: null,
    isFetching: false,
}

My visibleBlogPostsReducer looks like this:

case FETCH_BLOG_POST_REQUEST:
    return {
        ...state,
        isFetching: true
    };
case FETCH_BLOG_POST_SUCCESS:
    return {
        ...state,
        id: action.response.result, // The ID of the fetched blog post
        isFetching: false
    };
case FETCH_BLOG_POST_FAILURE:
    return {
        ...state,
        id: null,
        isFetching: false
    };

When I successfully fetch a given blog post, it will be added to entities.blogPosts, and my visibleBlogPosts will update to have the id of the fetched blog post.

Given this state shape and reducer functionality, I now have the following variables which are used to determine my loading state at any given time:

• object - The Blog Post object I want to render, which may be present but stale (i.e. found in my entities cache), or present and fresh (i.e. just fetched), or not present at all.
• objectId - The ID of the Blog Post I want to render, which is a number.
• storedObjectId - The ID of the Blog Post in my store (i.e. visibleBlogPosts.id), which is a number or null.
• isFetching - Whether we're currently fetching a Blog Post (i.e. visibleBlogPosts.isFetching), which is true or false.

Based on these variables, I want to reduce my loading state at any given time to be one of the following:

• 'FIRST_LOADING' - Object is loading for the first time (object is not present).
• 'RELOADING' - Object is reloading (object is present but stale).
• 'LOADED' - Object is loaded (object is present and fresh).

I can do so with the following logic:

const getLoadingState = (object, objectId, storedObjectId, isFetching) => {
    if(isFetching || objectId !== storedObjectId) {
        if(object) {
            return 'RELOADING';
        }

        return 'FIRST_LOADING';
    }

    return 'LOADED';
};

Given these 3 possible loading states, I can have the following logic in my component.

class BlogPostContainer extends React.Component {
    componentDidMount() {
        const {blogPostId, dispatch} = this.props;  // Injected via connect

        dispatch(fetchBlogPost(blogPostId));
    }

    componentDidUpdate(prevProps) {
        const {blogPostId, dispatch} = this.props;  // Injected via connect

        if (prevProps.blogPostId !== blogPostId) {
            dispatch(fetchBlogPost(blogPostId));
        }
    }

    render() {
        const {blogPost, blogPostId, storedBlogPostId, isFetching} = this.props;  // Injected via connect

        loadingState = getLoadingState(blogPost, blogPostId, storedBlogPostId, isFetching);

        if (loadingState === 'FIRST_LOADING') {
            // Just render spinner.
            return <Spinner />;
        }

        if (loadingState === 'RELOADING') {
            // Render our stale blog post, but with a semi-transparent overlay and spinner over the top.
            return (
                <SpinnerContainer>
                    <BlogPost blogPost={blogPost} />
                </SpinnerContainer>
            );
        }

        // If desired, I could also just return <Spinner /> if loadingState !== 'LOADED'.

        // loadingState === 'LOADED', so render our fresh blog post.
        return <BlogPost blogPost={blogPost} />;
    }
}

Of course, in my actual implementation I'm using constants rather than direct string comparisons, and I've created some abstractions for getting the current loading state, but I've deliberately tried to be explicit here.

So far, this solution appears to work well and keeps state/reducer boilerplate to a minimum (e.g. no need to maintain a separate isFetched variable). I'm curious to hear people's thoughts on this and whether there are any problems with this solution.

[naw]

@jordanmkoncz

At first glance this looks like a solid approach. However, there are some nuances to consider.

object - The Blog Post object I want to render, which may be present but stale (i.e. found in my entities cache), or present and fresh (i.e. just fetched), or not present at all.

In the scenario where you visit /blog/5, and then visit /some_other_page, and then visit /blog/5 (for a second time), you have no way of distinguishing if object is stale or fresh. storedObjectId will match immediately even though the object is stale, which means you'll have LOADED for a brief time before you switch to RELOADING and then back to LOADED. This may or may not be a big deal depending on your application. The storedObjectId is really mostRecentlyReceivedObjectId, and just because an object is the most recently received object doesn't mean it is fresh (indeed, it could be seconds, minutes, hours, or days old).

Also, it's feasible for AJAX calls to return in the wrong order. Suppose you visit /blog/5, and then /blog/27, but you receive the response for 27 before you receive the response for 5. In this case, you'll see:

1. FIRST_LOADING spinner
2. LOADED content for 27 when 27 returns
3. LOADING spinner perpetually once 5 returns

Probably the easiest way to guard against this is to prevent more than one fetch at a time. However, some features need to fire off more than one request at at time (like an autocomplete query, for example), so you may have to use more complicated techniques.

[alien109]

@naw

Any chance you've found a decent way to work around this? I have a very similar problem and been banging my head on the wall for a few days with no success. I'm new to React and Redux which is making this even more frustrating.

[naw]

@alien109 could you clarify what specific issue you've been having trouble with?