Skip to content

Commit

Permalink
perf(react): remove unnecessary useSetTimeout's callback calling (#264)
Browse files Browse the repository at this point in the history
fix #263 

# Overview

<!--
    A clear and concise description of what this pr is about.
 -->

@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](79fecd9)

## 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 (전민재) <[email protected]>
  • Loading branch information
manudeli and ssi02014 authored Oct 27, 2023
1 parent 3987632 commit e09f745
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 24 deletions.
5 changes: 5 additions & 0 deletions .changeset/rare-chefs-behave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@suspensive/react": patch
---

perf(react): remove unnecessary useSetTimeout's callback calling
24 changes: 17 additions & 7 deletions configs/test-utils/src/ThrowError.tsx
Original file line number Diff line number Diff line change
@@ -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 }>
Expand All @@ -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}</>
}

Expand All @@ -24,7 +34,7 @@ export const ThrowNull = ({ after, children }: ThrowNullProps) => {
if (isNeedError) {
throw null
}
useSetTimeout(() => setIsNeedError(true), after)
useTimeout(() => setIsNeedError(true), after)
return <>{children}</>
}

Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/Delay.tsx
Original file line number Diff line number Diff line change
@@ -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<{
Expand All @@ -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}</>
}

Expand Down
8 changes: 4 additions & 4 deletions packages/react/src/ErrorBoundary.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 '.'

Expand Down Expand Up @@ -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}</>
}),
})
Expand Down Expand Up @@ -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}</>
},
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
7 changes: 0 additions & 7 deletions packages/react/src/hooks/useSetTimeout.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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)
Expand Down
15 changes: 15 additions & 0 deletions packages/react/src/hooks/useTimeout.ts
Original file line number Diff line number Diff line change
@@ -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])
}

0 comments on commit e09f745

Please sign in to comment.