letakeane / emotican-app

1 stars 4 forks source link

Refactor: Play component #36

Open Jeff-Duke opened 7 years ago

Jeff-Duke commented 7 years ago

In app/components/Play/Play.js you're binding methods to pass down like so: <ImageCapture analyzeEmotions={this.analyzeEmotions.bind(this)} getImageURL={this.getImageURL.bind(this)}

This is considered an anti-pattern and has performance implications. Here's an interesting read if you'd like to know more: https://medium.com/@esamatti/react-js-pure-render-performance-anti-pattern-fb88c101332f

A couple options are to bind the action in the constructor:

export default class Play extends Component {
  constructor() {
    super()
    this.state = {
      emotions: [],
      canvasURL: ''
    }
    this.analyzeEmotions = this.analyzeEmotions.bind(this)
  }

Or use an arrow function in the class property:

analyzeEmotions = (faceBlob) => {}

Binding in the constructor is supported in ES6 and recommended by React. Using an arrow function in the class property is also recommended by React, but not supported in ES6, it's part of ES7 standard. As such you'll need to modify your .babelrc to include stage-2 presets.

Some more reading/info: