Allow arrow functions in JSX props

[gilbox]

Rationale:

• Improves code readability.
• Results in simpler solutions than the alternative.

[kesne]

You should reference the eslint rule (which in this case is also react/jsx-no-bind).

[tleunen]

👎
You can always find a way to avoid this. For example passing the item and the index as props, with the onClick callback.

[ljharb]

@tleunen what about at the bottom level, where you directly create a native DOM element?

[tleunen]

What do you mean by that?

To me, it's way better to have

function ItemList(props) {
  return (
    <ul>
      {props.items.map((item, index) => (
        <Item
          key={item.key}
          item={item}
          listIndex={index}
          onItemClick={doSomethingWith}
        />
      ))}
    </ul>
  );
}

And then Item can attach the onClick event where needed and call props.doSomethingWith(props.item.name, props.listIndex)

The downside is that it has to know the list index, but well.. I think it's better than creating a new bound function every time.

[ljharb]

@tleunen ok so what does the implementation of Item look like? somewhere you are creating the <a> or <input> and that place needs to create a new function for the event handler.

[tleunen]

Sure, but you only create it once

class Item extends React.Component {
    constructor(props) {
        super(props);
        this.handleClick = this.handleClick.bind(this);
    }
    handleClick(e) {
        this.props.doSomethingWith(props.item.name, props.listIndex);
    }
    render() {
        return <button onClick={this.handleClick}>Click me</button>
    }
}

[ljharb]

Right, so the question is really whether Item should be an SFC with an arrow function in onClick, or whether it should be a class-based component like in your example.

[kesne]

To me, the arrow function seems a lot cleaner:

function Item({ doSomethingWith, item, listIndex }) {
  return <button onClick={() => doSomethingWith(item.name, listIndex)}>Click me</button>;
}

[tleunen]

I know React can apply some performance optimizations on the SFC, but would that be better than having the class without recreating the function at every render?

[ljharb]

First, as I have recently learned, the only advantage with the constructor-binding is that .bind is slow - creating new functions is not.
Second, performance should always be a secondary concern to code clarity and readability - something would have to be very slow to trump readability concerns, and I've been convinced that creating non-bound functions as event handlers (not as props, however, just as bottom-level event handlers) is not a performance hit due to the design of React.

Thus this is mostly a question of subjective readability, I think.

[tleunen]

Make sense. So creating non-bound functions is fine, but not otherwise.

[lelandrichardson]

I am in favor of allowing arrow functions in render methods. We should continue disallowing .bind(this) though

[lelandrichardson]

will we be able to do this, then: ref={foo => this.foo = foo}?

[ljharb]

only if this was set to true, as i understand it

[lelandrichardson]

if arrow fns were allows, seems like we should change that as well?

[kesne]

I agree, arrows for refs are useful when you want to use inputs in a SFC.

[tleunen]

@lelandrichardson Would this be allowed in an arrow function?
I know Babel doesn't create a bound function, but how does the browser see the arrow function with the usage of this inside?

[ljharb]

@tleunen the same as one without. it basically does a var self = this and creates a closure.

[gilbox]

Which is more readable?

(A) Foo passes the data down and Bar passes it back up in onSomething

function Foo({ data }) {
  return (
    <Bar onSomething={Actions.zoom} data={data} />
  )
}

(B) Use a closure

function Foo({ data }) {
  return (
    <Bar onSomething={() => Actions.zoom(data)} />
  )
}

The answer is (B) because you don't need to look at the code of Bar to understand what data is being passed into Actions.zoom. This is why we should usually favor using closures even for non-leaf nodes. Passing the same data down and then back up again is an anti-pattern imo.

[ljharb]

I agree that B is more readable - however with the rules as-is on master, you'd model that as a stateful component.