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

[Question] Use arrow functions or bind manually in es6 classes? Any performance difference? #9851

Closed
lei-clearsky opened this issue Jun 5, 2017 · 28 comments

Comments

@lei-clearsky
Copy link

@lei-clearsky lei-clearsky commented Jun 5, 2017

In terms of performance, is there any difference between using arrow functions and bind manually when using es6 classes? Using arrow functions the methods are not on the class prototype, it will be on the class instance only. Using bind will attach the methods to class prototype. It sounds like bind manually will have better performance, does that mean we should consider using bind instead of arrow functions for class methods?

Any suggestions or comments are really appreciated!

So in terms of performance, would you recommend using

class MyComponent extends React.Component {
  constructor(props) {
    super(props)
  }

  methodA = () => { ... }
}

or

class MyComponent extends React.Component {
  constructor(props) {
    super(props)
    this.methodA = this.methodA.bind(this)
  }

  methodA() { ... }
}
@gaearon

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Jun 5, 2017

Hi!

These two ways of writing it are equivalent. (The second one is compiled to the first one.)

Using bind will attach the methods to class prototype.

In your example, you still attach the function to the instance:

this.methodA = this.methodA.bind(this)

So they’re essentially the same.

At Facebook, we use the second way (“class properties”) but be aware this is still experimental, and not part of ES6. If you only want to stick with stable syntax, then you could bind them manually.

I hope this helps!

@gaearon gaearon closed this Jun 5, 2017
@aweary

This comment has been minimized.

Copy link
Member

@aweary aweary commented Jun 5, 2017

As far as runtime behavior goes they are identical, but doing the later (binding a method in the constructor) means you're defining the method on the class prototype and the instance.

Doing the former will only define the method as an instance property, avoiding the duplication. But as mentioned, it's experimental so YMMV. Check out this example in the Babel REPL to see the difference.

@lei-clearsky

This comment has been minimized.

Copy link
Author

@lei-clearsky lei-clearsky commented Jun 5, 2017

Thanks @gaearon this is very helpful!

This question was coming from when testing component methods, as I can find the method on class prototype if I use this.methodA = this.methodA.bind(this), but can't find the method on prototype if I use methodA = () => { ... }. See this issue on enzyme: airbnb/enzyme#365

@lei-clearsky

This comment has been minimized.

Copy link
Author

@lei-clearsky lei-clearsky commented Jun 5, 2017

@aweary thanks! this helps a lot as we are in the process of converting React.createClass to es6 classes.

@gaearon

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Jun 5, 2017

@lei-clearsky

Also check out the automatic class transform:

jscodeshift -t ./transforms/class.js --mixin-module-name=react-addons-pure-render-mixin --flow=true --pure-component=true --remove-runtime-proptypes=false <path>

We used it to convert thousands of components.

It's documented here: https://github.com/reactjs/react-codemod#explanation-of-the-new-es2015-class-transform-with-property-initializers

@lucasconstantino

This comment has been minimized.

Copy link

@lucasconstantino lucasconstantino commented Jan 31, 2018

I've just read an article which seems to disagree with the conclusions on this issue. Do you guys mind to give an opinion on that? Here it goes:

https://medium.com/@charpeni/arrow-functions-in-class-properties-might-not-be-as-great-as-we-think-3b3551c440b1

@ljharb

This comment has been minimized.

Copy link
Contributor

@ljharb ljharb commented Jan 31, 2018

A best practice, imo, is to never use an arrow function in a class property; it harms testability and limits what the engine can optimize.

@creativetim

This comment has been minimized.

Copy link

@creativetim creativetim commented Feb 9, 2018

re: @lucasconstantino, would love to hear opinions from FB about that article in addition to how they go about testing those kinds of methods or perhaps deciding not to test them.

@TrySound

This comment has been minimized.

Copy link
Contributor

@TrySound TrySound commented Feb 9, 2018

@creativetim Component methods are usually implementation detail, so they shouldn't be tested. props/callbacks is the only interface you should use most of the time.

If you use some methods as part of public api, this api should be used only through ref. In this case you should also test those methods only though ref. So bind, arrow or even inlined functions shouldn't care you.

@ljharb

This comment has been minimized.

Copy link
Contributor

@ljharb ljharb commented Feb 9, 2018

I don’t agree with that; using refs from outside a component to reach into it is a bad practice.

@TrySound

This comment has been minimized.

Copy link
Contributor

@TrySound TrySound commented Feb 9, 2018

Do you mean using refs for testing?

@ljharb

This comment has been minimized.

Copy link
Contributor

@ljharb ljharb commented Feb 9, 2018

I mean that unless it’s an explicit part of a component’s API, I’d suggest not trying to get access to a component’s internal implementation. I’m not sure why it would be needed for testing; enzyme can be used to get any node you want without refs.

@TrySound

This comment has been minimized.

Copy link
Contributor

@TrySound TrySound commented Feb 9, 2018

Yeah, I meant the same. It's about cases when you can't describe events as state. For example react-virtualized scrollTo* props are shot in the foot. We are gonna change it to initial values + instance methods.

@caub

This comment has been minimized.

Copy link

@caub caub commented Mar 7, 2018

The performance difference between bind and arrows is not big (50% max) in his jsperf or a remake of it

you can also have a variant of PureComponent as a HOC to ignore all or some functions in shouldComponentUpdate to skip more re-renders

But I agree that all methods that can go on prototype (without even bind), should, since it saves some significant performance there

@lucasconstantino

This comment has been minimized.

Copy link

@lucasconstantino lucasconstantino commented Mar 7, 2018

