266

I am trying to refactor the following code from my render view:

<Button href="#" active={!this.state.singleJourney} onClick={this.handleButtonChange.bind(this,false)} >Retour</Button>

to a version where the bind is within the constructor. The reason for that is that bind in the render view will give me performance issues, especially on low end mobile phones.

I have created the following code, but I am constantly getting the following errors (lots of them). It looks like the app gets in a loop:

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

Below is the code I use:

var React = require('react');
var ButtonGroup = require('react-bootstrap/lib/ButtonGroup');
var Button = require('react-bootstrap/lib/Button');
var Form = require('react-bootstrap/lib/Form');
var FormGroup = require('react-bootstrap/lib/FormGroup');
var Well = require('react-bootstrap/lib/Well');

export default class Search extends React.Component {

    constructor() {
        super();

        this.state = {
            singleJourney: false
        };

        this.handleButtonChange = this.handleButtonChange.bind(this);
    }

    handleButtonChange(value) {
        this.setState({
            singleJourney: value
        });
    }

    render() {

        return (
            <Form>

                <Well style={wellStyle}>

                    <FormGroup className="text-center">

                        <ButtonGroup>
                            <Button href="#" active={!this.state.singleJourney} onClick={this.handleButtonChange(false)} >Retour</Button>
                            <Button href="#" active={this.state.singleJourney} onClick={this.handleButtonChange(true)} >Single Journey</Button>
                        </ButtonGroup>
                    </FormGroup>

                </Well>

            </Form>
        );
    }
}

module.exports = Search;
user3611459
  • 3,311
  • 6
  • 16
  • 18
  • I have this same issue but now related to onClick(): https://stackoverflow.com/questions/57079814/react-cannot-update-during-an-existing-state-transition-such-as-within-render – Saibamen Jul 18 '19 at 15:10
  • This same issue but without onClick(): https://stackoverflow.com/questions/57079814/react-cannot-update-during-an-existing-state-transition-such-as-within-render – Saibamen Jul 18 '19 at 15:11

11 Answers11

382

Looks like you're accidentally calling the handleButtonChange method in your render method, you probably want to do onClick={() => this.handleButtonChange(false)} instead.

If you don't want to create a lambda in the onClick handler, I think you'll need to have two bound methods, one for each parameter.

In the constructor:

this.handleButtonChangeRetour = this.handleButtonChange.bind(this, true);
this.handleButtonChangeSingle = this.handleButtonChange.bind(this, false);

And in the render method:

