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

[addons-react] Update FluxibleMixin to check for autobind:false #316

Closed
redonkulus opened this issue Nov 25, 2015 · 6 comments
Closed

[addons-react] Update FluxibleMixin to check for autobind:false #316

redonkulus opened this issue Nov 25, 2015 · 6 comments

Comments

@redonkulus
Copy link
Contributor

If set to false, then manually bind the onChange handler.

https://github.com/yahoo/fluxible/blob/master/packages/fluxible-addons-react/FluxibleMixin.js#L116

@Vijar
Copy link
Contributor

Vijar commented Nov 25, 2015

@mridgway thoughts?

We can avoid having users do the current workaround:

componentWillMount: function () {
    this.onChange = this.onChange.bind(this);
}

@mridgway
Copy link
Collaborator

Sure, I'm fine with this.

@mridgway mridgway changed the title Update FluxibleMixin to check for autobind:false [addons-react] Update FluxibleMixin to check for autobind:false Dec 17, 2015
@mridgway
Copy link
Collaborator

The autobind setting is not added to the React components, it is only used during construction of the React component class. Therefore it is currently not possible to detect whether autobinding is enabled or not.

@ericf
Copy link

ericf commented May 4, 2016

this.constructor.prototype.__reactAutoBindMap

Edit: This is a bad approach because it's private and might have changed in React 15.

@mridgway mridgway reopened this May 4, 2016
@ericf
Copy link

ericf commented May 4, 2016

@mridgway and I discussed this, and came up with some options. I think the simplest might be to just set the context to this (the component instance) when registering the event listener:

listener.store.on('change', listener.handler);

If we think the above could lead to BC breakage, we could always try to detect this pattern by updating getHandler():

getHandler: function (handler) {
    if ('string' === typeof handler) {
        handler = this[handler];
    }

    if (!handler) {
        throw new Error('storeListener attempted to add undefined handler. Make sure handlers actually exist.');
    }

    // Bind the handler to the component instance. This protects againsts:
    // `autobind: false` causing `onChange` to be called with the wrong context.
    if (handler === Object.getPrototypeOf(this).handler) {
        handler = handler.bind(this);
    }

    return handler;
},

@redonkulus
Copy link
Contributor Author

Closing as mixins are being deprecated.

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

No branches or pull requests

4 participants