From ce5458332aae9dbab8223c144ea6b4d4cfe0a5d1 Mon Sep 17 00:00:00 2001 From: Marcos Moura Date: Tue, 22 Aug 2023 16:04:19 +0200 Subject: [PATCH 1/3] feat(react-utilities): create useAnimationFrame hook --- ...-8a73f07d-6553-402e-b292-938e7141264a.json | 7 ++ .../etc/react-utilities.api.md | 5 +- .../react-utilities/src/hooks/index.ts | 1 + .../src/hooks/useAnimationFrame.test.ts | 106 ++++++++++++++++++ .../src/hooks/useAnimationFrame.ts | 14 +++ .../src/hooks/useBrowserTimer.test.ts | 61 ++++++++++ .../src/hooks/useBrowserTimer.ts | 47 ++++++++ .../react-utilities/src/hooks/useTimeout.ts | 26 +---- .../react-utilities/src/index.ts | 1 + 9 files changed, 246 insertions(+), 22 deletions(-) create mode 100644 change/@fluentui-react-utilities-8a73f07d-6553-402e-b292-938e7141264a.json create mode 100644 packages/react-components/react-utilities/src/hooks/useAnimationFrame.test.ts create mode 100644 packages/react-components/react-utilities/src/hooks/useAnimationFrame.ts create mode 100644 packages/react-components/react-utilities/src/hooks/useBrowserTimer.test.ts create mode 100644 packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts diff --git a/change/@fluentui-react-utilities-8a73f07d-6553-402e-b292-938e7141264a.json b/change/@fluentui-react-utilities-8a73f07d-6553-402e-b292-938e7141264a.json new file mode 100644 index 0000000000000..f21e13631a15d --- /dev/null +++ b/change/@fluentui-react-utilities-8a73f07d-6553-402e-b292-938e7141264a.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "feat: create a new useAnimationFrame hook", + "packageName": "@fluentui/react-utilities", + "email": "marcosvmmoura@gmail.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-utilities/etc/react-utilities.api.md b/packages/react-components/react-utilities/etc/react-utilities.api.md index 0c237a9665b02..832fd9488e27f 100644 --- a/packages/react-components/react-utilities/etc/react-utilities.api.md +++ b/packages/react-components/react-utilities/etc/react-utilities.api.md @@ -294,6 +294,9 @@ export type UnknownSlotProps = Pick, 'childr as?: keyof JSX.IntrinsicElements; }; +// @internal +export function useAnimationFrame(): readonly [(fn: () => void, delay?: number | undefined) => number, () => void]; + // @internal export const useControllableState: (options: UseControllableStateOptions) => [State, React_2.Dispatch>]; @@ -350,7 +353,7 @@ export function useScrollbarWidth(options: UseScrollbarWidthOptions): number | u export function useSelection(params: SelectionHookParams): readonly [Set, SelectionMethods]; // @internal -export function useTimeout(): readonly [(fn: () => void, delay: number) => void, () => void]; +export function useTimeout(): readonly [(fn: () => void, delay?: number | undefined) => number, () => void]; // (No @packageDocumentation comment for this package) diff --git a/packages/react-components/react-utilities/src/hooks/index.ts b/packages/react-components/react-utilities/src/hooks/index.ts index e5928f400aa78..018e521c02bb6 100644 --- a/packages/react-components/react-utilities/src/hooks/index.ts +++ b/packages/react-components/react-utilities/src/hooks/index.ts @@ -1,3 +1,4 @@ +export * from './useAnimationFrame'; export * from './useControllableState'; export * from './useEventCallback'; export * from './useFirstMount'; diff --git a/packages/react-components/react-utilities/src/hooks/useAnimationFrame.test.ts b/packages/react-components/react-utilities/src/hooks/useAnimationFrame.test.ts new file mode 100644 index 0000000000000..307eddec185bc --- /dev/null +++ b/packages/react-components/react-utilities/src/hooks/useAnimationFrame.test.ts @@ -0,0 +1,106 @@ +import { renderHook } from '@testing-library/react-hooks'; +import { useAnimationFrame } from './useAnimationFrame'; + +describe('useAnimationFrame', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('calls the callback only on the next frame', () => { + const [setTestRequestAnimationFrame] = renderHook(() => useAnimationFrame()).result.current; + const callback = jest.fn(); + + setTestRequestAnimationFrame(callback); + + expect(callback).not.toHaveBeenCalled(); + + jest.runAllTimers(); + + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('does not call the callback if cancel is called', () => { + const [setTestRequestAnimationFrame, cancelTEstRequestAnimationFrame] = renderHook(() => useAnimationFrame()).result + .current; + const callback = jest.fn(); + + setTestRequestAnimationFrame(callback); + cancelTEstRequestAnimationFrame(); + + jest.runAllTimers(); + + expect(callback).not.toHaveBeenCalled(); + }); + + it('cancel the previous requestAnimationFrame if set is called again', () => { + const [setTestRequestAnimationFrame] = renderHook(() => useAnimationFrame()).result.current; + const callbackA = jest.fn(); + const callbackB = jest.fn(); + + setTestRequestAnimationFrame(callbackA); + setTestRequestAnimationFrame(callbackB); + + jest.runAllTimers(); + + expect(callbackA).not.toHaveBeenCalled(); + expect(callbackB).toHaveBeenCalledTimes(1); + }); + + it('allows another requestAnimationFrame to be set after the previous has run', () => { + const [setTestRequestAnimationFrame] = renderHook(() => useAnimationFrame()).result.current; + const callbackA = jest.fn(); + const callbackB = jest.fn(); + + setTestRequestAnimationFrame(callbackA); + + jest.runAllTimers(); + + setTestRequestAnimationFrame(callbackB); + + jest.runAllTimers(); + + expect(callbackA).toHaveBeenCalledTimes(1); + expect(callbackB).toHaveBeenCalledTimes(1); + }); + + it('does not cancel the requestAnimationFrame between renders', () => { + const { result, rerender } = renderHook(() => useAnimationFrame()); + const [setTestRequestAnimationFrame] = result.current; + const callback = jest.fn(); + + setTestRequestAnimationFrame(callback); + + rerender(); + + jest.runAllTimers(); + + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('cancel the requestAnimationFrame when the component is unmounted', () => { + const { result, unmount } = renderHook(() => useAnimationFrame()); + const [setTestRequestAnimationFrame] = result.current; + const callback = jest.fn(); + + setTestRequestAnimationFrame(callback); + + unmount(); + + jest.runAllTimers(); + + expect(callback).not.toHaveBeenCalled(); + }); + + it('returns the same functions every render', () => { + const { result, rerender } = renderHook(() => useAnimationFrame()); + const [setTestRequestAnimationFrame, cancelTEstRequestAnimationFrame] = result.current; + + rerender(); + + expect(result.current).toStrictEqual([setTestRequestAnimationFrame, cancelTEstRequestAnimationFrame]); + }); +}); diff --git a/packages/react-components/react-utilities/src/hooks/useAnimationFrame.ts b/packages/react-components/react-utilities/src/hooks/useAnimationFrame.ts new file mode 100644 index 0000000000000..f2453bd545a4d --- /dev/null +++ b/packages/react-components/react-utilities/src/hooks/useAnimationFrame.ts @@ -0,0 +1,14 @@ +import { useBrowserTimer } from './useBrowserTimer'; + +/** + * @internal + * Helper to manage a browser requestAnimationFrame. + * Ensures that the requestAnimationFrame isn't set multiple times at once and is cleaned up + * when the component is unloaded. + * + * @returns A pair of [requestAnimationFrame, cancelAnimationFrame] that are stable between renders. + */ +export function useAnimationFrame() { + // TODO: figure it out a way to not call global.requestAnimationFrame and instead infer window from some context + return useBrowserTimer(requestAnimationFrame, cancelAnimationFrame); +} diff --git a/packages/react-components/react-utilities/src/hooks/useBrowserTimer.test.ts b/packages/react-components/react-utilities/src/hooks/useBrowserTimer.test.ts new file mode 100644 index 0000000000000..26600feb7262b --- /dev/null +++ b/packages/react-components/react-utilities/src/hooks/useBrowserTimer.test.ts @@ -0,0 +1,61 @@ +import { renderHook } from '@testing-library/react-hooks'; +import { useBrowserTimer } from './useBrowserTimer'; + +const setTimer = jest.fn((callback: jest.Func) => { + callback(); + return 0; +}); + +const cancelTimer = jest.fn(() => 0); + +describe('useBrowserTimer', () => { + it('should return array with functions', () => { + const hookValues = renderHook(() => useBrowserTimer(setTimer, cancelTimer)).result.current; + + expect(hookValues).toHaveLength(2); + expect(typeof hookValues[0]).toBe('function'); + expect(typeof hookValues[1]).toBe('function'); + expect(hookValues[0].name).toBe('set'); + expect(hookValues[1].name).toBe('cancel'); + }); + + it('calls the setter only n times', () => { + const [setTestTimer] = renderHook(() => useBrowserTimer(setTimer, cancelTimer)).result.current; + const callback = jest.fn(); + + setTestTimer(callback); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).not.toHaveBeenCalledTimes(2); + expect(setTimer).toHaveBeenCalledTimes(1); + expect(setTimer).not.toHaveBeenCalledTimes(2); + }); + + it('setter should return timer id', () => { + const [setTestTimer] = renderHook(() => useBrowserTimer(setTimer, cancelTimer)).result.current; + const callback = jest.fn(); + + const timerId = setTestTimer(callback); + + expect(callback).toHaveBeenCalledTimes(1); + expect(timerId).toBe(0); + }); + + it('should not call the cancel callback if not setter was called', () => { + const [, cancelTestTimer] = renderHook(() => useBrowserTimer(setTimer, cancelTimer)).result.current; + + cancelTestTimer(); + + expect(cancelTimer).not.toHaveBeenCalledTimes(1); + }); + + it('calls the cancel only n times', () => { + const [setTestTimer, cancelTestTimer] = renderHook(() => useBrowserTimer(setTimer, cancelTimer)).result.current; + + setTestTimer(jest.fn()); + cancelTestTimer(); + + expect(cancelTimer).toHaveBeenCalledTimes(1); + expect(cancelTimer).not.toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts b/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts new file mode 100644 index 0000000000000..5422ca5f79561 --- /dev/null +++ b/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts @@ -0,0 +1,47 @@ +import * as React from 'react'; + +type UseBrowserTimerSetter = + | ((fn: () => void, duration?: number, ...args: Record[]) => number) + | ((fn: () => void) => number); +type UseBrowserTimerCancel = ((timerId: number) => void) | (() => void); + +/** + * @internal + * Helper to manage a browser timer. + * Ensures that the timer isn't set multiple times at once, + * and is cleaned up when the component is unloaded. + * + * @param setTimer - The timer setter function + * @param cancelTimer - The timer cancel function + * @returns A pair of [setTimer, cancelTimer] that are stable between renders. + * + * @example + * const [setTimer, cancelTimer] = useBrowserTimer(setTimeout, cancelTimeout); + * + * setTimer(() => console.log('Hello world!'), 1000); + * cancelTimer(); + */ +export function useBrowserTimer( + setTimer: TSetter, + cancelTimer: TCancel, +) { + const [timeout] = React.useState(() => ({ + id: undefined as number | undefined, + set: (fn: () => void, delay?: number) => { + timeout.cancel(); + timeout.id = delay ? setTimer(fn, delay) : setTimer(fn); + return timeout.id; + }, + cancel: () => { + if (timeout.id !== undefined) { + cancelTimer(timeout.id); + timeout.id = undefined; + } + }, + })); + + // Clean up the timeout when the component is unloaded + React.useEffect(() => timeout.cancel, [timeout]); + + return [timeout.set, timeout.cancel] as const; +} diff --git a/packages/react-components/react-utilities/src/hooks/useTimeout.ts b/packages/react-components/react-utilities/src/hooks/useTimeout.ts index 250c64fdf9b9d..36c0813ef2107 100644 --- a/packages/react-components/react-utilities/src/hooks/useTimeout.ts +++ b/packages/react-components/react-utilities/src/hooks/useTimeout.ts @@ -1,30 +1,14 @@ -import * as React from 'react'; +import { useBrowserTimer } from './useBrowserTimer'; /** * @internal * Helper to manage a browser timeout. - * Ensures that the timeout isn't set multiple times at once, - * and is cleaned up when the component is unloaded. + * Ensures that the timeout isn't set multiple times at once and is cleaned up + * when the component is unloaded. * * @returns A pair of [setTimeout, clearTimeout] that are stable between renders. */ export function useTimeout() { - const [timeout] = React.useState(() => ({ - id: undefined as ReturnType | undefined, - set: (fn: () => void, delay: number) => { - timeout.clear(); - timeout.id = setTimeout(fn, delay); - }, - clear: () => { - if (timeout.id !== undefined) { - clearTimeout(timeout.id); - timeout.id = undefined; - } - }, - })); - - // Clean up the timeout when the component is unloaded - React.useEffect(() => timeout.clear, [timeout]); - - return [timeout.set, timeout.clear] as const; + // TODO: figure it out a way to not call global.setTimeout and instead infer window from some context + return useBrowserTimer(setTimeout, clearTimeout); } diff --git a/packages/react-components/react-utilities/src/index.ts b/packages/react-components/react-utilities/src/index.ts index ca628ac61ce07..97fc981ebeb74 100644 --- a/packages/react-components/react-utilities/src/index.ts +++ b/packages/react-components/react-utilities/src/index.ts @@ -30,6 +30,7 @@ export type { export { IdPrefixProvider, resetIdsForTests, + useAnimationFrame, useControllableState, useEventCallback, useFirstMount, From 6c08e5a35ce134338efb3592f952722b4fbc8495 Mon Sep 17 00:00:00 2001 From: Marcos Moura Date: Wed, 23 Aug 2023 16:51:36 +0200 Subject: [PATCH 2/3] fix: simplify typing and use refs instead of state --- .../src/hooks/useBrowserTimer.ts | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts b/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts index 5422ca5f79561..700ee620a6ae0 100644 --- a/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts +++ b/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts @@ -1,9 +1,8 @@ import * as React from 'react'; -type UseBrowserTimerSetter = +type BrowserTimerSetter = | ((fn: () => void, duration?: number, ...args: Record[]) => number) | ((fn: () => void) => number); -type UseBrowserTimerCancel = ((timerId: number) => void) | (() => void); /** * @internal @@ -21,27 +20,24 @@ type UseBrowserTimerCancel = ((timerId: number) => void) | (() => void); * setTimer(() => console.log('Hello world!'), 1000); * cancelTimer(); */ -export function useBrowserTimer( - setTimer: TSetter, - cancelTimer: TCancel, -) { - const [timeout] = React.useState(() => ({ - id: undefined as number | undefined, - set: (fn: () => void, delay?: number) => { - timeout.cancel(); - timeout.id = delay ? setTimer(fn, delay) : setTimer(fn); - return timeout.id; +export function useBrowserTimer(setTimer: BrowserTimerSetter, cancelTimer: (id: number) => void) { + const id = React.useRef(undefined); + const set = React.useCallback( + (fn: () => void, delay?: number) => { + id.current = setTimer(fn, delay); + return id.current; }, - cancel: () => { - if (timeout.id !== undefined) { - cancelTimer(timeout.id); - timeout.id = undefined; - } - }, - })); + [setTimer], + ); + const cancel = React.useCallback(() => { + if (id.current !== undefined) { + cancelTimer(id.current); + id.current = undefined; + } + }, [cancelTimer]); // Clean up the timeout when the component is unloaded - React.useEffect(() => timeout.cancel, [timeout]); + React.useEffect(() => cancel, [cancel]); - return [timeout.set, timeout.cancel] as const; + return [set, cancel] as const; } From 80eac6cebaf9d2eecb260a81df0169eacf625dba Mon Sep 17 00:00:00 2001 From: Marcos Moura Date: Wed, 23 Aug 2023 17:08:37 +0200 Subject: [PATCH 3/3] fix: cancel previous timer if set is called twice --- .../src/hooks/useAnimationFrame.test.ts | 11 +++-------- .../react-utilities/src/hooks/useBrowserTimer.test.ts | 8 ++++---- .../react-utilities/src/hooks/useBrowserTimer.ts | 8 +++++++- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/react-components/react-utilities/src/hooks/useAnimationFrame.test.ts b/packages/react-components/react-utilities/src/hooks/useAnimationFrame.test.ts index 307eddec185bc..23d721d6343cd 100644 --- a/packages/react-components/react-utilities/src/hooks/useAnimationFrame.test.ts +++ b/packages/react-components/react-utilities/src/hooks/useAnimationFrame.test.ts @@ -2,13 +2,8 @@ import { renderHook } from '@testing-library/react-hooks'; import { useAnimationFrame } from './useAnimationFrame'; describe('useAnimationFrame', () => { - beforeEach(() => { - jest.useFakeTimers(); - }); - - afterEach(() => { - jest.useRealTimers(); - }); + beforeEach(() => jest.useFakeTimers()); + afterEach(() => jest.useRealTimers()); it('calls the callback only on the next frame', () => { const [setTestRequestAnimationFrame] = renderHook(() => useAnimationFrame()).result.current; @@ -46,7 +41,7 @@ describe('useAnimationFrame', () => { jest.runAllTimers(); - expect(callbackA).not.toHaveBeenCalled(); + expect(callbackA).not.toHaveBeenCalledTimes(1); expect(callbackB).toHaveBeenCalledTimes(1); }); diff --git a/packages/react-components/react-utilities/src/hooks/useBrowserTimer.test.ts b/packages/react-components/react-utilities/src/hooks/useBrowserTimer.test.ts index 26600feb7262b..d42e1558104df 100644 --- a/packages/react-components/react-utilities/src/hooks/useBrowserTimer.test.ts +++ b/packages/react-components/react-utilities/src/hooks/useBrowserTimer.test.ts @@ -6,7 +6,9 @@ const setTimer = jest.fn((callback: jest.Func) => { return 0; }); -const cancelTimer = jest.fn(() => 0); +const cancelTimer = jest.fn(() => { + return; +}); describe('useBrowserTimer', () => { it('should return array with functions', () => { @@ -15,8 +17,6 @@ describe('useBrowserTimer', () => { expect(hookValues).toHaveLength(2); expect(typeof hookValues[0]).toBe('function'); expect(typeof hookValues[1]).toBe('function'); - expect(hookValues[0].name).toBe('set'); - expect(hookValues[1].name).toBe('cancel'); }); it('calls the setter only n times', () => { @@ -41,7 +41,7 @@ describe('useBrowserTimer', () => { expect(timerId).toBe(0); }); - it('should not call the cancel callback if not setter was called', () => { + it('should not call the cancel callback if no setter was called', () => { const [, cancelTestTimer] = renderHook(() => useBrowserTimer(setTimer, cancelTimer)).result.current; cancelTestTimer(); diff --git a/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts b/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts index 700ee620a6ae0..a6b527e60ee85 100644 --- a/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts +++ b/packages/react-components/react-utilities/src/hooks/useBrowserTimer.ts @@ -22,13 +22,19 @@ type BrowserTimerSetter = */ export function useBrowserTimer(setTimer: BrowserTimerSetter, cancelTimer: (id: number) => void) { const id = React.useRef(undefined); + const set = React.useCallback( (fn: () => void, delay?: number) => { + if (id.current !== undefined) { + cancelTimer(id.current); + } + id.current = setTimer(fn, delay); return id.current; }, - [setTimer], + [cancelTimer, setTimer], ); + const cancel = React.useCallback(() => { if (id.current !== undefined) { cancelTimer(id.current);