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

A question about binding functions inside PureComponent? #16879

Closed
hoorsa opened this issue Nov 18, 2017 · 4 comments
Closed

A question about binding functions inside PureComponent? #16879

hoorsa opened this issue Nov 18, 2017 · 4 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@hoorsa
Copy link

hoorsa commented Nov 18, 2017

Is this a bug report?

No

Have you read the Contributing Guidelines?

Yes

Environment

OS:  Windows 8
Node:  8.5.0
Yarn:  Not Found
npm:  4.6.1
Watchman:  Not Found
Xcode:  N/A
Android Studio:  2.3 AI-162.4069837

Packages: (wanted => installed)
react-native: ^0.49.2 => 0.49.5
react: ^16.0.0-beta.5 => 16.0.0

Target Platform: Android (6.0)

Hi,
Goal is best performance. Which of below script result is better in performance (this is the renderItem PureComponent of a large FlatList):

1:

class MyComponent extends React.PureComponent {
  
  constructor(props) {
    super(props);
    this.doWork = this.doWork.bind(this);
  }
  doWork() {
    // doing some work here.
  }
  
  render() {
    return <Text onPress={this.doWork}>Do Some Work</Text>
  }
  
}

2:

class MyComponent extends React.PureComponent {
  doWork = () => {
    // doing some work here.
  }
  
  render() {
    return <Text onPress={this.doWork}>Do Some Work</Text>
  }
  
}

3:

class MyComponent extends React.PureComponent {
  doWork(){
    // doing some work here.
  }
  
  render() {
    return <Text onPress={() => this.doWork}>Do Some Work</Text>
  }
  
}

4:

class MyComponent extends React.PureComponent {
  doWork(){
    // doing some work here.
  }
  
  render() {
    return <Text onPress={ this.doWork.bind(this) }>Do Some Work</Text>
  }
  
}

5:
Your script...

@ilijaNL
Copy link

ilijaNL commented Nov 20, 2017

1 & 2 (2 uses new ES class properties) both are the same and recommended

@somonek
Copy link

somonek commented Nov 20, 2017

3 & 4 are bad for performance because you do extra work on each render.
3 creates a new function each time and 4 does the binding on each render.
Personally I think # 2 is not so elegant, so number one wins even though I like this decorator https://github.com/andreypopp/autobind-decorator which gives you the overview of where the context is bound directly on the method. If you have a large class, you don't need to scroll up to the constructor and check. Often you don't even need the constructor.

@somonek
Copy link

somonek commented Nov 20, 2017

just for additional info, you can take a look at this proposal as well https://www.npmjs.com/package/babel-plugin-transform-function-bind

@hoorsa
Copy link
Author

hoorsa commented Nov 20, 2017

thank you @Rusfighter and @somonek .

@hoorsa hoorsa closed this as completed Nov 21, 2017
@facebook facebook locked as resolved and limited conversation to collaborators Nov 21, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Nov 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

4 participants