jsx-no-lambda and passing args in event handlers

[StokeMasterJack]

If one never uses lambdas in a jsx arg, how would one pass args to an event handler? For example:

<Foo onClick={ (event) => this.foo(23) }/>

[mohsen1]

You can do onClick={this.foo.bind(this, 23)} but I would question why this is necessary. 23 should be either a prop or part of state.

[StokeMasterJack]

1. I just hard-coded 23 as an example. Sorry. I thought that was obvious.
2. Wouldn't bind produce a similar warning as it too is creating a new function with each render?
3. I believe it would trigger the "jsx-no-bind" rule.

[IllusionMH]

@StokeMasterJack we are using class components with bindings in constructor in this case. React docs on handling events has same examples (see Toggle component example). In this case function is created and bound only once on instance and not on each re-render.

Moreover in TS you can use decorators for binding (e.g. first from search) or make own implementation like Office UI Fabric React did

[StokeMasterJack]

@IllusionMH I think you missed the main thing I am asking: about passing an arg to the event handler. The arg I am talking about is row-specific and part of a list. For example, in a list of customers like this like this:

customers.map( c => <Btn onClick={ () => this.deleteCust(c.id) } /> );

The only way I know of to pass the customer id (from the current row) to the deleteCust method is to either use a lambda (like in this example) or to use bind. Both techniques cause a new function object to be created on every render.

[IllusionMH]

@StokeMasterJack rendering in a loop is really different case which is hard to guess from first post :)

1. Use // tslint:disable-next-line jsx-no-lambda comment
2. Create class component which will use bound method (will be more useful Foo is larger that one button).

IMHO Not sure that there is anything than can (or should) be corrected in this rule.

[mohsen1]

What I usually do is creating another component that accepts customer as prop.

function DeleteCostomerButton({customer, deleteCust}) {
  return <Btn onClick={() => deleteCust(customer)} />
}

that's more reacty

[StokeMasterJack]

@mohsen1 but you still have a lambda as a jsx prop. Thus you will still get the warning, right?

[IllusionMH]

@StokeMasterJack @mohsen1 as far as I know - this is only way to avoid function creation on each render in a loop, but still can be slower depending on actual structure

class DeleteByIdButton extends PureComponent {
    constructor(props) {
        super(props);
        this.handleDelete= this.handleDelete.bind(this) // remove  in case you are using decorators for binding
    }
    // @autobind  // uncomment in case you are using decorators for binding
    handleDelete() {
        this.props.onClick(this.props.id);
    }
    render() {
        return <Btn onClick={this.handleDelete}>Delete</Btn>
    }
}
// ...
users.map(u => <DeleteByIdButton id={u.id} onClick={deleteUserFunction} key={u.id}/>)

[StokeMasterJack]

Now you are creating a whole new component with each render in a loop. I'm not so convinced that this represents a best practice.

Here is my take-away (unless presented with some new information): In my opinion, using a lambda (or a bind) in a jsx property (i.e. for an event handler) is a completely valid and appropriate use-case in certain situations: particularly inside of a loop when you need to pass arguments.

Here is a StackOverflow thread on the subject: Are lambda in JSX attributes an anti-pattern?

[adidahiya]

@StokeMasterJack I agree that in practice, you sometimes have to use lambdas in JSX properties and the performance cost is negligible. Despite that, I still think this lint rule is useful because it guards against sloppy code patterns in the majority of cases. In the few cases where you need to circumvent the rule you can simply use a // tslint:disable flag.

[bezreyhan]

@StokeMasterJack I may be wrong on this, but with @IllusionMH example, you only instantiate the component once. When you use bind or lambdas you create n new lambdas on each render, n being the number of times you are looping. By creating a new component, you are also able to do further optimizations with shouldComponentUpdate.

Though, creating a new component is more boilerplate than having to write a new component class...

[johnsimons]

In ES6 you should be able to use currying, eg:

<Foo onClick={ this.foo(23) }/>

foo = (event) => (id) => {

}

[adidahiya]

@johnsimons that's simply syntactic sugar. you are still creating a new lambda every time you call this.foo.

[derrickbeining]

@StokeMasterJack (and @adidahiya , @IllusionMH , @mohsen1, @bezreyhan , @johnsimons ), I've been looking for an answer to this for a while. Found this thread while googling it (yet again). I'm not a TS user, but this is a general React concern, especially if trying to take advantage of React.PureComponent.

I've been trying to come up with a non-laborious and not-too-clever way of doing exactly what you're looking for: avoiding passing a function literal on props, particularly when rendering out components from a list of some data on state, where it almost feels impossible to avoid passing function expressions on props.

Here's the solution I've come up with (sans TS):

Instead of doing this:

class CatList extends React.Component {

  static propTypes = {
    kitties: PropTypes.arrayOf(PropTypes.instanceOf(Cat)),
  }

  render() {
    // ... other stuff ...

    // ... the problem:
    {this.props.kitties.map(kitty => {
      return <PureKittyCat key={kitty.id} onClick={ () => this.props.adoptKitty(kitty) } />

      // onClick is getting a new Function object every time CatList re-renders
      // causing PureKittyCat to re-render as well because its props have "changed".
      // This feels necessary bc we need to pass `kitty` to `this.props.adoptKitty`
    })}
  }
}

Do this:

class CatList extends React.Component {

  static propTypes = {
    kitties: PropTypes.arrayOf(PropTypes.instanceOf(Cat)),
  }

  // create a component-local cache to store instance methods you want to associate
  // with arbitrary entities on state. Use WeakMap to mitigate potential memory leaks:

  this.methodCache = new WeakMap()
 

  render() {
    // ... other stuff...

    // ... the good stuff:
    {this.props.kitties.map(kitty => {
      if ( !this.methodCache.has(kitty) ) {
        this.methodCache.set(kitty, {
          adopt: () => this.props.adoptKitty(kitty),
        })
      }
                          // as long is this is the same in-memory kitty, onClick will
                          // receive the same in-memory function object every render
                         // so prop won't change, avoiding a re-render in a PureComponent
      return <PureKittyCat key={kitty.id} onClick={this.methodCache.get(kitty).adopt} />
    })}
  }
}

I believe this is a viable solution, but if someone foresees a problem, please share how this may not work out the way I think it should. Or if anyone has a more straightforward approach, I'd love to hear alternatives. But I think this is way is pretty explicit and minimalistic.

Of course for this to work, you have to consistently follow the convention of treating all of your state immutably throughout your application. This could create some weird gotchas if you're ever mutating state/props directly.