React-redux: Triggering async data request in ES6 "constructor()" or "componentWillMount()" or "componentDidMount()"

[bmueller-sykes]

(I'm fairly new to React and Redux, and so I'm still trying to figure out the best coding patterns and practices, so apologies if this is obvious to everybody but me.)

I'm working on a React/Redux app which has several components which request their own data. For example, when a "/users" route is invoked, the <Users /> component gets invoked, and that component triggers an async call (via redux-thunk) which updates the Redux store with the fetched user data, and then ultimately the users data appears within the <Users /> component.

The render() function within <Users /> is just watching for changes to this.props. The parent of <Users /> watches the Redux store, and of course passes the user data down.

This all worked fine in React 0.13. Or at least it "worked" with no errors/warnings in the console. I updated to 0.14 late last week, and started to see the following warning in the logs:

Warning: setState(...): Cannot update during an existing state transition (such as within render). Render methods should be a pure function of props and state.

I floundered around, and finally figured out that the problem was that my async data request was in the class constructor. Something like this:

constructor(props) {
    super(props);
    fetchUsers();
}

render() {
    return ( do the stuff to output the user data );
}

If I moved the fetchUsers() call to _either_ componentDidMount() or componentWillMount(), everything functioned properly, and the error went away.

So my question is ultimately this: what is going on here? What is the functional difference between these three functions? My (obviously incorrect) assumption was that I should request the data when the class gets initialized, which is why I put the async call in constructor(). Why does that not work, why do those two React lifecycle methods work, and which one of the two is preferable? React's docs say componentDidMount() is the right method to use, but I would think I'd want the data _before_ the component is mounted, which make me think componentWillMount() should be the method I use.

[gaearon]

Is the warning coming from React Redux's usage of state?
Or are you putting data from store into the state manually?
Can you provide an example reproducing the issue?

That said, using componentWillMount is the way to go. Why React doesn't encourage setState() calls inside constructor, I'm not sure, but it's better to ask in React issues than here.

[bmueller-sykes]

Thanks for the reply, Dan. I posted the question here because the problem _seems_ to be originating in redux-thunk.

I can do my best to piece together an example. This isn't exactly the code I have executing, but it is, I think, a close enough approximation to illustrate the issue.

index.js:

ReactDOM.render(
    <Provider store={store}>
    <App />
    </Provider>,
    document.getElementById('root')
);

app.js:

render () {
    <GetUsers />
}

export default connect (state => state)(App);

getusers.js

import React, { Component, PropTypes } from 'react';
import {connect} from 'react-redux';
import { fetchUsersFromServer } from "../actions"
class GetUsers extends Component {

constructor (props) {
    super(props);
    this.props.dispatch ( fetchUsersFromServer () );
}

render () {
    return (
        <OutputStuffHere data={this.props.users} />
    );
}
};


export default connect( (state) => {
return {
    users: state.users
}
})(GetUsers);

actions.js

function getUsers (payload) {
return {
    type: GET_USERS,
    payload: payload
}
}

function receiveGetUsers (payload) {
return {
    type: RECEIVE_GET_USERS,
    payload: payload
}
}

export function fetchUsersFromServer () {
return (dispatch) => {
    dispatch ( getUsers() );
            // doesn't matter if you make an async call here or not
            // a single "dispatch()" call will trigger the error
    // dispatch ( receiveGetUsers() );
}
}

reducers.js

function getUsers (state = defaultState.users, action) {
switch (action.type) {
    case GET_USERS:
        return Object.assign({}, state, {
            isFetching: true
        })
    case RECEIVE_GET_USERS:
        if (action.payload.success === true) {  
            return Object.assign ({}, state, {
                isFetching: false,
                courses: action.payload.data
            });
        } else {
            return Object.assign ({}, state, {
                isFetching: false,
                data: {},
                error: {
                    hasError: true,
                    message: action.payload.message
                }
            });
        }
    default:
        return state;
}
}

I hope the sample code is somewhat useful. What I found somewhat interesting was that in the fetchUsersFromServer() call, it didn't matter if I actually made an async request or not, as I indicate in the comment above. I guess maybe that means this is just me not understanding the React lifecycle properly. Again, I posted this problem here because the problem gets exposed within the thunk, though I realize that doesn't mean the thunk itself is at fault.

So if the real answer here is "make async calls in componentWillMount(), then I'm happy with that answer. I suppose I'm just trying to get a better understanding of how these parts work together, and why this issue only became apparent when I upgraded to React 0.14.

Thanks again!

[gaearon]

Is it too much to ask of you to create a test in this project that emits this warning?
This would help us fix it and ensure the right thing happens instead.

[bmueller-sykes]

Sure, I'll do that.

[bmueller-sykes]

Okay, I've got a simple working test, based on npm/webpack. I'm not sure what I should do with it at this point. I could upload it to my own github account, and then point you to it, I suppose. I wasn't sure if you actually wanted me to upload the whole thing to the react-redux repo. What makes the most sense from your perspective?

[gaearon]

Sorry, I meant a unit test.

git clone https://github.com/rackt/react-redux.git
cd react-redux
npm install
npm run test:watch

This should launch our test runner.

Then you can add the test emitting the warning to connect.spec.js (see other tests there for inspiration). To make it easier, you can write it.only() instead of it() so only your test runs.

Does this make sense?

[bmueller-sykes]

Yes, it does. I admit I've never written a unit test before, so figuring out how to do that is a) useful for me, but b) will take some time. This isn't urgent for me, but if it's somewhat urgent for you, i could at the very least zip up the sample project I have and get that to you somehow--at least that would illustrate the issue I'm seeing.

[gaearon]

No urgency—take your time!

[gaearon]

By the way, I wasn't able to reproduce this myself.
Are you sure it's a problem with react-redux@latest?

[bmueller-sykes]

I’ve set this aside for now, but yeah, let me fire it up again, make sure I’m on the latest libraries from NPM, and test again.

On Oct 7, 2015, at 1:09 PM, Dan Abramov <[email protected][e-mail]> wrote:

By the way, I wasn't able to reproduce this myself.
Are you sure it's a problem with react-redux@latest?

—
Reply to this email directly or view it on GitHubhttps://github.com/rackt/react-redux/issues/129#issuecomment-146281090.

[jesenko]

I just encountered this issue with
react-redux: 3.1.0
redux: 3.0.2
react: 0.14.0

Moving async actions to componentWillMount fixed it for me...

[jesenko]

Note that I could not consistently reproduce this; sometimes it raised warning and sometimes it did not, I assume it could be related to batching of updates that React does to optimize perf?

[gaearon]

@jesenko Can you try creating a test that emits the warning?

[bmueller-sykes]

Hi Dan,

I know this may be somewhat less than ideal, but I'm not going to be able to create a unit test in timely fashion, so in absence of that, I have created a small sample project that should demonstrate the problem. Or, at least, it demonstrates it on my machine. Here's the repo:

https://github.com/bmueller-sykes/redux-setstate-test

Once you run npm install and start, you should be able to to go localhost:3000, and then toggle between the two links. If you look in the console, you should see the setState() warnings.

FWIW, this project is on React 0.14, react-redux 3.1.0, redux 3.0.2, and redux-thunk 1.0.0.

The code which appears to trigger the error is in src/components/users.js. You'll see that the fetchUsers() call is currently invoked in constructor(). If you move the call to the currently commented-out componentWillMount(), then everything appears to work properly.

Again, apologies that this is not an official unit test, but at this point it seems like getting a working test in front of you sooner rather than later is at least a decent step forward.

Thanks!

[gaearon]

Thanks for putting up a repro! I'll look into it with a couple of days.
If somebody feels like turning this into a failing test, please do.

[bmueller-sykes]

Cool. Let's hope it's useful!

[esamattis]

Tried creating a failing test for this

https://github.com/rackt/react-redux/compare/master...epeli:fail129?expand=1

but it's not failing! I don't know why. Seems like it should because the constructor and dispatch is called between the component render calls which would indicate it's part of the render but I guess it's not...

btw @gaearon I had a weird issue with the testing environment https://github.com/rackt/react-redux/commit/bba883ab347064d5d75c05a222a874fbeda17061

[esamattis]

@bmueller-sykes your example is not actually async but actually this issue seems to be about sync dispatches from constructor anyway.

https://github.com/epeli/redux-setstate-test/commit/62c3dc885fb17b598ac74559ac6f6adc15ef401c

[bmueller-sykes]

Yes, my apologies if I didn't say that. I had thought that the problem was with an async data call, and therefore perhaps some race condition was being encountered. Then I pulled out the async data call, but left the "thunk" stuff in place, and just made the two dispatch calls, and I still saw the problem. And it doesn't happen every time, either--in the test I built, it appears to happen roughly every other time. That kind of looks like a React lifecycle issue, but again, that's only a marginally informed guess.

As I said at the top, things work (or at least I see no errors in the log) if the call is put in componentWillMount(). Maybe this means that React is doing some type of cleanup on calls that are in that method as opposed to constructor()?

[jesenko]

I also tried creating isolated unit test for this, but could not get it to emit warning...dispatching multiple actions in constructor resulted in no warnings...

[gaearon]

This is not a bug in React Redux.

This happens when dispatch() inside one component's constructor causes a setState() inside another component. React keeps track of the “current owner” for such warnings—and it thinks we're calling setState() inside the constructor when technically constructor _causes_ a setState() inside some other part of the application.

I don't think we should handle this—it's just React trying its best do its job. The solution is, as you correctly noted, to dispatch() inside componentWillMount() instead.

[bmueller-sykes]

Hi Dan,

Thanks for the reply. I was never certain this was a “bug” or just "bad practice by me", nor if it were a bug, where the bug was. It just seemed like this repo was the most likely place I’d get an answer (if it was in react, redux, or react-redux).

So I have a question and a reaction.

The question is, regarding the sample app I posted, what are the two components in question? You said a dispatch() in one component triggers a setState() in another component. In my example, is that App and Users? And why did dispatch() in one component cause a setState() in another? Is that due to a bad coding pattern, or something internal to React? I think I'm still trying to understand what the functional difference is between constructor() and componentWillMount(), since they are both called once at the start of a component's life cycle.

My reaction to this is that perhaps I wish there were documentation somewhere about when constructor() and componentWillMount() should be used in React. It seems like the lesson here is that you shouldn't use constructor to execute Redux calls, but I don't know if there are other gotchas out there. I don't think I'm suggesting that you need to be the one to write this documentation--I'm more just throwing it out to the universe.

Anyway, thanks!

[gaearon]

My reaction to this is that perhaps I wish there were documentation somewhere about when constructor() and componentWillMount() should be used in React.

Just don't execute side effects in constructor. It's only for initializing state (and perhaps other variables). Don't make calls or change the state of your app from there.

The question is, regarding the sample app I posted, what are the two components in question? You said a dispatch() in one component triggers a setState() in another component. In my example, is that App and Users?

A route change caused dispatch which caused mounting Users which caused another dispatch in its the constructor. So, dispatch inside connect(Users) caused a setState inside connect(App).

And why did dispatch() in one component cause a setState() in another? Is that due to a bad coding pattern, or something internal to React?

Any time you call dispatch(), all connect()-ed component wrappers have their setState() called so that the connected component receive that new state as their props. It's just how React Redux works.

[bmueller-sykes]

Ok, thanks.