-
Notifications
You must be signed in to change notification settings - Fork 531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add useInterval hook #5
Conversation
Thanks! I found this other solution you proposed to be pretty easy to understand: facebook/react#14543 (comment) Are there any downsides to that solution vs. the one in this pull request? |
Let me highlight the problem: we need to execute some code in cycle with constant time interval. Similar to what const [state, setState] = useState(0)
useEffect(() => {
const id = setInterval(
() => { console.log(state) } // always logs "0"
, 1000)
return () => clearInterval(id)
}, []) is that the first state is trapped in interval closure. useEffect(() => {
const id = setInterval(
() => { console.log(state) } // works almost as expected
, 1000)
return () => clearInterval(id)
}, [state]) Looks fine, but now it doesn't meet the criteria of constant time interval between calls. Because every time Looking back there was no such problem before: using class components we have componentDidMount() {
setInterval(
() => { console.log(this.state) } // "unboxing" state gives actual value every time
, 1000)
} So how it could be solved? The first approach is pretty straight. We need something that will hold the reference to const [state, setState] = useState()
const stateRef = useRef(state)
useEffect({
const id = setInterval(
() => { console.log(stateRef.current) } // still logs "0" every time
, 1000)
return () => clearInterval(id)
}, []) // no state here Now we have to update this reference every time state changes. const stateRef = useRef(state)
useEffect(
() => stateRef.current = state,
[state]
)
useEffect({
const id = setInterval(
() => { console.log(stateRef.current) } // works nicely
, 1000)
return () => clearInterval(id)
}, []) // neither state nor stateRef here Ok, now we've got working solution. Is everything fine? Almost. const stateRef = useRef(state)
useEffect(
() => stateRef.current = state,
[state]
)
useEffect({
const id = setInterval(
() => { console.log(stateRef.current === state) } // true
, 1000)
return () => clearInterval(id)
}, [])
console.log(stateRef.current === state) // true? only until setState(...) will be called anywhere So there are still issues. What if we suppose that mutating reference outside of effect is... acceptable? const stateRef = useRef(state)
stateRef.current = state;
useEffect({
const id = setInterval(
() => { console.log(stateRef.current === state) } // true
, 1000)
return () => clearInterval(id)
}, [])
console.log(stateRef.current === state) // obviously always true :) I could finish here, but i feel like this is not ideal, at least until react team told that this is ok. And there are at least two more idiomatic solutions :) The other possible solution is about turning original problem in the following way: what we actually want is two separate effects. One is timer tick, the other is tick body. const [tick, setTick] = useState(0)
useEffect(() => {
const id = setInterval(() => {
setTick(t => t+1)
}, 5000)
return () => clearInterval(id)
}, [])
useEffect(() => {
console.log(state) // always actual state
}, [tick]) // although state is not here In my eyes it looks very nice although it causes rerenders every 5 seconds. What would you prefer? :) |
@escaton Thanks for breaking that down for me! I'd be interested to hear from the React team if mutating the reference outside of the effect is acceptable in this case. I sure find it easier to quickly understand, but of course don't want to recommend an anti-pattern. Otherwise the last solution will probably work. Will think on this some more and see if I can get any input from them. |
Dan wrote a blogpost on this exact topic: https://overreacted.io/making-setinterval-declarative-with-react-hooks/ |
Of course |
Closing because explaining how to make setInternal declarative is too lengthy for the format of this blog. I'll leave that to Dan :) |
No description provided.