-
-
Save ogwurujohnson/9c6edfae770e81d07d0ecad752c2c78f to your computer and use it in GitHub Desktop.
import { useEffect } from 'react'; | |
function MyComponent({ fetchDrafts, fetchHistory }) { | |
useEffect(() => { | |
if (!fetchDrafts || !fetchHistory) { | |
return null | |
} | |
fetchDrafts(); | |
fetchHistory(); | |
const fetchDraftsTimer = setInterval(() => { | |
fetchDrafts(); | |
}, 120000); | |
return () => { | |
clearInterval(fetchDraftsTimer); | |
}; | |
}, []); | |
return null; | |
} |
Yeah, one bug i can think of is, because fetchDrafts() and fetchHistory() aren't passed an an argument when this component is rendered, both props will be undefined, and we calling them will lead to an error. Two ways of solving this will be to make use of propTypes and set both props as required and a function or add a conditional statement to only call this methods when they aren't undefined. What do you think
I updated the gist
The useEffect
needs a dependency array as a second argument. The second argument is optional, but in this case where we only want the code to execute once, i.e. componentDidMount
it is required to pass in an empty dependency array []
As the gist is written right now, everything would work until the component re-rendered. After a re-render then the useEffect code would be called a second time. The interval timer would be cancelled and rescheduled from time 0. If some other function was in the setInterval besides fetchDrafts
it may never be called depending on how often MyComponent
re-renders.
That makes sense now. I thought removing the dependency array entirely would force it to run only once, this is my first time learning about this difference. Thanks will keep this in mind going forward.
The class implementation definition of props is a good catch, but not what is wrong with your code.
This code runs as you already know, but it has a bug in it.