No .bind() or Arrow Functions in JSX Props making no sense to me when I need to pass arguments

Question

Having a look at the eslint rule, No .bind() or Arrow Functions in JSX Props.

It says not to use, arrow functions or bind:

<div onClick={this._handleClick.bind(this)}></div>
<div onClick={() => console.log('Hello!'))}></div>

But rather use:

<div onClick={this._handleClick}></div>

This is all well and good but how do I pass arguments to this function on the click event?

Here is my naive code example:

export class UserList extends Component {
  constructor(props) {
    super(props);
    this.handleToggleActive = this.handleToggleActive.bind(this);
  }
  handleToggleActive() {
    console.log(arguments);
  }
  render() {
    return (
      <ul className="user-list">
        {this
          .props
          .users
          .map(user => (
            <li key={user.id}>
              <Link to={`/users/${user.id}`}>{user.name}</Link>
              <Button onClick={this.handleToggleActive}>Toggle Active</Button>
            </li>
          ))}
      </ul>
    );
  }
}

How do I get the user.id to my handleToggleActive function? when I try something like:

<Button onClick={this.handleToggleActive(user.id)}>Toggle Active</Button>

It is executed when rendering, and the only other way I can see to do it is to use an arrow function?

What is the correct way to do this?

There's a pretty decent discussion here https://github.com/airbnb/javascript/issues/659 — patrick, Feb 14 '17 at 03:24

score 2 · Accepted Answer · answered Feb 14 '17 at 09:58

2

Either using bind and arrow functions, will create a new function passing a completely new prop to your component, causing unnecessary rerenders and extra work for the garbage collector, using data-attributes is a bad design and is not the React way of doing things, you should not access the DOM directly, but rather let React handle things. Instead of that you can just make a new component and pass the the handler as a prop, handling the "binding" of the id in a method of the component:

class User extends Component {
  handleClick() {
    this.props.handleToggleActive(this.props.user.id);
  }

  render() {
    return (
       <li>
         <Link to={`/users/${this.props.user.id}`}>{this.props.user.name}</Link>
         <Button onClick={this.handleClick}>Toggle Active</Button>
      </li>
    )
  } 
}

export class UserList extends Component {
  constructor(props) {
    super(props);
    this.handleToggleActive = this.handleToggleActive.bind(this);
  }
  handleToggleActive(userId) {
    console.log(userId);
  }
  render() {
    return (
      <ul className="user-list">
        {this
          .props
          .users
          .map(user => (
            <User key={user.id} user={user} handleToggleActive={this.handleToggleActive} />
          ))}
      </ul>
    );
  }
}

answered Feb 14 '17 at 09:58

Teo

614
1
4
13

There is such a thing as overoptimization. Making a handler be a function which returns a new function can't possibly cause enough damage to efficiency that it justifies creating an entirely new component. It seems like over-engineering for a process which may or may not ever affect the life of the application. It's definitely good to point out the downsides of this method, but both solutions, I think, are valid. I completely agree with you on the data attributes one though. – ZekeDroid Feb 15 '17 at 14:12
@ZekeDroid I don't know how to put this, but can you please provide actual arguments for your statements, maybe you should get your facts straight before downvoting the only correct answer ? I would suggest you to make a small proof of concept using the componentWillUpdate lifecycle method, maybe then you will realise that avoiding the eslint rule and respecting it are two different things. This eslint rule was created for a good reason, do you think that creating completely new functions and passing those at each render step is a good idea, sure go ahead, this should be fine for a simple app – Teo Feb 15 '17 at 15:35
where this rule actually exceeds is in large apps that receive lots of updates, a render bind or passing a completely new function in render in this cases will result in actual performance issues. – Teo Feb 15 '17 at 15:36
Actually from a structural point of view in this specific case it makes a lot more sense to use a separated User component, instead of plain markup in the map function, this is not "over-engineering" or "over-optimization". – Teo Feb 15 '17 at 15:38
Didn't mean to upset you! Just meant as constructive criticism, and wow, did not mean to click downvote on yours at all, I was literally looking to downvote the data one since that one does go against React principles. But I've found downvoting has a very negative impact in SO so I'm going to refrain from using it here either way and will try to remove your downvote. I'm not sure what you mean but "actual arguments"? I thought it was clear what I was referring to. The trade off between an entirely new component vs sacrificing creating a new function on render – ZekeDroid Feb 15 '17 at 15:39
@ZekeDroid, you have not upset me, can you please give an example of what are the trade-offs of creating a entirely new component ? – Teo Feb 15 '17 at 15:43
Yeah, it's getting into a conversation but I work in high churn applications and we tend to make the trade off of potential inefficiencies vs more code because more code means more to test, more to document, and more prone to bugs. Don't get me wrong, we make tons of components and love expanding like that! But we rarely flinch at making click handlers be functions that return functions. – ZekeDroid Feb 15 '17 at 15:49
After using @ZekeDroid method initially which works well, I wen't away and refactored ending up with @Teo method (prior to seeing his answer) - from my newly formed understanding of React this is the `React` way of doing this correctly, and it separates the code well.Thanks for the discussion, I've learnt a lot. – shenku Feb 15 '17 at 23:27

score 0 · Answer 2 · answered Feb 14 '17 at 03:36

I guess you can use data-* attributes.

export class UserList extends Component {
    constructor(props) {
        super(props);
        this.handleToggleActive = this.handleToggleActive.bind(this);
    }
    handleToggleActive(event) {
        console.log(event.target.dataset.id);
    }
    render() {
        return (
            <ul className="user-list">
                {this
                .props
                .users
                .map(user => (
                    <li key={user.id}>
                    <Link to={`/users/${user.id}`}>{user.name}</Link>
                    <Button onClick={this.handleToggleActive} data-id={user.id}>Toggle Active</Button>
                    </li>
                ))}
            </ul>
        );
    }
}

using data- attributes is a bad design, reminding me of the jquery days, it does not represent the React way of doing things, rather than accessing the DOM yourself you should let React handle things for you. — Teo, Feb 14 '17 at 09:59

score 0 · Answer 3 · answered Feb 14 '17 at 14:24

In order to address the performance concern (which is why the rule exists), what about using memoization?

For example, with the package lru-memoize you get (with a dumb component) something like:

var memoize100 = require('lru-memoize')(100);

let handleToggleActive = (user) => () => {
    console.log(user.id)
}
handleToggleActive = memoize100(handleToggleActive)

const UserList = (props) =>
  <ul className="user-list">
  { props.users
      .map(user => (
        <li key={user.id}>
          <Link to={`/users/${user.id}`}>{user.name}</Link>
          <Button onClick={handleToggleActive(user)}>Toggle Active</Button>
        </li>
      ))
    }
  </ul>

score 0 · Answer 4 · answered Jun 06 '18 at 12:04

Using arrow function or bind will create a different callback instance everytime the component is rendered. Thus, the function is recreated every time, and the old one is garbage collected, having an overload on the garbage collector. Also, due to this the pure components and the components with shallow compare in shouldcomponentupdate will always rerender as it will find changes in its props because the old function is discarded and a new function is created every time the parent component is rendered.

We can use the property initializer syntax with currying to avoid using bind or arrow function inside the JSX code.

handleClick = (param) => (e) => {
  console.log('Event', e);
  console.log('Parameter', param);
}

render() {
  <button onClick={this.handleClick('Parameter')}></button>
}

No .bind() or Arrow Functions in JSX Props making no sense to me when I need to pass arguments

4 Answers4

Linked