Skip to content
This repository has been archived by the owner on Jul 19, 2019. It is now read-only.

Event handlers complain when attaching component method #37

Open
RGBboy opened this issue Nov 27, 2014 · 14 comments
Open

Event handlers complain when attaching component method #37

RGBboy opened this issue Nov 27, 2014 · 14 comments

Comments

@RGBboy
Copy link

RGBboy commented Nov 27, 2014

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?

@dmitrif
Copy link

dmitrif commented Mar 3, 2015

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

@ThomWright
Copy link
Contributor

@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)} />

@sophiebits
Copy link
Member

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

@ThomWright
Copy link
Contributor

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.

@sophiebits
Copy link
Member

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.

@ThomWright
Copy link
Contributor

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

@luodaxu
Copy link

luodaxu commented Jul 7, 2015

try use null instead of this

@ConAntonakos
Copy link

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?

@sophiebits
Copy link
Member

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.

@ConAntonakos
Copy link

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.

@joshma
Copy link

joshma commented Aug 18, 2015

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

@ThomWright
Copy link
Contributor

@joshma Nothing better than my makeListener hack above.

@tara1128
Copy link

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

@vaskort
Copy link

vaskort commented Dec 29, 2016

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants