diff --git a/package-lock.json b/package-lock.json index 6ad72ec989..e5f9b4a07d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25341,6 +25341,7 @@ "@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap", "@deephaven/jsapi-types": "file:../jsapi-types", "@deephaven/log": "file:../log", + "@deephaven/react-hooks": "file:../react-hooks", "@deephaven/storage": "file:../storage", "@deephaven/utils": "file:../utils", "@fortawesome/react-fontawesome": "^0.2.0", @@ -27257,6 +27258,7 @@ "@deephaven/jsapi-types": "file:../jsapi-types", "@deephaven/log": "file:../log", "@deephaven/mocks": "file:../mocks", + "@deephaven/react-hooks": "file:../react-hooks", "@deephaven/storage": "file:../storage", "@deephaven/utils": "file:../utils", "@fortawesome/react-fontawesome": "^0.2.0", diff --git a/packages/console/package.json b/packages/console/package.json index d62261b932..977a3f911f 100644 --- a/packages/console/package.json +++ b/packages/console/package.json @@ -31,6 +31,7 @@ "@deephaven/log": "file:../log", "@deephaven/storage": "file:../storage", "@deephaven/utils": "file:../utils", + "@deephaven/react-hooks": "file:../react-hooks", "@fortawesome/react-fontawesome": "^0.2.0", "classnames": "^2.3.1", "linkifyjs": "^4.1.0", diff --git a/packages/console/src/HeapUsage.tsx b/packages/console/src/HeapUsage.tsx index 303ce7e3f8..793a7a3cb7 100644 --- a/packages/console/src/HeapUsage.tsx +++ b/packages/console/src/HeapUsage.tsx @@ -1,9 +1,10 @@ -import React, { useEffect, useState, ReactElement, useRef } from 'react'; +import { useState, ReactElement, useRef, useCallback } from 'react'; import classNames from 'classnames'; import { Tooltip } from '@deephaven/components'; import type { QueryConnectable } from '@deephaven/jsapi-types'; import { Plot, ChartTheme } from '@deephaven/chart'; import Log from '@deephaven/log'; +import { useAsyncInterval } from '@deephaven/react-hooks'; import './HeapUsage.scss'; const log = Log.module('HeapUsage'); @@ -38,55 +39,39 @@ function HeapUsage({ usages: [], }); - useEffect( - function setUsageUpdateInterval() { - const fetchAndUpdate = async () => { - try { - const newUsage = await connection.getWorkerHeapInfo(); - setMemoryUsage(newUsage); - - if (bgMonitoring || isOpen) { - const currentUsage = - (newUsage.totalHeapSize - newUsage.freeMemory) / - newUsage.maximumHeapSize; - const currentTime = Date.now(); - - const { timestamps, usages } = historyUsage.current; - while ( - timestamps.length !== 0 && - currentTime - timestamps[0] > monitorDuration * 1.5 - ) { - timestamps.shift(); - usages.shift(); - } - - timestamps.push(currentTime); - usages.push(currentUsage); - } else { - historyUsage.current = { timestamps: [], usages: [] }; - } - } catch (e) { - log.warn('Unable to get heap usage', e); + const setUsageUpdateInterval = useCallback(async () => { + try { + const newUsage = await connection.getWorkerHeapInfo(); + setMemoryUsage(newUsage); + + if (bgMonitoring || isOpen) { + const currentUsage = + (newUsage.totalHeapSize - newUsage.freeMemory) / + newUsage.maximumHeapSize; + const currentTime = Date.now(); + + const { timestamps, usages } = historyUsage.current; + while ( + timestamps.length !== 0 && + currentTime - timestamps[0] > monitorDuration * 1.5 + ) { + timestamps.shift(); + usages.shift(); } - }; - fetchAndUpdate(); - - const updateUsage = setInterval( - fetchAndUpdate, - isOpen ? hoverUpdateInterval : defaultUpdateInterval - ); - return () => { - clearInterval(updateUsage); - }; - }, - [ - isOpen, - hoverUpdateInterval, - connection, - defaultUpdateInterval, - monitorDuration, - bgMonitoring, - ] + + timestamps.push(currentTime); + usages.push(currentUsage); + } else { + historyUsage.current = { timestamps: [], usages: [] }; + } + } catch (e) { + log.warn('Unable to get heap usage', e); + } + }, [isOpen, connection, monitorDuration, bgMonitoring]); + + useAsyncInterval( + setUsageUpdateInterval, + isOpen ? hoverUpdateInterval : defaultUpdateInterval ); const toDecimalPlace = (num: number, dec: number) => diff --git a/packages/react-hooks/src/index.ts b/packages/react-hooks/src/index.ts index af844dcf58..70f7c32b95 100644 --- a/packages/react-hooks/src/index.ts +++ b/packages/react-hooks/src/index.ts @@ -1,8 +1,10 @@ +export * from './useAsyncInterval'; export { default as useContextOrThrow } from './useContextOrThrow'; export { default as usePrevious } from './usePrevious'; export { default as useForwardedRef } from './useForwardedRef'; export { default as useCopyToClipboard } from './useCopyToClipboard'; export { default as useFormWithDetachedSubmitButton } from './useFormWithDetachedSubmitButton'; +export * from './useIsMountedRef'; export { default as usePromiseFactory } from './usePromiseFactory'; export type { UseFormWithDetachedSubmitButtonResult } from './useFormWithDetachedSubmitButton'; export type { diff --git a/packages/react-hooks/src/useAsyncInterval.test.ts b/packages/react-hooks/src/useAsyncInterval.test.ts new file mode 100644 index 0000000000..300098526f --- /dev/null +++ b/packages/react-hooks/src/useAsyncInterval.test.ts @@ -0,0 +1,135 @@ +import { renderHook, act } from '@testing-library/react-hooks'; +import { TestUtils } from '@deephaven/utils'; +import useAsyncInterval from './useAsyncInterval'; + +beforeEach(() => { + jest.clearAllMocks(); + expect.hasAssertions(); + jest.useFakeTimers(); + jest.spyOn(window, 'setTimeout').mockName('setTimeout'); +}); + +afterAll(() => { + jest.useRealTimers(); +}); + +describe('useAsyncInterval', () => { + function createCallback(ms: number) { + return jest.fn( + async (): Promise => + new Promise(resolve => { + setTimeout(resolve, ms); + }) + ); + } + + const targetIntervalMs = 1000; + + it('should call the callback function after the target interval', async () => { + const callback = createCallback(50); + + renderHook(() => useAsyncInterval(callback, targetIntervalMs)); + + // First tick should be scheduled for target interval + expect(callback).not.toHaveBeenCalled(); + expect(window.setTimeout).toHaveBeenCalledWith( + expect.any(Function), + targetIntervalMs + ); + + // Callback should be called after target interval + act(() => jest.advanceTimersByTime(targetIntervalMs)); + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('should adjust the target interval based on how long async call takes', async () => { + const callbackDelayMs = 50; + const callback = createCallback(callbackDelayMs); + + renderHook(() => useAsyncInterval(callback, targetIntervalMs)); + + // Callback should be called after target interval + expect(callback).not.toHaveBeenCalled(); + act(() => jest.advanceTimersByTime(targetIntervalMs)); + expect(callback).toHaveBeenCalledTimes(1); + + jest.clearAllMocks(); + + // Mimick the callback Promise resolving + act(() => jest.advanceTimersByTime(callbackDelayMs)); + await TestUtils.flushPromises(); + + // Next target interval should be adjusted based on how long the callback took + const nextTargetIntervalMs = targetIntervalMs - callbackDelayMs; + + expect(callback).not.toHaveBeenCalled(); + expect(window.setTimeout).toHaveBeenCalledTimes(1); + expect(window.setTimeout).toHaveBeenCalledWith( + expect.any(Function), + nextTargetIntervalMs + ); + + act(() => jest.advanceTimersByTime(nextTargetIntervalMs)); + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('should schedule the next callback immediately if the callback takes longer than the target interval', async () => { + const callbackDelayMs = targetIntervalMs + 50; + const callback = createCallback(callbackDelayMs); + + renderHook(() => useAsyncInterval(callback, targetIntervalMs)); + + // Callback should be called after target interval + expect(callback).not.toHaveBeenCalled(); + act(() => jest.advanceTimersByTime(targetIntervalMs)); + expect(callback).toHaveBeenCalledTimes(1); + + jest.clearAllMocks(); + + // Mimick the callback Promise resolving + act(() => jest.advanceTimersByTime(callbackDelayMs)); + await TestUtils.flushPromises(); + + expect(callback).not.toHaveBeenCalled(); + expect(window.setTimeout).toHaveBeenCalledTimes(1); + expect(window.setTimeout).toHaveBeenCalledWith(expect.any(Function), 0); + + act(() => jest.advanceTimersByTime(0)); + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('should stop calling the callback function after unmounting', async () => { + const callback = createCallback(50); + + const { unmount } = renderHook(() => + useAsyncInterval(callback, targetIntervalMs) + ); + + unmount(); + + act(() => jest.advanceTimersByTime(targetIntervalMs)); + + expect(callback).not.toHaveBeenCalled(); + }); + + it('should not re-schedule callback if callback resolves after unmounting', async () => { + const callbackDelayMs = 50; + const callback = createCallback(callbackDelayMs); + + const { unmount } = renderHook(() => + useAsyncInterval(callback, targetIntervalMs) + ); + + act(() => jest.advanceTimersByTime(targetIntervalMs)); + expect(callback).toHaveBeenCalledTimes(1); + jest.clearAllMocks(); + + unmount(); + + // Mimick the callback Promise resolving + act(() => jest.advanceTimersByTime(callbackDelayMs)); + await TestUtils.flushPromises(); + + expect(window.setTimeout).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/react-hooks/src/useAsyncInterval.ts b/packages/react-hooks/src/useAsyncInterval.ts new file mode 100644 index 0000000000..edeb769028 --- /dev/null +++ b/packages/react-hooks/src/useAsyncInterval.ts @@ -0,0 +1,78 @@ +import { useCallback, useEffect, useRef } from 'react'; +import Log from '@deephaven/log'; +import { useIsMountedRef } from './useIsMountedRef'; + +const log = Log.module('useAsyncInterval'); + +/** + * Calls the given async callback at a target interval. + * + * If the callback takes less time than the target interval, the target interval + * for the next tick will be adjusted to target the remaining time in the current + * interval. + * + * e.g. If the target interval is 1000ms, and the callback takes 50ms to resolve, + * the next tick will be scheduled for 950ms from now via `setTimeout(callback, 950)`. + * + * If the callback takes longer than the target interval, the next tick will be + * scheduled immediately via `setTimeout(callback, 0)`. In such cases, the time + * between ticks may be > than the target interval, but this guarantees that + * a callback won't be scheduled until after the previous one has resolved. + * @param callback Callback to call at the target interval + * @param targetIntervalMs Target interval in milliseconds to call the callback + */ +export function useAsyncInterval( + callback: () => Promise, + targetIntervalMs: number +) { + const isMountedRef = useIsMountedRef(); + const trackingRef = useRef({ count: 0, started: Date.now() }); + const setTimeoutRef = useRef(0); + + const tick = useCallback(async () => { + const now = Date.now(); + let elapsedSinceLastTick = now - trackingRef.current.started; + + trackingRef.current.count += 1; + trackingRef.current.started = now; + + log.debug( + `tick #${trackingRef.current.count}.`, + elapsedSinceLastTick, + 'ms elapsed since last tick.' + ); + + await callback(); + + if (!isMountedRef.current) { + return; + } + + elapsedSinceLastTick += Date.now() - trackingRef.current.started; + + // If elapsed time is > than the target interval, adjust the next tick interval + const nextTickInterval = Math.max( + 0, + Math.min( + targetIntervalMs, + targetIntervalMs - (elapsedSinceLastTick - targetIntervalMs) + ) + ); + + log.debug('adjusted minIntervalMs:', nextTickInterval); + + setTimeoutRef.current = window.setTimeout(tick, nextTickInterval); + }, [callback, isMountedRef, targetIntervalMs]); + + useEffect(() => { + log.debug('Setting interval minIntervalMs:', targetIntervalMs); + + setTimeoutRef.current = window.setTimeout(tick, targetIntervalMs); + + return () => { + window.clearTimeout(setTimeoutRef.current); + }; + }, [targetIntervalMs, tick]); +} + +export default useAsyncInterval; diff --git a/packages/react-hooks/src/useIsMountedRef.test.ts b/packages/react-hooks/src/useIsMountedRef.test.ts new file mode 100644 index 0000000000..d5ed204187 --- /dev/null +++ b/packages/react-hooks/src/useIsMountedRef.test.ts @@ -0,0 +1,19 @@ +import { renderHook } from '@testing-library/react-hooks'; +import { useIsMountedRef } from './useIsMountedRef'; + +beforeEach(() => { + jest.clearAllMocks(); + expect.hasAssertions(); +}); + +describe('useIsMountedRef', () => { + it('should return a ref which tracks whether the component is mounted or not', () => { + const { result, unmount } = renderHook(() => useIsMountedRef()); + + expect(result.current.current).toBe(true); + + unmount(); + + expect(result.current.current).toBe(false); + }); +}); diff --git a/packages/react-hooks/src/useIsMountedRef.ts b/packages/react-hooks/src/useIsMountedRef.ts new file mode 100644 index 0000000000..22ffc634e7 --- /dev/null +++ b/packages/react-hooks/src/useIsMountedRef.ts @@ -0,0 +1,20 @@ +import { useEffect, useRef } from 'react'; + +/** + * Returns a ref which tracks whether the component is mounted or not. + */ +export function useIsMountedRef() { + const isMountedRef = useRef(false); + + useEffect(() => { + isMountedRef.current = true; + + return () => { + isMountedRef.current = false; + }; + }, []); + + return isMountedRef; +} + +export default useIsMountedRef; diff --git a/packages/utils/src/TestUtils.ts b/packages/utils/src/TestUtils.ts index a87cfc2635..0bfa9b1a81 100644 --- a/packages/utils/src/TestUtils.ts +++ b/packages/utils/src/TestUtils.ts @@ -43,6 +43,13 @@ export type PickMethods = { export type ConsoleMethodName = keyof PickMethods; class TestUtils { + /** + * jest.useFakeTimers mocks `process.nextTick` by default. Hold on to a + * reference to the real function so we can still use it. + */ + static realNextTick = + typeof process !== 'undefined' ? process.nextTick : undefined; + /** * Type assertion to "cast" a function to it's corresponding jest.Mock * function type. Note that this is a types only helper for type assertions. @@ -158,6 +165,23 @@ class TestUtils { } } + /** + * Jest doesn't have a built in way to ensure native Promises have resolved + * when using fake timers. We can mimic this behavior by using `process.nextTick`. + * Since `process.nextTick` is mocked by default when using jest.useFakeTimers(), + * we use the "real" process.nextTick stored in `TestUtils.realNextTick`. + * + * NOTE: Jest can be configured to leave `process.nextTick` unmocked, but this + * requires devs to configure it on every test. + * e.g. + * jest.useFakeTimers({ + * doNotFake: ['nextTick'], + * }); + */ + static async flushPromises(): Promise { + await new Promise(TestUtils.realNextTick ?? (() => undefined)); + } + static async rightClick( user: ReturnType, element: Element