Update ES6 class documentation with binding perf

[mfunkie]

Adding a note in the ES6 class documentation about function binding. Recommending that you bind your handlers in the constructor so that they are referentially the same function every time render is invoked (helps with child components that might potentially call shouldComponentUpdate).

[gaearon]

It is a bit inconsistent that just before this paragraph we bind in the render() method. Maybe we can change that example instead of introducing the new one, and just repeat the constructor in this section instead?

[facebook-github-bot]

@mfunkie updated the pull request.

[facebook-github-bot]

@mfunkie updated the pull request.

[gaearon]

Nits:

• double space before It is
• every render => on every render (IMO that reads better)

[facebook-github-bot]

@mfunkie updated the pull request.

[gaearon]

I read this a couple of times, and that sentence still stands out as too long to me.
I think we can do better and I have another idea.

1. Change it to We recommend that you bind your event handlers in the constructor so they are only bound once for every instance:
2. Change the example below to include the state assignment so reader doesn’t assume it can be removed for some reason.
3. Before our new sentence, add an example demonstrating the previous (existing sentence), like

// You can use bind() to preserve `this`
<div onClick={this.tick.bind(this}>

// Or you can use arrow functions
<div onClick={() => this.tick()}>

4. Finally, at the very end, add another one liner, a la Now you can use this.tick directly as it was bound once in the constructor. This is better for performance of your application:

// It is already bound in the constructor
<div onClick={this.tick}>

Sorry for the back and forth. What do you think of this?

[mfunkie]

The back and forth is good :) In fact, experimenting with this in the comments (because they're markdown anyways) is what I should have been doing in the first place. I want to sneak something in there mentioning why instantiating functions over and over is bad (shouldComponentUpdate). Before I commit and squash, how does this look?

Methods follow the same semantics as regular ES6 classes, meaning that they don't automatically bind this to the instance. You'll have to explicitly use .bind(this) or arrow functions =>:

// You can use bind() to preserve `this`
<div onClick={this.tick.bind(this)}>

// Or you can use arrow functions
<div onClick={() => this.tick()}>

We recommend that you bind your event handlers in the constructor so they are only bound once for every instance:

constructor(props) {
  super(props);
  this.state = {count: props.initialCount};
  this.tick = this.tick.bind(this);
}

Now you can use this.tick directly as it was bound once in the constructor. This is better for performance of your application, especially if you compare props using shouldComponentUpdate:

// It is already bound in the constructor
<div onClick={this.tick}>

[gaearon]

This is looking great! Maybe worth changing “if you compare props using shouldComponentUpdate” to “if you implement shouldComponentUpdate() with a shallow comparison in the child components”.

[mfunkie]

Great idea. One last thing, what do you think about moving

"This is better for performance of your application, especially if you implement shouldComponentUpdate() with a shallow comparison in the child components."

to below the usage example?

e.g.

// It is already bound in the constructor
<div onClick={this.tick}>

This is better for performance of your application, especially if you implement shouldComponentUpdate() with a shallow comparison in the child components.

[gaearon]

👍 this makes more sense

[gaearon]

Let’s use a link from the root, i.e. [shallow comparison](/react/docs/...)

[facebook-github-bot]

@mfunkie updated the pull request.

[gaearon]

Let’s add some backticks for this.tick

[facebook-github-bot]

@mfunkie updated the pull request.

[gaearon]

Let’s make shouldComponentUpdate() a link to /react/docs/component-specs.html#updating-shouldcomponentupdate.

[facebook-github-bot]

@mfunkie updated the pull request.

[gaearon]

Thanks for your work on this! This will be up on the website soon.

[micky2be]

Good call