Skip to content

Commit

Permalink
Removing calls to setState after unmount (ReactTraining#139)
Browse files Browse the repository at this point in the history
Issue ReactTraining#139 was the same issue I was having when resizing a window.
I was receiving an error about a possible memory leak since setState
was called on an unmounted component. It seems like the `getMatches`
call was taking just long enough that the component could be unmounted
while it was executing. By using a `_mounted` flag, we can determine
if it is safe to call `setState` and bypass it if the component is
unmounted. I looked at using an abort signal, but I don't think the
`matchMedia` API is robust enough for that currently.
  • Loading branch information
stevenwcarter committed Nov 21, 2019
1 parent 617c888 commit ba31564
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions modules/Media.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class Media extends React.Component {
onChange: PropTypes.func
};

_mounted = false;

queries = [];

constructor(props) {
Expand Down Expand Up @@ -98,14 +100,11 @@ class Media extends React.Component {
};

updateMatches = () => {
const newMatches = this.getMatches();
const matches = this.getMatches();

this.setState(
() => ({
matches: newMatches
}),
this.onChange
);
if (this._mounted) {
this.setState({matches}, this.onChange);
}
};

initialize() {
Expand All @@ -132,6 +131,7 @@ class Media extends React.Component {
}

componentDidMount() {
this._mounted = true;
this.initialize();
// If props.defaultMatches has been set, ensure we trigger a two-pass render.
// This is useful for SSR with mismatching defaultMatches vs actual matches from window.matchMedia
Expand All @@ -149,6 +149,7 @@ class Media extends React.Component {
}

componentWillUnmount() {
this._mounted = false;
this.queries.forEach(({ mqListener }) => mqListener.cancel());
}

Expand Down

0 comments on commit ba31564

Please sign in to comment.