Another related post: https://flexport.engineering/ending-the-debate-on-inline-functions-in-react-8c03fabd144 (Ending the debate on inline functions in React)

@dienluong

This comment has been minimized.

Copy link

@dienluong dienluong commented Jun 17, 2018

@lucasconstantino
I read that article on Medium by Nicolas Charpentier. I think he has a point BUT not in the context of React because, as @gaearon mentionned, you will bind() the method in the constructor anyway, which ultimately creates a copy of the function for every instance, just like the other way using arrow functions does. So the arguments presented in that article crumble in the context of React. My 2 cents.

@ljharb

This comment has been minimized.

Copy link
Contributor

@ljharb ljharb commented Jun 17, 2018

@dienluong it doesn't create a copy of the function, it creates a shallow "bind proxy" to the meat of the function, which is shared on the prototype, and thus much more optimizeable. So, in fact, constructor-binding is much better and does NOT crumble in any context in JavaScript, React or otherwise.

@dienluong

This comment has been minimized.

Copy link

@dienluong dienluong commented Jun 19, 2018

@ljharb
Thanks for the input. I stand corrected about the copy (it's not an exact copy the function).

However, I did not find any source demonstrating that constructor-binding offer much better performance (compared to class property syntax). In fact, the official React documentation does recommend both constructor-binding and the 'class property' syntax, with the caveat that the latter is still experimental.

(Note that my point wasn't that constructor-binding was a bad approach; my point was that the arguments put forth in that Medium article, which by the way disagrees with the accepted conclusion in this thread (i.e. that both methods are essentially the same), might be questionable.)

@ljharb

This comment has been minimized.

Copy link
Contributor

@ljharb ljharb commented Jun 19, 2018

Another concern with them is testability - with prototype methods, you can mock them out prior to creating your enzyme wrapper (or any other form of creating elements and testing them) - with arrows in class properties, you have to create the element, then get at the instance, then mock out the own property, and then force a rerender.

@paddotk

This comment has been minimized.

Copy link

@paddotk paddotk commented Sep 27, 2018

For convenience, I'd like to add that when not being able to access the this scope with arrow functions, you can simply prefix with an underscore. So when you're at a breakpoint and the console returns undefined or some 'can't find' error, changing this.props to _this.props for example will do the trick.
That said, it's not exacly ideal of course.

@ljharb

This comment has been minimized.

Copy link
Contributor

@ljharb ljharb commented Sep 27, 2018

You’re referring, i believe, to babel’s output there, and to the chrome debugger’s inability to properly handle source maps in the repl - which is unrelated to this issue.

@pavilion

This comment has been minimized.

Copy link

@pavilion pavilion commented Jan 30, 2019

Hi @gaearon ! Our team have an ongoing debate about what should we actually use, either binding methods to the class prototype or using arrow functions.

We are having this debate because we have this circumstances:

  • Almost all of our components/containers are written using arrow functions.
  • We have just started now testing them, using Enzyme and Jest.
  • @ljharb, enzyme's actively maintainer, suggests that we should bind methods to the class prototype in order to be able to do a more efficient testing of this methods that have been bound. In some of his posts, he comments that in Airbnb arrow functions are forbidden to be used with class properties.
  • We would have to refactor all of our already written containers/components to use methods bound to the class in order to test them instead of doing either update() or forceUpdate().

May I ask what is your opinion about this, since your team actually suggests using Enzyme for testing? How do you guys deal with this kind of tests at Facebook? :)

@1978milanbabic

This comment has been minimized.

Copy link

@1978milanbabic 1978milanbabic commented Feb 16, 2019

ES5:
`

function Foo() {
    this.prop1 = "";
    this.method1 = function () {return "bar"};
}

Foo.prototype.protoMethod2 = function () {return "bar"};

`

ES6:
`

class Foo {
    constructor() {
        this.prop1 = "";
        this.method1 = () => {return "bar"};
    }

    protoMethod2() {
        return "proto bar";
    }
}

`

*(ES6 + babel): ("initiated as variables-functions" are converted into main props and methods, while "directly named" functions are prototypes)
`

class Foo {
    prop1 = "";
    method1 = () => {return "bar"};
    protoMethod2() {return "bar}
}

`
****but, you can not bind in (ES6 + babel) directly to this => you must "get back" one step to es6 and use constructor like this:

`

class Foo {
    constructor() {
        this.anymethod = this.anymethod.bind(this);
    }
}

`

@ljharb

This comment has been minimized.

Copy link
Contributor

@ljharb ljharb commented Feb 16, 2019

That’s not ES7; ES7 is ES2016, and class fields are stage 3 (meaning they aren’t even in ES2019, and aren’t certain to be in ES2020 either)

@1978milanbabic

This comment has been minimized.

Copy link

@1978milanbabic 1978milanbabic commented Feb 16, 2019

Sorry, thats babel + es6 - but in plan to be in some "new" es. - Hoppe now I'm right? @ljharb
I'll just edit my mistake in previous post.

@strap8

This comment has been minimized.

Copy link

@strap8 strap8 commented Aug 7, 2019

@lucasconstantino in the comment section of the article you linked this insight was shared.
Arrow functions vs manual binding in the constructor are synonymous.

@ljharb

This comment has been minimized.

Copy link
Contributor

@ljharb ljharb commented Aug 8, 2019

Not quite; manual binding leaves the method on the prototype for sharing and testing, arrows do not.

@strap8

This comment has been minimized.

Copy link

@strap8 strap8 commented Aug 8, 2019

Thanks for the correction @ljharb!

moshfeu referenced this issue in olawwska/Eco-finder Aug 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.