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

[ariddell]

I'm still encountering this with React 0.14.

[sophiebits]

The bug here was that inputs weren't firing with this as undefined. ES6 classes do not autobind, nor will they. Your options are to bind manually in render, add this.foo = this.foo.bind(this); to the constructor, or use the experimental ES7 property initializer syntax which you need to enable manually in Babel:

class Foo extends React.Component {
  handleClick = (event) => {
  }

  render() {}
}

[Josh-a-e]

@spicyj certainly! #5109

[ariddell]

@spicyj Thanks. I missed the discussion of (not) autobinding in the React v0.13 release notes: https://facebook.github.io/react/blog/2015/01/27/react-v0.13.0-beta-1.html#autobinding

[ghost]

Alternatively, you may use the @autobind Decorator pattern from ES7 Stage 1 on the function using the autobind-decorator. Unlike function binding approach suggested by @MyBoon use of the decorator does not require use of the experimental Strawman (Stage 0) proposal.

The downside to using autobind-decorator is that it's function-level only, so you may end up using it several times to decorate class methods as more of them require use of the class' this context.

It seems to me the most favorable approach would be to use the class-level @autobind found in core-decorators if anything.