Eslint-plugin-react: Jsx-no-bind rule

[markwain]

A bind call or arrow function in a JSX prop will create a brand new function on every single render. This is bad for performance, as it will result in the garbage collector being invoked way more than is necessary.

Can someone explain me a little more why arrow functions are bad for performance?

[markwain]

If I am not wrong render is just normal function-method. According to Ecma-spec all variables or function declaration within the function are special internal properties of the object Lexical Environment, which is created when it starts. So as I understood anyway when the function render get called all functions inside it will be re-assigned to lexical environment object. Or there is in V8 performance improve about what I don't aware of?

[ljharb]

@markwain here is my understanding: when doing DOM diffing to decide if components need to update, function props are compared with ===. If a function that's passed as a prop is created in the render path, then each time it renders, it will be considered different, and everything will need to update, which could be a huge performance hit when nothing else has changed.

The "lexical environment" of a function stops at its containing function, so in fact a function created inside render is always recreated every time render calls - without exception.

[markwain]

The "lexical environment" of a function stops at its containing function, so in fact a function created inside render is always recreated every time render calls - without exception.

Yep, that's what I meant.

So where is the difference between.
<div onClick={() => console.log('Hello!'))}></div>
and
<div onClick={ function(){ return console.log('Hello!')} }></div>
Because anyway that function is recreated every time render calls

[ljharb]

@markwain in both of those cases, a new function is indeed created. Both cases should be avoided.

[sashafklein]

Related issue -- in our app we want bind to raise errors for the above reason, but because of the huge app refactor involved in getting rid of all the fat arrow function calls that take arguments, we wanted to set the fat-arrow rule to a warning only (at least for now). So ideally we could do something like:

"react/jsx-no-bind": {
    "allowArrowFunctions": 1,
    "allowBind": 2
}

Unfortunately, it looks like the reporting level is set for the whole rule, and I can't differentiate reporting between these two. Is that correct? Any way around that?

Related question -- Am I correct that in ES6 with autobind, code like the below --

class SomeComponent extends React.Component {
  render() {
    <a onClick={ (event) => this.props.callMethod({ event, object: 'string-1' }) }>Click</a>
    <a onClick={ (event) => this.props.callMethod({ event, object: 'string-2' }) }>Click</a>
  }
}

requires an additional method to be written for each onClick event, ie:

@autobind
class SomeComponent extends React.Component {

  callMethodWithString1(event) { this.props.callMethod({event, object: 'string-1'}) }
  callMethodWithString2(event) { this.props.callMethod({event, object: 'string-2'}) }

  render() {
    <a onClick={ this.callMethodWithString1 }>Click</a>
    <a onClick={ this.callMethodWithString2 }>Click</a>
  }
}

As in, there's no way to do something like in the below pseudocode?

    <a onClick={ this.props.callMethod.returnUncalledFuncWithArg('string-1') }>Click</a>

(Obviously, the above is a search for something like () => without the negative performance effects.)

Thanks, and please let me know if you'd rather I make this a separate issue, or ask elsewhere.

[ljharb]

@sashafklein yes, you are correct - a stateless functional component can't have any functions passed as props in its render path unless they're passed through untouched from their own props. The proper thing to do is your class-based example (including the autobind)

[markwain]

@ljharb

in both of those cases, a new function is indeed created. Both cases should be avoided.

So it seems logically that jsx-no-bind rule has to be changed because there is no sense to warn only about arrow function. I think there should be new rule for warning about possibility of performance regarding issues related to function which has been declared in render.

[ljharb]

@markwain it warns about bind and arrows (which is an implicit bind) - i agree tho perhaps it makes more sense to create a new rule jsx-no-new-function-props-in-render that subsumes this rule, and includes creation of any functions in the render path.

[markwain]

@ljharb yep, exactly
@yannickcr Can it be implemented?

[yannickcr]

Unfortunately, it looks like the reporting level is set for the whole rule, and I can't differentiate reporting between these two. Is that correct? Any way around that?

The error level is managed directly by ESLint, the plugin cannot do anything about it. The only thing that can be done for case like that is to split the rule in two.

i agree tho perhaps it makes more sense to create a new rule jsx-no-new-function-props-in-render that subsumes this rule, and includes creation of any functions in the render path.

Yes, it would be more clear and catch more edge cases than the current rule.

[markwain]

@yannickcr

Yes, it would be more clear and catch more edge cases than the current rule.

So does it mean, that you will make this new rule?

[tcoopman]

Is there some kind of benchmark that shows that recreating the function is bad for performance?

[linclark]

i agree tho perhaps it makes more sense to create a new rule jsx-no-new-function-props-in-render that subsumes this rule, and includes creation of any functions in the render path.

I need this for work so could take it on. But before I start, is anyone else currently working on it?

@tcoopman You can read more about this problem in React.js pure render performance anti-pattern under _Functions create identities too_.

[mjomble]

The documentation for this rule mentions that creating a new function on each render is bad for performance, but later recommends code like this:

var List = React.createClass({
  render() {
    return (
      <ul>
        {this.props.items.map(item =>
          <ListItem key={item.id} item={item} onItemClick={this.props.onItemClick} />
        )}
      </ul>
    );
  }
});

This example still contains an arrow function inside render.
Isn't this just as bad? At least as far as the garbage collection argument is concerned.

[ljharb]

@mjomble .bind on render is very bad for performance. creating new functions in render is negligibly bad for performance.

[mjomble]

This should probably be mentioned in the documentation. Currently it gives the impression that they are more or less equally bad.

[langri-sha]

What about stateless functional components? Seems it would be innocuous to create and assign an anonymous function to a prop.

[ljharb]

@langri-sha the same advice applies to SFCs. If you need a function, you're best served using a class-based component and a constructor-bound instance method.

[langri-sha]

@ljharb sorry if I'm dragging you back to this, just wanted to confirm things 🙈.

const Foo = ({id, onClick}) => <input type={'button'} onClick={() => onClick(id)} />

So, hypothetically speaking, it's likely that after the component is mounted only the id property will change, in which case I would be re-creating the anonymous function and this is something I can avoid with creating a CBC?

[ljharb]

@langri-sha I agree in your case, that the only way the SFC will be rerendered is if id or onClick changes, and in a class component it'd be identically efficient - so in your case, an SFC is totally fine. However, it always depends on the exact component implementation, so as a general rule, it's better to avoid creating functions inside the render path.

[dounan]

At Flexport we ran into performance issues with inline functions causing our pure components to wastefully re-render. Instead of forcing developers to not use arrow functions, we created new library, reflective-bind, that lets you keep your inline functions without wasteful re-renders. For one of our more complex forms, using reflective-bind reduced the wasted render time from 175ms to 18ms. 🎉

You can read more about it here:
https://flexport.engineering/ending-the-debate-on-inline-functions-in-react-8c03fabd144