Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add jsx-no-bind rule to config #14

Open
kadamwhite opened this issue Aug 18, 2017 · 5 comments
Open

Add jsx-no-bind rule to config #14

kadamwhite opened this issue Aug 18, 2017 · 5 comments

Comments

@kadamwhite
Copy link
Contributor

For React rendering performance we should consider adding jsx-no-bind to this eslint config; bound functions and arrow functions created within a render method's JSX will never be treated as equivalent when React is comparing subcomponent properties and can lead to a lot of unnecessary (and therefore expensive) re-renders.

@rmccue
Copy link
Member

rmccue commented Aug 20, 2017

The only thing I'd want to see here is that it does actually make a difference. In my experience, using pre-bound functions over arrow functions hasn't had any sort of significant or measurable effect on performance, compared to e.g. using PureComponent religiously.

It also strongly depends on whether create-react-app includes the property initialiser feature, since the generally recommended alternative approach is to bind there:

class Component extends React.Component {
    onClick = e => {
        e.preventDefault();
        this.props.onClick( e );
    }
}

Ergonomically, I still think arrow functions are the nicest, but property initialisers as about as good.

@rmccue
Copy link
Member

rmccue commented Aug 20, 2017

It also strongly depends on whether create-react-app includes the property initialiser feature

It seems like it does support it:

In addition to ES6 syntax features, it also supports:

@pdewouters
Copy link
Contributor

Why not add it if it can help avoid unnecessary re-renders in child components?

The problem with this syntax is that a different callback is created each time the LoggingButton renders. In most cases, this is fine. However, if this callback is passed as a prop to lower components, those components might do an extra re-rendering. We generally recommend binding in the constructor or using the class fields syntax, to avoid this sort of performance problem.

@rmccue
Copy link
Member

rmccue commented Oct 17, 2018

Why not add it if it can help avoid unnecessary re-renders in child components?

Generally speaking, it's an unnecessary optimisation, so you just don't need to do it. Using property initialisers is cognitive overhead for negligible gain most of the time.

@kadamwhite
Copy link
Contributor Author

Property initializer is more widely available now, and also more widespread, we can ban .bind. The main place it gets used is when the dev is unaware that property initializers would be a better solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants