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.

Answer 2

So I have found the solution, it turns out I only had to assign this._onChange.bind(this) to an internal property before passing it as an argument to removechangelistener and addchangelistener. Here is the solution:

  componentDidMount() {
    this.changeListener = this._onChange.bind(this);
    ProductsStore.addChangeListener(this.changeListener);
    this._showProducts();
  }
  componentWillUnmount() {
    ProductsStore.removeChangeListener(this.changeListener);
  }

I do not know, however, why this solves the issue. Any ideas?

Answer 3

Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the exports component.

I am using the exact same implementation across multiple react components. i.e. this is repeated across several .jsx components.

componentDidMount: function() {
    console.log('DidMount- Component 1');
   ViewStateStore.addChangeListener(this._onChange);
},

componentWillUnmount: function() {
    console.log('DidUnMount- Component 1');
   ViewStateStore.removeChangeListener(this._onChange);
},

_onChange:function()
{
    console.log('SetState- Component 1');
    this.setState(getStateFromStores());
},

Possible Solution

Currently the following is working out for me, but it has been a little temperamental. Wrap the call back in a function/named-function.

ViewStateStore.addChangeListener(function (){this._onChange});

one might also try

ViewStateStore.addChangeListener(function named(){this._onChange});

Theory

EventEmitter is for some reason getting confused identifying the callback to remove. Using a named function is perhaps helping with that.