Autobind decorator

[slonoed]

I want to use autobind decorator for components.

This https://github.com/andreypopp/autobind-decorator/blob/master/src/index.js work fine with React (method binding).
But have issue with RHL. Decorator bind method to instance. When new RHL add new prototype to prototype chain nothing happened, because method already in instance and new method in proto not visible when getter called.

I use ugly hack and check in getter if (this.props) to determine is this – instance.

It is not an issue with RHL or decorator itself. I try to find nice solution.

[chrisvariety]

The exact message I get when trying this is Cannot set property fooBarMethod of #<Component> which has only a getter

[gaearon]

You can try finding a fix by adding a failing test to react-hotify. Then, if you find a solution, I can backport it to RHL.

[slonoed]

@gaearon should I add test for work with autobind-decorator?

[gaearon]

@slonoed Yes, if you also find a fix.

[gaearon]

@slonoed It's also worth checking if this change helps (or makes it worse, I dunno)

[vasa-chi]

Dunno if much help, but here is a test for react-hotify for this problem (using core-decorators):

import React, { Component } from 'react';
import createShallowRenderer from './helpers/createShallowRenderer';
import expect from 'expect.js';
import makeHotify from '../src/makeHotify';

import { autobind } from 'core-decorators';

class Bar {

  @autobind
  getProps() {
    return typeof this.props;
  }

  render() {
    return <div>{this.getProps()}</div>;
  }
}

describe('autobind usage', function() {

  let renderer;
  let hotify;

  beforeEach(() => {
    renderer = createShallowRenderer();
    hotify = makeHotify();
  });

  it('should bind methods of hotified components', function() {
    const HotBar = hotify(Bar);
    const barInstance = renderer.render(<HotBar />);
    expect(renderer.getRenderOutput().props.children).to.equal('object');
  });
})

[gaearon]

Thanks! I'll definitely get back to this after my React Europe talk.

[cpsubrian]

Happy to help test possible fixes. Also wondering if this is a general problem interoperating with decorators in general, and if that should be the focus rather than @autobind in particular?

[slonoed]

For me this issue not actual now. I switch to arrow functions in class definition.

class Cmp extends Component {
  myBindedFunc = () => {
     this.callSmth();
  }
}

Can I close this issue?

[cpsubrian]

I've done the same, but consider it a temporary fix. I prefer the decorator approach (because I bind at the class level, not the method level). I will chime back in here if my other decorators have issues (I haven't gotten that far yet, this is my first hot-reload project).

[lelandrichardson]

@slonoed this arrow function approach works? Will it always? I would have thought that in that scenario the arrow function would have made this bound to the this in the context outside of your class definition. Is this just a convenient bug or is this actually the spec'd behavior of arrow functions inside class definitions?

[opatut]

I am having issues with the arrow function, actually. The hot reloading does not apply when using render () => { ... } instead of render () {}. Hence, no 'autobinding' :( Any ideas on how to fix this?

[slonoed]

@lancefisher I think there is no spec for arrow functions in class.
This is babel stuff https://gist.github.com/jeffmo/054df782c05639da2adb
Babel put this functions assignments in constructor. And bound to this in constructor (i.e. instance of class).

@opatut why you want bind render? It is always called by library code, isnt it?

[opatut]

It's not only about binding render, when I add other methods, such as event callbacks, I cannot change logic inside them with hot reloading, since they are not proxied.

However, I find that binding the methods with .bind() or .call() upon usage works great, especially after I discovered the new (stage 0) function bind syntax. Example:

class MyComponent extends React.Component {
    onClick () {
        console.log(this); // `this` is bound
    }

    render () {
        return <button onClick={::this.onClick}>Click me</button>;
    }
}

[gaearon]

Properties are very tricky to hot reload because they're really being defined inside the constructor.
We'll see what we can do with a Babel plugin later, but for now, the workaround from @opatut is the best we got.

[gaearon]

We're supporting autobind decorator in the next beta release coming soon.
Stay tuned.

[gaearon]

Can I ask you to check whether [email protected] solves this issue?
Please provide any feedback on it in this PR: #182

[steadicat]

2.0.0-alpha fixes hot reload of fat arrow methods for me. Thank you!

[gaearon]

@steadicat Hmm, I'm pretty sure fat arrow methods still shouldn't get hot reloaded. Can you check again, or share a reproducible case?

[gaearon]

(That said @autobind decorator should work now. Just not the fat arrows.)

[steadicat]

@gaearon You're right, it turns out I have a decorator on my components that somehow works around this issue. Here’s a reduced test case (code below):

import React from 'react';

function wrap(Component) {
  return class Wrapper {
    render() {
      return <Component {...this.props} />;
    }
  }
}

@wrap
export default class Test {

  render() {
    return this.renderInner();
  }

  renderInner = () => {
    return <h1>Version 4</h1>;
  }

}

[gaearon]

Oh. It works because it generates a new class on every invocation. The downside is Test state will be reset on every hot reload.

[yangmillstheory]

I don't think this is fully working with [email protected] yet.

Small test commit here.

In the demo above there's 3 steps:

1. without @autobind on onClick, click the button. This doesn't have the right context - this is null.
2. add @autobind to onClick - this is recognized by RHL and correct logs the right state in the console (this is bound to the Counter instance).
3. remove @autobind, this is still bound to the Counter instance (you would expect it not to be).

RHL doesn't remove the binding when using fat arrows to declare the method either.

Hope that's helpful, would really love to use this with RHL!

[geyang]

I'm having trouble too, no production ready example yet.

It might make sense to reopen this issue.

[gaearon]

Add/removing @autobind is not going to be hot reloaded correctly—it doesn’t seem to be very useful to support this corner case. But tweaking @autobound methods should work. If not, please file a new issue with a reproducing project.

[geyang]

got it! I moved to the beta and everything worked after I updated to node 6.x, so it is all good!
Thanks for the response.