react-art icon indicating copy to clipboard operation
react-art copied to clipboard

Event handlers complain when attaching component method

Open RGBboy opened this issue 10 years ago • 14 comments

I have added an onClick handler to the group component but I am getting this warning in my console:

bind(): React component methods may only be bound to the component instance. See Component

Here is a shortened version of my component which exhibits the warning. (Plain JS, no JSX)

var Component = React.createClass({
  displayName: 'Component',
  handleClick: function (event) {
    console.log('Clicked!');
  },
  render: function () {
    return (
      Surface({ width: 320, height: 480 },
        Group({ onClick: this.handleClick })
      )
    );
  }
});

I believe it has to do with the fact that Group is a React component and that it should only be binding its own methods to the event listener. Is there a way to put this together without getting this warning?

RGBboy avatar Nov 27 '14 10:11 RGBboy

Any react component change handler will be called much like this.handleClick.bind(this) or this.handleClick.call(this). Therefore, the easiest way to get around the message is bind a null or the component on which the handler is set for the scope of the handler like:

Group({ onClick: this.handleClick.bind(this) })

dmitrif avatar Mar 03 '15 19:03 dmitrif

@dmitrif Trying that gives me this: Warning: bind(): You are binding a component method to the component. React does this for you automatically in a high-performance way, so you can safely remove this call. See exports

I think the problem comes from ReactART.js line 293. How come listeners are bound to this?

My (quick and hacky) solution takes advantage of ReactART.js line 295:

var makeListener = function(f) {
  return {
    handleEvent: f
  };
};
...
<Group onClick={makeListener(this.myClickHandler)} />

ThomWright avatar May 13 '15 22:05 ThomWright

@ThomWright That .call shouldn't cause the warning. Not sure off the top of my head what causes it; probably something in art internals.

sophiebits avatar May 13 '15 22:05 sophiebits

Really? Maybe I'm missing something (not unlikely!) but the warning says React component methods may only be bound to the component instance.

In this case, the .call() is trying to bind a component method (myClickHandler in my case) to something else.

ThomWright avatar May 13 '15 22:05 ThomWright

If you look at the implementation of that warning in the React repo you'll see that it intercepts .bind calls, not .call. It's mostly meant as a warning to prevent people from binding unnecessarily and creating extra garbage.

sophiebits avatar May 13 '15 22:05 sophiebits

Ah, gotcha. Having a quick look at the ART internals it does indeed look like it's calling .bind on listeners.

ThomWright avatar May 13 '15 22:05 ThomWright

try use null instead of this

luodaxu avatar Jul 07 '15 08:07 luodaxu

So, when you .bind(null, ...), the event parameter is undefined. But I need it as a parameter so that Firefox doesn't spit errors. Any ideas?

ConAntonakos avatar Jul 14 '15 16:07 ConAntonakos

So, when you .bind(null, ...), the event parameter is undefined.

That doesn't sound right. The first arg is used as context (i.e., this), not as the first arg of the called function.

sophiebits avatar Jul 15 '15 13:07 sophiebits

Thanks, @spicyj. I cleared up my misunderstanding. In Chrome, I was realizing I didn't have to explicitly define the event parameter of the called func in a React component. Yet in Firefox, I wasn't given that flexibility.

ConAntonakos avatar Jul 15 '15 22:07 ConAntonakos

@ThomWright were you ever able to resolve the bind warning? We're on React 0.12 and it looks the same - the source of the warning comes from https://github.com/sebmarkbage/art/blob/master/dom/dummy.js#L100 - it's binding a function which, in this case, is a React component's function.

joshma avatar Aug 18 '15 06:08 joshma

@joshma Nothing better than my makeListener hack above.

ThomWright avatar Aug 18 '15 13:08 ThomWright

You can cache the React component instance into a variable, instead of using "this", because "this" means the DOM element itself in some cases.

Like this:

render: function() { var component = this; return ( ......... onClick={component.handleClick.bind(component)} .... ) }

It works in my project :-)

tara1128 avatar Apr 26 '16 07:04 tara1128

I had this error when my function call was like this: onPress={ this.props.onAddLocation(markerLocation) } but changed it to: onPress={ this.props.onAddLocation.bind(this, markerLocation) }

vaskort avatar Dec 29 '16 14:12 vaskort