Reputation: 7001
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
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
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