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

rickhanlonii commented Dec 2, 2018

Overview

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

See:

@ljharb
ljharb approved these changes Dec 2, 2018
Copy link
Collaborator

ljharb left a comment

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.

This comment has been minimized.

Copy link
@ljharb

ljharb Dec 2, 2018

Collaborator

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 added the documentation label Dec 2, 2018
@ljharb ljharb merged commit 499707a into yannickcr:master Dec 2, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 97.45%
Details
@rickhanlonii rickhanlonii deleted the rickhanlonii:patch-1 branch Dec 2, 2018
@rickhanlonii

This comment has been minimized.

Copy link
Contributor Author

rickhanlonii commented Dec 2, 2018

Thanks @ljharb!

@renovate renovate bot mentioned this pull request Dec 28, 2018
0 of 1 task complete
@renovate-bot renovate-bot mentioned this pull request Dec 28, 2018
0 of 1 task complete
This was referenced Dec 28, 2018
This was referenced Jan 4, 2019
@ibf-renovate ibf-renovate mentioned this pull request Jan 12, 2019
0 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.