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 #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.