fix(mobile): add correct dependencies in hooks

vydimitrov · Aug 29, 2020 · 9d77921 · 9d77921
1 parent 288320f
commit 9d77921
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 6 deletions.
diff --git a/packages/mobile/__tests__/TimeWrapper.test.js b/packages/mobile/__tests__/TimeWrapper.test.js
@@ -9,7 +9,7 @@ Math.random = () => 0.124578
 
 const fixture = {
   durationMilliseconds: 10000,
-  animatedElapsedTime: { addListener: () => {} },
+  animatedElapsedTime: { addListener: jest.fn(), removeListener: jest.fn() },
   renderAriaTime: ({ remainingTime }) => remainingTime,
   animatedColor: {},
   children: ({ remainingTime }) => <Text>{remainingTime}</Text>,
@@ -40,6 +40,7 @@ describe('behaviour tests', () => {
   it('should pass the new remaining time to the children when new value is provided by the listener', () => {
     let listener
     const animatedElapsedTime = {
+      removeListener: jest.fn(),
       addListener: (cb) => {
         listener = cb
       },

diff --git a/packages/mobile/src/components/TimeWrapper.jsx b/packages/mobile/src/components/TimeWrapper.jsx
@@ -27,14 +27,18 @@ const TimeWrapper = (props) => {
   })
 
   useEffect(() => {
-    animatedElapsedTime.addListener(({ value }) => {
+    const animatedListenerId = animatedElapsedTime.addListener(({ value }) => {
       setTimeProps({
         remainingTime: Math.ceil((durationMilliseconds - value) / 1000),
         elapsedTime: value,
         animatedColor,
       })
     })
-  }, [])
+
+    return () => {
+      animatedElapsedTime.removeListener(animatedListenerId)
+    }
+  }, [animatedElapsedTime, animatedColor, durationMilliseconds])
 
   return (
     <>

diff --git a/packages/mobile/src/hooks/useCountdown.js b/packages/mobile/src/hooks/useCountdown.js
@@ -67,7 +67,7 @@ export const useCountdown = ({
       animatedElapsedTime.removeAllListeners()
       clearTimeout(repeatTimeoutRef.current)
     }
-  }, [])
+  }, [animatedElapsedTime, startAt])
 
   // toggle playing effect
   useEffect(() => {
@@ -81,7 +81,7 @@ export const useCountdown = ({
         if (finished && elapsedTime.current === durationMilliseconds) {
           setIsProgressPathVisible(false)
           if (typeof onComplete === 'function') {
-            totalElapsedTime.current += duration
+            totalElapsedTime.current += durationMilliseconds / 1000
 
             const [shouldRepeat = false, delay = 0] =
               onComplete(totalElapsedTime.current) || []
@@ -106,7 +106,7 @@ export const useCountdown = ({
     return () => {
       animatedElapsedTime.stopAnimation()
     }
-  }, [isPlaying])
+  }, [isPlaying, animatedElapsedTime, durationMilliseconds, onComplete])
 
   return {
     path,