sheaivey / react-axios

Axios Components for React with child function callback
MIT License
180 stars 20 forks source link

may call setState() after being unmounted due to debounce #9

Closed binki closed 7 years ago

binki commented 7 years ago

I have this component in my application and sometimes I get warnings from React about calling setState() after a component is unmounted. It appears that react-axios is being mounted and because of navigation or something it is being unmounted before the debounce period completes.

I would recommend to track whether or not the component is mounted using a pattern like:

class MyComponent extends Component {
  componentDidMount() {
    this._mounted = true;
  }

  componentWillUnmount() {
    this._mounted = false;
  }

  someAsyncCallback(results) {
    if (!this._mounted) {
      return;
    }
    this.setState({
      results: results,
    });
  }
}

Ideally you could also try to cancel all ongoing Axios requests upon unmount. I guess ideally you shouldn’t use the pattern that I first suggest but should somehow cancel everything in componentWillUnmount() and then in async callbacks detect that the callback was called after a cancellation. Depends on how axios works though. It might be that if it has a success callback queued and you call cancel the success callback may still end up being called, so maybe my _mounted = true/false pattern is necessary anyway.

sheaivey commented 7 years ago

This has been fixed as of v1.0.1

I have added a componentWillUnmount() check to cancel any pending requests.

Thanks

binki commented 7 years ago

I was using 1.01 (EDIT: 1.0.1) when I observed this. From reading the source, I think there is a race condition:

  1. componentWillReceiveProps() or componentDidMount() call debounceMakeRequest()
  2. Component is unmounted. componentWillUnmount() runs but this.source is still undefined, so the cancellation logic does nothing.
  3. debounce time period expires and makeRequest() runs which calls setState() which caused React to print out a warning to my console.

Sorry I did not make a repro, let me know if that is needed.

binki commented 7 years ago

@sheaivey Please see my repro for this issue in pull #12. I’m convinced this bug exists and this issue should be reopened.