CODEWITHSUNDEEP
javascriptreactjssetstatereact-props
user2456977
user2456977

Reputation: 3964

Setting state in render by using this.state

I've recently seen this type of react pattern where the state is being set in a render by using this.state:

class ShowMe extends React.Component {

    constructor(props) {
        super(props);

        this.state = {
            showButton: false,
        };
    }

    render() {
        if (this.props.show) {
            this.state.showButton = true; //setting state in render!!
        }

        return (
            <div>
                <div> Show or hide button </div>
                {this.state.showButton && <Button content='Btn'/>}
            </div>
        )
    }
}

This seems like an anti-pattern. Can this cause bugs? It seems to work properly though.

I would just use a component lifecycle to set the state:

class ShowMe extends React.Component {

    constructor(props) {
        super(props);

        this.state = {
            showButton: false,
        };
    }

    componentWillReceiveProps(nextProps) {
        if(nextProps.show) {
            this.setState({
                showButton: true,         
            })
        }
     }

    render() {
        return (
            <div>
                <div> Show or hide button </div>
                {this.state.showButton && <Button content='Btn'/>}
            </div>
        )
    }
}

What is the recommended way?

Upvotes: 6

Views: 2403

Answers (3)

Ali Ankarali
Ali Ankarali

Reputation: 3127

It is an anti-pattern. If showButton state is not always equal to show props (which is the case in the example), I would use this:

class ShowMe extends React.Component {

    constructor(props) {
        super(props);

        this.state = {
            showButton: this.props.show,
        };
    }

    componentDidUpdate(prevProps, prevState) {
        prevProps.show !== this.props.show && this.setState({showButton: this.props.show})
    }

    render() {

        return (
            <div>
                <div> Show or hide button </div>
                {this.state.showButton && <Button content='Btn'/>}
            </div>
        )
    }
}

Edit: As of React 16.3 one should use getDerivedStateFromProps in this case.

Note that componentWillReceiveProps will be deprecated.

From the docs: getDerivedStateFromProps is invoked after a component is instantiated as well as when it receives new props. It should return an object to update state, or null to indicate that the new props do not require any state updates.

https://reactjs.org/docs/react-component.html#static-getderivedstatefromprops

Upvotes: 4

Artem Mirchenko
Artem Mirchenko

Reputation: 2170

It is incorrect setting state in render method. You can set state in lifecyles method. But other thing is that your component can receive same props many times, so your component will be set state many times, and renderd. To solve this problem you need to compare your new with your current props for example compare json objects: 

componentWillReceiveProps(nextProps) {
    if(JSON.stringify(this.props) !== JSON.stringify(nextProps) && nextProps.show) {
        this.setState({
            showButton: true,         
        })
    }
 }

or use PureComponent. And that garentee you that your component will not rerendered constantly.

And it will be better if you do not rerender component if state.showButton currently seted to true. 

componentWillReceiveProps(nextProps) {
    if(JSON.stringify(this.props) !== JSON.stringify(nextProps) && nextProps.show) {
       if(!this.state.showButton) {
         this.setState({
            showButton: true,         
        })
       }

    }
 }

Upvotes: 1

Hamza Fatmi
Hamza Fatmi

Reputation: 1255

render should always be pure without any side effects, so it's certainly a bad practice.

from the React docs :

The render() function should be pure, meaning that it does not modify component state, it returns the same result each time it’s invoked, and it does not directly interact with the browser. If you need to interact with the browser, perform your work in componentDidMount() or the other lifecycle methods instead. Keeping render() pure makes components easier to think about.

Take a look also here and here.

Upvotes: 4

Related Questions