Merge pull request #1452 from rjmunro/rjmunro/fix-clock-accuracy

Author: jstarpl

packages/webui/jest.config.cjs

@@ -7,6 +7,7 @@ module.exports = {
 	globals: {},
 	moduleFileExtensions: ['js', 'ts', 'tsx'],
 	moduleNameMapper: {
+		'sha.js': 'sha.js',
 		'meteor/(.*)': '<rootDir>/src/meteor/$1',
 		'(.+)\\.js$': '$1',
 	},

packages/webui/src/client/lib/__tests__/lib.test.ts

@@ -0,0 +1,50 @@
+import { useCurrentTime } from '../lib' // Adjust the import path as needed
+import { act, renderHook } from '@testing-library/react'
+
+describe('useCurrentTime Hook', () => {
+	it('should return the current time in milliseconds initially', () => {
+		const initialTime = Date.now()
+		const { result } = renderHook(() => useCurrentTime())
+		expect(result.current).toBeGreaterThanOrEqual(initialTime)
+		expect(result.current).toBeLessThanOrEqual(initialTime + 50) // Allow for a small margin
+	})
+
+	it('should update the time after the default refresh period (1000ms)', () => {
+		jest.useFakeTimers() // Enable Jest's fake timers
+		const { result } = renderHook(() => useCurrentTime())
+		const initialTime = result.current
+
+		act(() => {
+			jest.advanceTimersByTime(1000) // Advance the timers by the default refresh period
+		})
+
+		const nextSecond = Math.ceil((initialTime + 1) / 1000) * 1000 // Round to the next second
+		expect(result.current).toBeGreaterThanOrEqual(nextSecond)
+		expect(result.current).toBeLessThanOrEqual(nextSecond + 50) // Allow for a small margin
+	})
+
+	it('should update the time after a custom refresh period', () => {
+		jest.useFakeTimers() // Enable Jest's fake timers
+		const refreshPeriod = 500
+		const { result } = renderHook(() => useCurrentTime(refreshPeriod))
+		const initialTime = result.current
+
+		act(() => {
+			jest.advanceTimersByTime(refreshPeriod)
+		})
+
+		const nextInterval = Math.ceil((initialTime + 1) / refreshPeriod) * refreshPeriod // Round to the next refreshPeriod
+		expect(result.current).toBeGreaterThanOrEqual(nextInterval - 50)
+		expect(result.current).toBeLessThanOrEqual(nextInterval + 50) // Allow for a small margin
+	})
+
+	it('should clear the timeout on unmount', () => {
+		const { unmount } = renderHook(() => useCurrentTime())
+		const mockClearTimeout = jest.spyOn(global, 'clearTimeout')
+
+		unmount()
+
+		expect(mockClearTimeout).toHaveBeenCalledTimes(1)
+		// You could also assert that the timeout ID stored in the ref was passed to clearTimeout
+	})
+})

packages/webui/src/client/lib/lib.tsx

@@ -178,16 +178,33 @@ export function useDebounce<K>(value: K, delay: number, shouldUpdate?: (oldVal:
 	return debouncedValue
 }
 
+/**
+ * This hook returns the current time and updates it at the specified refresh period.
+ * It uses a timeout to schedule the next update based on the refresh period.
+ * The time is calculated as the current time in milliseconds since the Unix epoch.
+ * The hook also cleans up the timeout when the component using it unmounts.
+ *
+ * @param refreshPeriod - The period in milliseconds at which the current time should be refreshed.
+ * @returns The current time in milliseconds since the Unix epoch.
+ */
 export function useCurrentTime(refreshPeriod = 1000): number {
 	const [time, setTime] = useState(getCurrentTime())
+	const timeoutId = useRef<ReturnType<typeof setTimeout> | null>(null)
 
 	useEffect(() => {
-		const interval = setInterval(() => {
-			setTime(getCurrentTime())
-		}, refreshPeriod)
+		function updateTime() {
+			const current = getCurrentTime()
+			setTime(current)
+			const delay = refreshPeriod - (current % refreshPeriod)
+			timeoutId.current = setTimeout(updateTime, delay)
+		}
+
+		updateTime() // Initial call to start the timer
 
 		return () => {
-			clearInterval(interval)
+			if (timeoutId.current) {
+				clearTimeout(timeoutId.current) // Cleanup on unmount
+			}
 		}
 	}, [refreshPeriod])