From c09f52a4d936208d1e3a54d072e67ee5ce7196ed Mon Sep 17 00:00:00 2001 From: Jonghyeon Ko Date: Fri, 27 Oct 2023 23:07:40 +0900 Subject: [PATCH] perf(react): remove unnecessary useSetTimeout's callback calling (#264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix #263 # Overview @ssi02014 Thanks for reporting important issue. Could you review this Pull Request too? I remove unnecessary useSetTimeout's callback calling and I added you as co-author in [79fecd9](https://github.com/suspensive/react/pull/264/commits/79fecd9a87a63e71cffd321d0e0e22208384e1c5) ## PR Checklist - [x] I did below actions if need 1. I read the [Contributing Guide](https://github.com/suspensive/react/blob/main/CONTRIBUTING.md) 2. I added documents and tests. --------- Co-authored-by: Gromit (전민재) --- .changeset/rare-chefs-behave.md | 5 ++++ configs/test-utils/src/ThrowError.tsx | 24 +++++++++++++------ packages/react/src/Delay.tsx | 4 ++-- packages/react/src/ErrorBoundary.spec.tsx | 8 +++---- packages/react/src/hooks/index.ts | 2 +- packages/react/src/hooks/useSetTimeout.ts | 7 ------ ...eSetTimeout.spec.ts => useTimeout.spec.ts} | 6 ++--- packages/react/src/hooks/useTimeout.ts | 15 ++++++++++++ 8 files changed, 47 insertions(+), 24 deletions(-) create mode 100644 .changeset/rare-chefs-behave.md delete mode 100644 packages/react/src/hooks/useSetTimeout.ts rename packages/react/src/hooks/{useSetTimeout.spec.ts => useTimeout.spec.ts} (82%) create mode 100644 packages/react/src/hooks/useTimeout.ts diff --git a/.changeset/rare-chefs-behave.md b/.changeset/rare-chefs-behave.md new file mode 100644 index 000000000..6760803c2 --- /dev/null +++ b/.changeset/rare-chefs-behave.md @@ -0,0 +1,5 @@ +--- +"@suspensive/react": patch +--- + +perf(react): remove unnecessary useSetTimeout's callback calling diff --git a/configs/test-utils/src/ThrowError.tsx b/configs/test-utils/src/ThrowError.tsx index 5e919c2b5..89858ddbc 100644 --- a/configs/test-utils/src/ThrowError.tsx +++ b/configs/test-utils/src/ThrowError.tsx @@ -1,11 +1,21 @@ import type { PropsWithChildren } from 'react' -import { useEffect, useState } from 'react' +import { useEffect, useLayoutEffect, useRef, useState } from 'react' + +const isClient = typeof window !== 'undefined' +export const useIsomorphicLayoutEffect = isClient ? useLayoutEffect : useEffect + +export const useTimeout = (fn: () => void, ms: number) => { + const fnRef = useRef(fn) + + useIsomorphicLayoutEffect(() => { + fnRef.current = fn + }, [fn]) -const useSetTimeout = (fn: (...args: []) => void, delay: number) => useEffect(() => { - const timeout = setTimeout(fn, delay) - return () => clearTimeout(timeout) - }, [fn, delay]) + const id = setTimeout(fnRef.current, ms) + return () => clearTimeout(id) + }, [ms]) +} const throwErrorIsNeed = { current: false } type ThrowErrorProps = PropsWithChildren<{ message: string; after?: number }> @@ -14,7 +24,7 @@ export const ThrowError = ({ message, after = 0, children }: ThrowErrorProps) => if (isNeedError) { throw new Error(message) } - useSetTimeout(() => setIsNeedError(true), after) + useTimeout(() => setIsNeedError(true), after) return <>{children} } @@ -24,7 +34,7 @@ export const ThrowNull = ({ after, children }: ThrowNullProps) => { if (isNeedError) { throw null } - useSetTimeout(() => setIsNeedError(true), after) + useTimeout(() => setIsNeedError(true), after) return <>{children} } diff --git a/packages/react/src/Delay.tsx b/packages/react/src/Delay.tsx index 12158de14..8db099133 100644 --- a/packages/react/src/Delay.tsx +++ b/packages/react/src/Delay.tsx @@ -1,6 +1,6 @@ import type { ComponentProps, ComponentType, PropsWithChildren } from 'react' import { createContext, useContext, useState } from 'react' -import { useSetTimeout } from './hooks' +import { useTimeout } from './hooks' import type { PropsWithoutChildren } from './types' export type DelayProps = PropsWithChildren<{ @@ -11,7 +11,7 @@ export const Delay = ({ ms, children }: DelayProps) => { const delayContextValue = useContext(DelayContext) const delayMs = ms ?? delayContextValue.ms ?? 0 const [isDelayed, setIsDelayed] = useState(delayMs === 0) - useSetTimeout(() => setIsDelayed(true), delayMs) + useTimeout(() => setIsDelayed(true), delayMs) return <>{isDelayed ? children : null} } diff --git a/packages/react/src/ErrorBoundary.spec.tsx b/packages/react/src/ErrorBoundary.spec.tsx index aa0c50aff..c5a2d18ff 100644 --- a/packages/react/src/ErrorBoundary.spec.tsx +++ b/packages/react/src/ErrorBoundary.spec.tsx @@ -4,7 +4,7 @@ import type { ComponentProps, ComponentRef } from 'react' import { createElement, createRef } from 'react' import { createRoot } from 'react-dom/client' import { vi } from 'vitest' -import { useSetTimeout } from './hooks' +import { useTimeout } from './hooks' import { assert } from './utils' import { ErrorBoundary, useErrorBoundary, useErrorBoundaryFallbackProps, withErrorBoundary } from '.' @@ -257,12 +257,12 @@ describe('useErrorBoundary', () => { onError, fallback: function ErrorBoundaryFallback() { const props = useErrorBoundaryFallbackProps() - useSetTimeout(props.reset, MS_100) + useTimeout(props.reset, MS_100) return <>{props.error.message} }, children: createElement(() => { const errorBoundary = useErrorBoundary() - useSetTimeout(() => errorBoundary.setError(new Error(ERROR_MESSAGE)), MS_100) + useTimeout(() => errorBoundary.setError(new Error(ERROR_MESSAGE)), MS_100) return <>{TEXT} }), }) @@ -320,7 +320,7 @@ describe('useErrorBoundaryFallbackProps', () => { onReset, fallback: function ErrorBoundaryFallback() { const props = useErrorBoundaryFallbackProps() - useSetTimeout(props.reset, MS_100) + useTimeout(props.reset, MS_100) return <>{props.error.message} }, diff --git a/packages/react/src/hooks/index.ts b/packages/react/src/hooks/index.ts index 825ac85ad..d13e6f8be 100644 --- a/packages/react/src/hooks/index.ts +++ b/packages/react/src/hooks/index.ts @@ -3,4 +3,4 @@ export { useIsClient } from './useIsClient' export { useIsomorphicLayoutEffect } from './useIsomorphicLayoutEffect' export { useKey } from './useKey' export { usePrevious } from './usePrevious' -export { useSetTimeout } from './useSetTimeout' +export { useTimeout } from './useTimeout' diff --git a/packages/react/src/hooks/useSetTimeout.ts b/packages/react/src/hooks/useSetTimeout.ts deleted file mode 100644 index 5fed960d1..000000000 --- a/packages/react/src/hooks/useSetTimeout.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { useEffect } from 'react' - -export const useSetTimeout = (fn: (...args: []) => void, delay: number) => - useEffect(() => { - const timeout = setTimeout(fn, delay) - return () => clearTimeout(timeout) - }, [fn, delay]) diff --git a/packages/react/src/hooks/useSetTimeout.spec.ts b/packages/react/src/hooks/useTimeout.spec.ts similarity index 82% rename from packages/react/src/hooks/useSetTimeout.spec.ts rename to packages/react/src/hooks/useTimeout.spec.ts index f18f8d453..e131c85cc 100644 --- a/packages/react/src/hooks/useSetTimeout.spec.ts +++ b/packages/react/src/hooks/useTimeout.spec.ts @@ -1,14 +1,14 @@ import { MS_100 } from '@suspensive/test-utils' import { act, renderHook } from '@testing-library/react' import { describe, expect, it, vi } from 'vitest' -import { useSetTimeout } from '.' +import { useTimeout } from '.' vi.useFakeTimers() -describe('useSetTimeout', () => { +describe('useTimeout', () => { it('should run given function once after given timeout', () => { const fn = vi.fn() - const rendered = renderHook(() => useSetTimeout(fn, MS_100)) + const rendered = renderHook(() => useTimeout(fn, MS_100)) expect(fn).toHaveBeenCalledTimes(0) act(() => vi.advanceTimersByTime(MS_100)) expect(fn).toHaveBeenCalledTimes(1) diff --git a/packages/react/src/hooks/useTimeout.ts b/packages/react/src/hooks/useTimeout.ts new file mode 100644 index 000000000..de7eeb2cb --- /dev/null +++ b/packages/react/src/hooks/useTimeout.ts @@ -0,0 +1,15 @@ +import { useEffect, useRef } from 'react' +import { useIsomorphicLayoutEffect } from './useIsomorphicLayoutEffect' + +export const useTimeout = (fn: () => void, ms: number) => { + const fnRef = useRef(fn) + + useIsomorphicLayoutEffect(() => { + fnRef.current = fn + }, [fn]) + + useEffect(() => { + const id = setTimeout(fnRef.current, ms) + return () => clearTimeout(id) + }, [ms]) +}