<Button href="#" active={!this.state.singleJourney} onClick={this.handleButtonChangeSingle} >Retour</Button>
<Button href="#" active={this.state.singleJourney} onClick={this.handleButtonChangeRetour}>Single Journey</Button>
Vladimir Rovensky
  • 4,684
  • 1
  • 13
  • 16
  • I have tried that solution, and it works. But I don't see this solution online anywere as preferred solution. What I want to accomplish is that my bind is outside the render view. In this solution, this.handleButtonChange = this.handleButtonChange.bind(this); in the constructor is not needed anymore, does that mean that my binding is again in my render view? – user3611459 May 23 '16 at 10:07
  • 2
    Maybe what happens is when i set the active state, it will trigger onClick, resulting in a loop. is there any way to set active state without triggering onClick? – user3611459 May 23 '16 at 10:21
  • 2
    Why a lambda is the solution? Which concept is behind this? I can not see it. – Vicens Fayos Feb 09 '17 at 20:19
  • 42
    The main difference is between onClick={this.handleButtonChange(false)} and onClick={() => this.handleButtonChange(false)} - the first one is wrong in that it just invokes the handleButtonChange method immediately and assigns its return value (undefined) to the onClick handle - therefore nothing happens. The second one actually assigns a method to onClick - one that invokes handleButtonChange. – Vladimir Rovensky Feb 10 '17 at 09:17
  • I have a senior supervisor on React helping me from time to time. He says I should avoid anonymous functions because performance issues. He says an anonymous function allocates memory that doesn't get destroyed or something like that. I would like to know the opinion of you about this, thanks. – Becario Senior May 05 '17 at 08:03
  • There are cases where the anonymous function would hurt performance, the most common one being when the component you pass the anonymous function to as a prop (Button in the example) defines shouldComponentUpdate that compares props using === (e.g. PureComponent). Passing an anonymous function prevents this optimization from working, since a new instance of the function is created on every render, therefore the === in shouldComponentUpdate always returns false and the child component has to re-render. In such cases, it's better to use a named method like in the second half of my answer. – Vladimir Rovensky May 06 '17 at 23:05
  • You can read more about this issue in [this article](https://medium.com/@esamatti/react-js-pure-render-performance-anti-pattern-fb88c101332f). – Vladimir Rovensky May 06 '17 at 23:07
  • with onFocus={() => this.handleButtonChange(false)} is not working, but onFocus={ this.handleButtonChange(false)} works but give me error because in that function i use .setState and, it's forbiden in render... any advise? – DDave Oct 18 '17 at 09:15
  • @VladimirRovensky thanks for your clear explanation. A question: when you said that If you don't want to create a lambda... create 2 funcs, I wonder if the lamda is needed only when we want to pass an argument to an event handler. In other words: do I need a lamda just when Im trying to script a function to handle multiple options (instead than one function per each option)? – paulocleon Aug 24 '20 at 20:32
19

I am giving a generic example for better understanding, In the following code

render(){
    return(
      <div>

        <h3>Simple Counter</h3>
        <Counter
          value={this.props.counter}
          onIncrement={this.props.increment()} <------ calling the function
          onDecrement={this.props.decrement()} <-----------
          onIncrementAsync={this.props.incrementAsync()} />
      </div>
    )
  }

When supplying props I am calling the function directly, this wold have a infinite loop execution and would give you that error, Remove the function call everything works normally.

render(){
    return(
      <div>

        <h3>Simple Counter</h3>
        <Counter
          value={this.props.counter}
          onIncrement={this.props.increment} <------ function call removed
          onDecrement={this.props.decrement} <-----------
          onIncrementAsync={this.props.incrementAsync} />
      </div>
    )
  }
Ignatius Andrew
  • 8,012
  • 3
  • 54
  • 54
  • No wonder at that time, I got infinite loop exception. silly me. After that, I'm using `bind` button on `constructor` and arrow function `()=>` – Luiey Oct 01 '20 at 02:55
15

That usually happens when you call

onClick={this.handleButton()} - notice the () instead of:

onClick={this.handleButton} - notice here we are not calling the function when we initialize it

Gal Bracha
  • 19,004
  • 11
  • 72
  • 86
8

THE PROBLEM is here: onClick={this.handleButtonChange(false)}

When you pass this.handleButtonChange(false) to onClick, you are actually calling the function with value = false and setting onClick to the function's return value, which is undefined. Also, calling this.handleButtonChange(false) then calls this.setState() which triggers a re-render, resulting in an infinite render loop.

THE SOLUTION is to pass the function in a lambda: onClick={() => this.handleButtonChange(false)}. Here you are setting onClick to equal a function that will call handleButtonChange(false) when the button is clicked.

The below example may help:

function handleButtonChange(value){
  console.log("State updated!")
}

console.log(handleButtonChange(false))
//output: State updated!
//output: undefined

console.log(() => handleButtonChange(false))
//output: ()=>{handleButtonChange(false);}
Spenser Saling
  • 181
  • 2
  • 5
4

If you are trying to add arguments to a handler in recompose, make sure that you're defining your arguments correctly in the handler. It is essentially a curried function, so you want to be sure to require the correct number of arguments. This page has a good example of using arguments with handlers.

Example (from the link):

withHandlers({
  handleClick: props => (value1, value2) => event => {
    console.log(event)
    alert(value1 + ' was clicked!')
    props.doSomething(value2)
  },
})

for your child HOC and in the parent

class MyComponent extends Component {
  static propTypes = {
    handleClick: PropTypes.func, 
  }
  render () {
    const {handleClick} = this.props
    return (
      <div onClick={handleClick(value1, value2)} />
    )
  }
}

this avoids writing an anonymous function out of your handler to patch fix the problem with not supplying enough parameter names on your handler.

Nick Brady
  • 6,084
  • 1
  • 46
  • 71
3

The problem is certainly the this binding while rending the button with onClick handler. The solution is to use arrow function while calling action handler while rendering. Like this: onClick={ () => this.handleButtonChange(false) }

2

From react docs Passing arguments to event handlers

<button onClick={(e) => this.deleteRow(id, e)}>Delete Row</button>
<button onClick={this.deleteRow.bind(this, id)}>Delete Row</button>
Rahil Ahmad
  • 3,056
  • 1
  • 16
  • 21
2

This same warning will be emitted on any state changes done in a render() call.

An example of a tricky to find case: When rendering a multi-select GUI component based on state data, if state has nothing to display, a call to resetOptions() is considered state change for that component.

The obvious fix is to do resetOptions() in componentDidUpdate() instead of render().

Jari Turkia
  • 1,184
  • 1
  • 21
  • 37
1

I got the same error when I was calling

this.handleClick = this.handleClick.bind(this);

in my constructor when handleClick didn't exist

(I had erased it and had accidentally left the "this" binding statement in my constructor).

Solution = remove the "this" binding statement.

Sam Henderson
  • 479
  • 5
  • 7
1

The onClick function must pass through a function that returns the handleButtonChange() method. Otherwise it will run automatically, ending up with the error/warning. Use the below to solve the issue.

onClick={() => this.handleButtonChange(false)}
Vidya Sagar H J
  • 157
  • 1
  • 3
0

The solution that I use to open Popover for components is reactstrap (React Bootstrap 4 components).

    class Settings extends Component {
        constructor(props) {
            super(props);

            this.state = {
              popoversOpen: [] // array open popovers
            }
        }

        // toggle my popovers
        togglePopoverHelp = (selected) => (e) => {
            const index = this.state.popoversOpen.indexOf(selected);
            if (index < 0) {
              this.state.popoversOpen.push(selected);
            } else {
              this.state.popoversOpen.splice(index, 1);
            }
            this.setState({ popoversOpen: [...this.state.popoversOpen] });
        }

        render() {
            <div id="settings">
                <button id="PopoverTimer" onClick={this.togglePopoverHelp(1)} className="btn btn-outline-danger" type="button">?</button>
                <Popover placement="left" isOpen={this.state.popoversOpen.includes(1)} target="PopoverTimer" toggle={this.togglePopoverHelp(1)}>
                  <PopoverHeader>Header popover</PopoverHeader>
                  <PopoverBody>Description popover</PopoverBody>
                </Popover>

                <button id="popoverRefresh" onClick={this.togglePopoverHelp(2)} className="btn btn-outline-danger" type="button">?</button>
                <Popover placement="left" isOpen={this.state.popoversOpen.includes(2)} target="popoverRefresh" toggle={this.togglePopoverHelp(2)}>
                  <PopoverHeader>Header popover 2</PopoverHeader>
                  <PopoverBody>Description popover2</PopoverBody>
                </Popover>
            </div>
        }
    }
dev-siberia
  • 2,746
  • 2
  • 21
  • 17