Sign in
Log inSign up

Why we should not have browserHistory.push('/some-path') in render() method ?

Ghanshyam K Dobariya's photo
Ghanshyam K Dobariya
·Nov 2, 2018

Below is my code

class LoginContainer extends Component {
   ...
   render() {
     return <LoginApp  isAuthenticated={this.props.isAuthenticated} />
   }
}

/* ==> Implemetation of LoginApp is like below <== */

import { browserHistory } from 'react-router';
class LoginApp extends Component {
   ...
   render() {
     if (this.props.isAuthenticated) {
       browserHistory.push('/dashboard');
       return null;
    }
    /* code to render login form elements....*/
  }
}

This code was giving warning like below when true is passed as value of isAuthenticated to LoginApp

Warning: setState(...): Cannot update during an existing state transition (such as within render or another component's constructor). Render methods should be a pure function of props and state; constructor side-effects are an anti-pattern, but can be moved to componentWillMount.

As of now not able to make any changes in LoginApp component so made a small workaround in LoginContainer (which I really didn't like)

Changes I brought is

  1. Not passing value of "isAuthenticated" to LoginApp component.
  2. Redirecting from container component itself via component did mount & did update

New implementation of LoginContainer is as below

class LoginContainer extends Component {
     ...
    componentDidMount() {
      if (this.props.isAuthenticated) {
        browserHistory.push(prependAppRoute('/'))
      }
    }

   componentDidUpdate() {
     if (this.props.isAuthenticated) {
        browserHistory.push(prependAppRoute('/'))
     }
   }
   render() {
     return <LoginApp />
   }
}

And now I don't have that warning & I am in a mode - My code is working (As there is no warnings) but I don't know why 🙄??

React version: 15 React-router version: 2.8.1