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

Remove debatable statement on perf in jsx-no-bind #2067

Merged
merged 1 commit into from Dec 2, 2018

Conversation

rickhanlonii
Copy link
Contributor

Overview

The PR removes a controversial and debatable line that is the source of much confusion

See:

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, i think this is a good change. Nobody should be thinking about memory management in a memory managed language anyways :-)

@@ -1,6 +1,6 @@
# No `.bind()` or Arrow Functions in JSX Props (react/jsx-no-bind)

A `bind` call or [arrow function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions) 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. It may also cause unnecessary re-renders if a brand new function is passed as a prop to a component that uses reference equality check on the prop to determine if it should update.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There’s a minor performance issue that applies only to functions, which is that the same function object isn’t optimized until it’s been called multiple times - so if you create a new one on every render, you’re resetting the progress on optimizing that function.

@ljharb ljharb merged commit 499707a into jsx-eslint:master Dec 2, 2018
@rickhanlonii rickhanlonii deleted the patch-1 branch December 2, 2018 16:19
@rickhanlonii
Copy link
Contributor Author

Thanks @ljharb!

This was referenced Dec 28, 2018
This was referenced Jan 4, 2019
@ghost ghost mentioned this pull request Jan 12, 2019
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants