Reputation: 6015
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;
Upvotes: 23
Views: 26147
Reputation: 1075655
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;
About setting timerHandle
to 0
when there's no active timer: It makes a convenient flag (see clearTimer
above), and 0
is guaranteed not to be a valid timer handle by the timers specification. (For Node.js, you'd use null
, since setTimeout
returns an object, not a number as it does in browsers.)
Upvotes: 42
Reputation: 7094
React 16.3: this solution worked for me, the anothers solutios did not work in my case:
class Modal extends Component {
constructor(props) {
super(props);
this.timeout = null;
this.state = {
clicked: false,
};
}
handleClick = async (e, action) => {
if (!this.state.clicked) { /
this.setState( {clicked: true} , async () => {
const res = await action();
this.timeout = setTimeout(() => {
if (this.mounted) this.setState( {clicked: false});
this.timeout = null;
}, 2000);
});
}
};
componentDidMount = () => {
this.mounted = true;
};
componentWillUnmount = () =>{
this.mounted = false;
if (this.timeout) {
clearTimeout(this.timeout)
}
};
Upvotes: 1
Reputation: 2337
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);
Upvotes: 6