Binding, ES6 classes, and onChange

[skevy]

When using ES6 classes, autobinding doesn't happen, as we all hopefully know by now :).

So when I do:

<button onClick={this.handleClick}>Test Button</button>

I'd expect that "this" inside of my handleClick function in my class is undefined. This is, in fact, what happens.

However, when I do:

<input onChange={this.handleChange} type="checkbox" />

..."this" inside of my handleChange function is the input component's constructor.

Obviously, if I just bind my functions all the time, the problem is resolved. But since "this.setState" exists on the constructor...I was calling this.setState inside my handleChange function, not getting an error, and nothing was happening. So it was confusing to track down.

Admittedly I'm not immediately sure what to do about this...but I thought I'd make an issue and open it up for discussion.

[zpao]

This sounds like perhaps we should be explicitly calling onChange with undefined in our controlled component implementations.

[sophiebits]

Yup, agreed.

[skevy]

So just in ReactDOMInput (and wherever else it may appear)? I had seen this, I just wanted to make sure from you guys who know the code backwards and forwards that onChange wasn't being called with "this" for some other reason.

[sophiebits]

Yeah, ReactDOMInput, ReactDOMSelect, ReactDOMTextarea, and maybe LinkedValueUtils too (just off the top of my head).

[skevy]

Cool. I'll work on this. Thanks.

[tuespetre]

Thought I was going crazy because there was no error, until I checked the value of 'this'. I'll be watching so I can remove the .bind(this) from all of the onChange attributes 👍

[Zenwolf]

I am also experiencing strange behavior with input onChange after converting to ES6 class that extends React.Component. I tried binding the handler functions in the constructor, as well as in the render() method but the onChange={ this.myHandler } is never even called at all. It worked fine when not using ES6 classes. I've been debugging it for a day, but cannot find the root problem. I have also noticed some other mouse click events are no longer happening in some scenarios as well after converting to classes, but I have mainly focused on the onChange debugging so far, with no luck. There are no errors thrown or warnings, just that the handler functions are never even called.

[rsylvian]

@Zenwolf you still need to .bind(this) or you can use the experimental ES7 function's binding operator ::

onChange={::this.handleChange}

[Zenwolf]

@MyBoon In the component's constructor, I bound the function to the component instance:

constructor(props) {
    super(props);
    this.handleChange = this.handleChange.bind(this);
}

After doing that, the handler was still never called.

[Josh-a-e]

Has any progress been made on this @skevy? I was considering picking it up but can look for something else if you're making progress on it

[Josh-a-e]

So I wrote a test that I was expecting to fail based on the description:

  it('should have a this value of undefined if bind is not used', function() {
    var unboundInputOnChange = function() {
      expect(this).toBe(undefined);
    }

    var instance = <input type="text" onChange={unboundInputOnChange} />;
    instance = ReactTestUtils.renderIntoDocument(instance);

    ReactTestUtils.Simulate.change(ReactDOM.findDOMNode(instance));
  });

But pleasantly it passes - has this been fixed incidentally or have I missed something?

[sophiebits]

It looks like I accidentally fixed this in 52a229f. Want to send a pull request with your test to make sure we don't regress? (You don't need the findDOMNode call at all.)