5

In a lot of my components I need to do something like this:

handleSubmit() {
  this.setState({loading: true})
  someAsyncFunc()
    .then(() => {
      return this.props.onSuccess()
    })
    .finally(() => this.setState({loading: false}))
}

The onSuccess function

  • may or may not be a promise (if it is, loading should stay true until it is resolved)
  • may or may not unmount the component (it may close the modal this component is in or even navigate to different page)

If the function unmounts the component, this.setState({loading: false}) obviously triggers a warning Can't call setState (or forceUpdate) on an unmounted component.

My 2 questions:

  1. Is there a simple way to avoid the issue ? I don't want to set some _isMounted variable in componentDidMount and componentWillUnmount and then check it when needed in most of my components, plus I may forget to do it next time writing something like this ...
  2. Is it really a problem ? I know that, according to the warning, it indicates a memory leak in my application, but it is not a memory leak in this case, is it ? Maybe ignoring the warning would be ok ...

EDIT: The second question is a little bit more important for me than the first. If this really is a problem and I just can't call setState on unmounted component, I'd probably find some workaround myself. But I am curious if I can't just ignore it.

Live example of the problem:

const someAsyncFunc = () => new Promise(resolve => {
  setTimeout(() => {
   console.log("someAsyncFunc resolving");
    resolve("done");
  }, 2000);
});

class Example extends React.Component {
  constructor(...args) {
    super(...args);
    this.state = {loading: false};
  }
  
  componentDidMount() {
    setTimeout(() => this.handleSubmit(), 100);
  }
  
  handleSubmit() {
    this.setState({loading: true})
    someAsyncFunc()
      /*
      .then(() => {
        return this.props.onSuccess()
      })
      */
      .finally(() => this.setState({loading: false}))
  }
  
  render() {
    return <div>{String(this.state.loading)}</div>;
  }
}

class Wrapper extends React.Component {
 constructor(props, ...rest) {
   super(props, ...rest);
    this.state = {
     children: props.children
    };
  }
 componentDidMount() {
   setTimeout(() => {
     console.log("removing");
     this.setState({children: []});
    }, 1500)
  }
 render() {
   return <div>{this.state.children}</div>;
  }
}

ReactDOM.render(
 <Wrapper>
    <Example />
 </Wrapper>,
  document.getElementById("root")
);
.as-console-wrapper {
  max-height: 100% !important;
}
<div id="root"></div>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react-dom.js"></script>
lukasbalaz7
  • 360
  • 2
  • 11
  • In component modal closing and navigation,why you need loading false?Anyway the state associated with it going to destroy – RIYAJ KHAN Jun 06 '18 at 09:51
  • @RIYAJKHAN As I wrote: the `onSuccess` function **may or may not unmount the component** - in some cases, the modal should stay open, without loading of course ... – lukasbalaz7 Jun 06 '18 at 09:54
  • Possible duplicate of [React - setState() on unmounted component](https://stackoverflow.com/questions/32903001/react-setstate-on-unmounted-component) – burak Jun 06 '18 at 10:06

4 Answers4

1

Unfortunately you have to keep track of "isMounted" yourself. To simplify you control flow you could use async/await:

handleSubmit() {
  this.setState({loading: true})
  try {
    await someAsyncFunction()
    await this.props.onSuccess()
  } finally {
    if (this._isMounted) {
      this.setState({loading: false})
    }
  }
}

This is actually mentioned in the react docs, which points to this solution: https://gist.github.com/bvaughn/982ab689a41097237f6e9860db7ca8d6

If your someAsyncFunction supports cancelation, you should do so in componentWillUnmount, as encouraged by this article. But then - of course - check the return value and eventually not call this.props.onSuccess.

lipp
  • 5,586
  • 1
  • 21
  • 32
  • Upvoted because you are technically correct, but I am actually more interested if I can't just ignore the warning. I updated the question ... – lukasbalaz7 Jun 06 '18 at 10:41
  • Thanks. I think you should never tolerate a warning. A newer version of React might get stricter in this regard. – lipp Jun 06 '18 at 11:10
1
class myClass extends Component {
  _isMounted = false;

  constructor(props) {
    super(props);

    this.state = {
      data: [],
    };
  }

  componentDidMount() {
    this._isMounted = true;
    this._getData();
  }

  componentWillUnmount() {
    this._isMounted = false;
  }

  _getData() {
    axios.get('example.com').then(data => {
      if (this._isMounted) {
        this.setState({ data })
      }
    });
  }


  render() {
    ...
  }
}
john_per
  • 320
  • 6
  • 16
0

You should be able to use this._isMounted to check if the component is actually mounted.

handleSubmit() {
  this.setState({loading: true})
  someAsyncFunc()
    .then(() => {
      return this.props.onSuccess()
    })
    .finally(() => {
      if (this && this._isMounted) { // check if component is still mounted
        this.setState({loading: false})
      }
    })
}

But be aware that this approach is considered to be an anitpattern. https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html

lipp
  • 5,586
  • 1
  • 21
  • 32
Hinrich
  • 13,485
  • 7
  • 43
  • 66
  • So `this` is already `undefined` at that stage? You could check that, too. I updated my answer. – Hinrich Jun 06 '18 at 10:19
  • No, check the last line of the article you linked in your answer: `Getting rid of isMounted() makes it one step easier for you to upgrade to ES6 classes, where using isMounted() is already prohibited`. It's just not in react anymore. – lukasbalaz7 Jun 06 '18 at 10:26
0

What about

componentWillUnmount() {  
    // Assign this.setState to empty function to avoid console warning
    // when this.setState is called on an unmounted component
    this.setState = () => undefined;
}
anonymous
  • 1
  • 1