Mr. Goferito
Mr. Goferito

Reputation: 7001

React Component with Memory Leak

I have a piece of legacy code, which renders a react component on the server on every request, which makes it obvious there is a memory leak. I have corner the problem up to this code:

  componentWillMount: function () {
    var onLogin = this.props.onLogin || function () {},
        onLogout = this.props.onLogout || function () {};

    this.on('authChange', function () {
      console.log('user authenticated:', this.state.isAuthenticated);
      return this.state.isAuthenticated
              ? onLogin(this.state)
              : onLogout(this.state);
    }.bind(this));
  },

I believe that on every request the this object is storing a new listener, but I don't get why the this element is not being marked as garbage when the rendering of the component is done.

Upvotes: 5

Views: 13582

Answers (2)

xiao
xiao

Reputation: 1758

I would move your function into componentDidMount and add cleanup on componentWillUnmount

Important: componentWillMount is called on the server and client, but componentDidMount is only called on the client.

If you’re using eventListeners, setInterval or other functions that needs to be cleaned, put them in componentDidMount. The server will not call componentWillUnmount and is usually the cause of memory leaks.

Upvotes: 3

Tom Fenech
Tom Fenech

Reputation: 74595

You need to unbind the authChange handler before the component is unmounted. You can do this in componentWillUnmount.

Since you're creating the handler function using the first props that are passed in, you should save it to a property so you can unbind it later:

  componentWillMount: function () {
    var onLogin = this.props.onLogin || function () {},
        onLogout = this.props.onLogout || function () {};

    this.authChange = function () {
      console.log('user authenticated:', this.state.isAuthenticated);
      return this.state.isAuthenticated
              ? onLogin(this.state)
              : onLogout(this.state);
    }.bind(this);

    this.on('authChange', this.authChange);
  },

  componentWillUnmount: function () {
      this.off('authChange', this.authChange);
      this.authChange = null;
  }

Note that when I saw this.on I thought you might be using jQuery but it's not clear how that would be the case. My answer uses this.off to detach the event listener but you should use whatever the corresponding method is in your framework.

Upvotes: 8

Related Questions