Stores' change listeners not getting removed on componentWillUnmount?

Question

I am coding a simple app on reactjs-flux and everything works fine except I am receiving a warning from reactjs telling me that I am calling setState on unmounted components.

I have figured out this is because changelisteners to which components are hooked are not being removed from the store on componentWillUnmount. I know it because when I print the list of listeners from Eventemitter I see the listener which was supposed to be destroyed still there, and the list grows larger as I mount/unmount the same component several times.

I paste code from my BaseStore:

import Constants from '../core/Constants';
import {EventEmitter} from 'events';

class BaseStore extends EventEmitter {
  // Allow Controller-View to register itself with store
  addChangeListener(callback) {
    this.on(Constants.CHANGE_EVENT, callback);
  }

  removeChangeListener(callback) {
    this.removeListener(Constants.CHANGE_EVENT, callback);
  }

  // triggers change listener above, firing controller-view callback
  emitChange() {
    this.emit(Constants.CHANGE_EVENT);
  }
}

export default BaseStore;

I paste the relevant code from a component experiencing this bug (it happens with all components, though):

@AuthenticatedComponent
class ProductsPage extends React.Component {
  static propTypes = {
    accessToken: PropTypes.string
  };

  constructor() {
    super();
    this._productBatch;
    this._productBatchesNum;
    this._activeProductBatch;
    this._productBlacklist;
    this._searchById;
    this._searchingById;
    this.state = this._getStateFromStore();
  }

  componentDidMount() {
    ProductsStore.addChangeListener(this._onChange.bind(this));
  }

  componentWillUnmount() {
    ProductsStore.removeChangeListener(this._onChange.bind(this));
  }

  _onChange() {
    this.setState(this._getStateFromStore());
  }
}

This is driving me pretty nuts at this point. Any ideas?

Thank you!

Answer 1

Short version: expect(f.bind(this)).not.toBe(f.bind(this));

Longer explanation:

The cause of the issue is that EventEmitter.removeListener requires that you pass a function you have previously registered with EventEmitter.addListener. If you pass a reference to any other function, it is a silent no-op.

In your code, you are passing this._onChange.bind(this) to addListener. bind returns a new function that is bound to this. You are then discarding the reference to that bound function. Then you try to remove another new function created by a bind call, and it's a no op, since that was never added.

React.createClass auto-binds methods. In ES6, you need to manually bind in your constructor:

@AuthenticatedComponent
class ProductsPage extends React.Component {
  static propTypes = {
    accessToken: PropTypes.string
  };

  constructor() {
    super();
    this._productBatch;
    this._productBatchesNum;
    this._activeProductBatch;
    this._productBlacklist;
    this._searchById;
    this._searchingById;
    this.state = this._getStateFromStore();
    // Bind listeners (you can write an autoBind(this);
    this._onChange = this._onChange.bind(this);
  }

  componentDidMount() {
    // listener pre-bound into a fixed function reference. Add it
    ProductsStore.addChangeListener(this._onChange);
  }

  componentWillUnmount() {
    // Remove same function reference that was added
    ProductsStore.removeChangeListener(this._onChange);
  }

  _onChange() {
    this.setState(this._getStateFromStore());
  }

There are various ways of simplifying binding - you could use an ES7 @autobind method decorator (e.g. autobind-decorator on npm), or write an autoBind function that you call in the constructor with autoBind(this);.

In ES7, you will (hopefully) be able to use class properties for a more convenient syntax. You can enable this in Babel if you like as part of the stage-1 proposal http://babeljs.io/docs/plugins/transform-class-properties/ . Then, you just declare your event listener methods as class properties rather than methods:

_onChange = () => {
    this.setState(this._getStateFromStore());
}

Because the initializer for _onChange is invoked in the context of the constructor, the arrow function auto-binds this to the class instance so you can just pass this._onChange as an event handler without needing to manually bind it.