facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.13k stars 46.3k forks source link

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

Closed lei-clearsky closed 7 years ago

lei-clearsky commented 7 years ago

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 commented 7 years ago

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!

aweary commented 7 years ago

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](http://babeljs.io/repl/#?babili=false&evaluate=false&lineWrap=false&presets=es2015%2Creact%2Cstage-2&targets=&browsers=&builtIns=false&debug=false&code=%2F%2F%20Declare%20a%20method%20as%20an%20instance%20property%0Aclass%20Foo%20%7B%0A%20%20foo%20%3D%20()%20%3D%3E%20console.log('foo')%0A%7D%0A%0A%0A%2F%2F%20Binding%20a%20method%20as%20an%20instance%20property%0Aclass%20Bar%20%7B%0A%20%20constructor()%20%7B%0A%20%20%20%20this.bar%20%3D%20this.bar.bind(this)%3B%0A%20%20%7D%0A%20%20%0A%20%20bar()%20%7B%0A%20%20%20%20console.log('bar')%3B%0A%20%20%7D%0A%7D&experimental=false&loose=false&spec=false&playground=true) to see the difference.

lei-clearsky commented 7 years ago

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: https://github.com/airbnb/enzyme/issues/365

lei-clearsky commented 7 years ago

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

gaearon commented 7 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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

TrySound commented 6 years ago

Do you mean using refs for testing?

ljharb commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

dienluong commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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 commented 5 years ago

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 commented 5 years ago

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.

i-oliva commented 5 years ago

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:

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.

nathanhfoster commented 5 years ago

@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 commented 5 years ago

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

nathanhfoster commented 5 years ago

Thanks for the correction @ljharb!

Sin9k commented 4 years ago

Hello guys, a lot of colleagues asked me to clarify them the difference between classic method and arrow function method in ES6 class and I created a short video where i explain the differences. May be it will be usefull for you too:

EN: https://youtu.be/19teBd3lrpk RU: https://youtu.be/sJ4zRZ4fOdc