Skip to content

The :: bind operator probably won't be going forward #539

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

Closed
samdesota opened this issue Nov 13, 2015 · 7 comments · Fixed by #547
Closed

The :: bind operator probably won't be going forward #539

samdesota opened this issue Nov 13, 2015 · 7 comments · Fixed by #547

Comments

@samdesota
Copy link

Hey guys,

From the looks of things, the bind operator won't be going forward.

I'm currently using this little helper:

export default function bindHandlers(Target) {
  return class BoundComponent extends Target {
    constructor(args...) {
      super(args...)

       // Get's the target prototypes properties
      const properties = Object.getOwnPropertyNames(Target.prototype)
      for (let methodIndex = 0, len = properties.length; methodIndex < len; methodIndex++) {

        const method = properties[methodIndex]
        // If the method's name starts with an _, bind the method
        if (method.startsWith('_')) {
          this[method.substring(1)] = this[method].bind(this)
        }
      }

    }
  }
}

Not sure if this is the best way to do it, but it's a decorator that binds all methods that begin with an _, removing the _ for the bound method. It may be better to just add a bindHandlers function to the prototype.

Do think the project should still use the bind operator or switch to an auto-binding option?

@samdesota samdesota changed the title The :: probably won't be The :: probably won't be going forward Nov 13, 2015
@samdesota samdesota changed the title The :: probably won't be going forward The :: bind operator probably won't be going forward Nov 13, 2015
@erikras
Copy link
Owner

erikras commented Nov 13, 2015

Thanks for pointing this out. It makes me a bit sad, but my IDE never really liked it, so my code might actually be "green" now.

The this keyword has always been a bit of a third rail in javascript. To be honest, I don't even see the need for the _ in your example. I'm never going to put a method on a javascript class that I want to ever be run with another this value. autobind-decorator supposedly does this, but I found that it didn't work too well whenever I used it.

@quicksnap
Copy link
Collaborator

There's another way of getting around the common use case of :: in Component event handlers:

class MyThing extends Component {
  const onClick = (e) => { this.isBound(); }

  render() { return <div onClick={this.onClick} /> }
}

@erikras
Copy link
Owner

erikras commented Nov 13, 2015

Arrows to the rescue!

@samdesota
Copy link
Author

The arrow is a good idea. That still works with super, correct?

Also, this solves a problem with unbinding listeners.

This returns false: ::this.handle === ::this.handle.

meaning you can't un-attach bound listeners easily.

@samdesota
Copy link
Author

After testing, you can't call super with the arrow approach. Babel reports unexpected super.

Classes really should have been spec`ed to be bound by default. 😬

@erikras
Copy link
Owner

erikras commented Nov 13, 2015

IMHO, you shouldn't be using OO inheritance at all in React, so super is not an issue.

@justingreenberg
Copy link
Contributor

@quicksnap i've been using the same technique! one thing in your code snippet just wanted to point out click handler would have to be a class method to be invoked using this

class MyThing extends Component {
  boundHandler = () => this.isBound();
  render() { return <button onClick={this.boundHandler} /> }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants