Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably also add ", which includes arrow functions assigned in class properties" or something similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ljharb not quite sure what you mean here. Having the arrow function assigned as a class property is the recommended approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It most certainly is not the recommended approach.
That said, it does avoid the creation of functions in the render path, so you're right that it doesn't belong here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
** recommended approach if one needs to preserve the
this
of a component, which is not always needed. Or is that otherwise? Yes, as for rendering, arrow functions that are not created inrender
won't trigger a re-render so this description should be fine.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The recommended approach is a constructor-bound instance method, so the meat of the function can be optimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ljharb what do you mean by "the meat of the function can be optimized."? Can you point me to some resource that explains this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a resource to point to; I just mean that the bulk of the code can live on a shared prototype method, that is thus invoked many times (and shared across all instances), so that can get optimized - then the only part that's "one per instance" is the bind-proxy. With the class property approach, your function (which has, say, 6 lines of code) gets recreated for every instance, so the engine can't optimize those 6 lines - since every instance has a distinct copy of those 6 lines of code.