contiamo / operational-ui

Building blocks for effective operational interfaces
https://operational-ui.netlify.com
MIT License
258 stars 47 forks source link

useInterval leaks components #1333

Closed justinmchase closed 4 years ago

justinmchase commented 4 years ago

I'm pretty sure that this line is a leak: https://github.com/contiamo/operational-ui/blob/a912bc2645dec90a041e4f375c3e44f3fc1ddbb5/src/useInterval/index.ts#L38

Where its only clearing the interval if the delay changes, it should be:

useEffect(() => { ... }, [callback, delay])

Which would cause it to call clearInterval if the callback were to change.

I am using nextjs which has HMR and it can cause a component to get reloaded on a change, in this case the callback is updated with a new instance but the delay is the same. Now I have two intervals running, one in the UI and somewhere in memory. I can tell because I'm seeing multiple counts being logged at about the same time whenever I make a change.

justinmchase commented 4 years ago

Shoot never mind.. I'm an idiot. I had my page open in a second tab and didn't know it. LOL. But it does seem strange that callback isn't a parameter there still.