dis-gui

dis-gui copied to clipboard

I've come across this in the code:

{this.state.options.map(function(opt,index) {
              return (
                <option
                  value={opt}
                  key={opt + index}
                  style={{
                    backgroundColor: this.context.style.font,
                  }}
                >
                  {opt}
                </option>
              )
            }.bind(this))}

Not sure if it's a good idea to have a call to bind() inside of render().

Not entirely sure where this comes from ( i know it as transform-class-properties )

class Foo{
constructor(){}
method = ()=>{}
} 

But another pattern would also solve this I believe:

constructor(){
  this._renderOption = this._renderOption.bind(this)
  this._onChange = this._onChange.bind(this)
}
_renderOption(opt,index){...}

well, the problem was {this.state.options.map(function(opt,index) { - as soon as you use a classic function() {} in JSX, the scope is lost. For iterations in JSX we should always use arrow syntax, e.g. {this.state.options.map((opt,index) => {`

But binding in the constructor should be lighter?

yes, that for sure. But we'll go with class properties indeed, e.g. handleChange = e => {} - it's the lightest variant.

I personally always use autobind-decorator in other projects, e.g.

@autobind
handleChange(e) {
...
}

mostly because it improves workflow with Sublime Text (Ctrl+R gives you a way to jump to class methods, but only if they are regular syntax and not an arrow). But we shouldn't bloat this library, arrow is good enough.

Alright, there's lots of instances across the codebase. Should take some time and go through them one by one. I totally agree that we shouldn't dynamically bind, as it creates a new function object each time.