Better way to clearTimeout in componentWillUnmount

Question

I have a working Loading Component that cancels out when it has been loading for 8 seconds. This code works but it feels off to me and I'm wondering if there is a better way to do this.

Without setting this.mounted I get the error:

Warning: Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op. Please check the code for the Loading component.

This make me think that the timer is not getting canceled so it continues with this.seState. Why would that be if I set clearTimeout in componentWillUnmount? Is there a better way to handle this than using a global this.mounted?

class Loading extends Component {   state = {     error: false,   };    componentDidMount = () => {     this.mounted = true;     this.timer();   };    componentWillUnmount = () => {     this.mounted = false;     clearTimeout(this.timer);   };    timer = () =>     setTimeout(() => {       (this.mounted && this.setState({ error: true })) || null;     }, 8000);    render() {     const { showHeader = false } = this.props;     const { error } = this.state;     return (       <View style={backgroundStyle}>         {showHeader && <HeaderShell />}         {!error &&           <View style={loadingHeight}>             <PlatformSpinner size="large" />           </View>}         {error && <Error code="service" />}       </View>     );   } }  Loading.propTypes = {   showHeader: PropTypes.bool, };  Loading.defaultProps = {   showHeader: false, };  export default Loading; 

Answer 1

This make me think that the timer is not getting canceled

As Pointy said, it isn't. You're passing a function (this.timer) into clearTimeout. You need to pass the setTimeout return value (the handle of the timer), so you can use that handle to cancel it.

In such a simple componennt, I don't see the need for the timer function, it just adds complexity; I'd just set up the timer in CDM:

class Loading extends Component {   state = {     error: false,   };    componentDidMount = () => {                // ***     // Remember the timer handle             // ***     this.timerHandle = setTimeout(() => {    // ***       this.setState({ error: true });        // ***       this.timerHandle = 0;                  // ***     }, 8000);                                // ***   };                                         // ***                                              // ***   componentWillUnmount = () => {             // ***     // Is our timer running?                 // ***     if (this.timerHandle) {                  // ***         // Yes, clear it                     // ***         clearTimeout(this.timerHandle);      // ***         this.timerHandle = 0;                // ***     }                                        // ***   };                                         // ***    render() {     const { showHeader = false } = this.props;     const { error } = this.state;     return (       <View style={backgroundStyle}>         {showHeader && <HeaderShell />}         {!error &&           <View style={loadingHeight}>             <PlatformSpinner size="large" />           </View>}         {error && <Error code="service" />}       </View>     );   } }  Loading.propTypes = {   showHeader: PropTypes.bool, };  Loading.defaultProps = {   showHeader: false, };  export default Loading; 

But if there's more logic than shown, or just personal preference, yes, separate functions are good:

class Loading extends Component {   state = {     error: false,   };    componentDidMount = () => {     this.setTimer();   };    componentWillUnmount = () => {     this.clearTimer();   };    setTimer = () => {     if (this.timerHandle) {       // Exception?       return;     }     // Remember the timer handle     this.timerHandle = setTimeout(() => {       this.setState({ error: true });       this.timerHandle = 0;     }, 8000);   };    clearTimer = () => {     // Is our timer running?     if (this.timerHandle) {         // Yes, clear it         clearTimeout(this.timerHandle);         this.timerHandle = 0;     }   };    render() {     const { showHeader = false } = this.props;     const { error } = this.state;     return (       <View style={backgroundStyle}>         {showHeader && <HeaderShell />}         {!error &&           <View style={loadingHeight}>             <PlatformSpinner size="large" />           </View>}         {error && <Error code="service" />}       </View>     );   } }  Loading.propTypes = {   showHeader: PropTypes.bool, };  Loading.defaultProps = {   showHeader: false, };  export default Loading; 

Answer 2

You need to clear using the returned value from setTimeout (see below). However, doing clearTimeout in componentWillUnmount is a correct way to do it, I haven't seen anyone do it any differently.

  componentDidMount = () => {     this.mounted = true;     this.timeout = this.timer();   };    componentWillUnmount = () => {     this.mounted = false;     clearTimeout(this.timeout);   };    timer = () =>     setTimeout(() => {       (this.mounted && this.setState({ error: true })) || null;     }, 8000);