Skip to content

#703 Fix unstable options object in useDebounceCallback breaking debouncing #704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

#703 Fix unstable options object in useDebounceCallback breaking debouncing #704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
556bfcc
test: #703 Unstable options object: a failing test case
lolmaus Sep 5, 2025
159f567
fix: #703 Unstable options object in useDebounceCallback breaks debou…
lolmaus Sep 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions packages/usehooks-ts/src/useDebounceCallback/useDebounceCallback.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,30 @@ describe('useDebounceCallback()', () => {
// The callback should be invoked immediately after flushing
expect(debouncedCallback).toHaveBeenCalled()
})

it('should maintain debouncing across rerenders with an unstable options object', () => {
const delay = 500
const callback = vitest.fn()

const { result: debouncedCallback, rerender } = renderHook(() =>
useDebounceCallback(callback, delay, { leading: false }),
)

act(() => {
debouncedCallback.current('first')
})

vitest.advanceTimersByTime(200)

// Simulate a component rerender, which creates a new instance of the options object
rerender()

act(() => {
debouncedCallback.current('second')
})

vitest.advanceTimersByTime(350)

expect(callback).not.toHaveBeenCalled()
})
})
4 changes: 2 additions & 2 deletions packages/usehooks-ts/src/useDebounceCallback/useDebounceCallback.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,12 @@ export function useDebounceCallback<T extends (...args: any) => ReturnType<T>>(
}

return wrappedFunc
}, [func, delay, options])
}, [func, delay])

// Update the debounced function ref whenever func, wait, or options change
useEffect(() => {
debouncedFunc.current = debounce(func, delay, options)
}, [func, delay, options])
}, [func, delay])
Comment on lines -107 to +112
Copy link

@Pasalietis Pasalietis Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about instead of removing options changing it to memorization

const {leading, trailling, maxWait} = options
const memOptions = useMemo(() => ({leading, trailling, maxWait}), [leading, trailling, maxWait])

?

Copy link
Author

@lolmaus lolmaus Sep 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work.

Copy link
Author

@lolmaus lolmaus Sep 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, you suggest destructuring the options object and memoizing values individually.

I'll try.

Copy link
Author

@lolmaus lolmaus Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pasalietis I tried it and it does not work: other debounce tests start failing.

image

Copy link

@Pasalietis Pasalietis Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a strange debounce functionality when an empty options object is passed.

Old tests are passing with this:

 export function useDebounceCallback<T extends (...args: any) => ReturnType<T>>(
   func: T,
   delay = 500,
-  options?: DebounceOptions,
+  { leading, trailing, maxWait }: DebounceOptions = {},
 ): DebouncedState<T> {
   const debouncedFunc = useRef<ReturnType<typeof debounce>>()
+  const memoizedOptions = useMemo(() => {
+    if (!leading && !trailing && !maxWait) return undefined
+
+    return ({ leading, trailing, maxWait })
+  }, [leading, trailing, maxWait])

   useUnmount(() => {
     if (debouncedFunc.current) {
@@ -85,7 +90,7 @@ export function useDebounceCallback<T extends (...args: any) => ReturnType<T>>(
   })

   const debounced = useMemo(() => {
-    const debouncedFuncInstance = debounce(func, delay, options)
+    const debouncedFuncInstance = debounce(func, delay, memoizedOptions)

     const wrappedFunc: DebouncedState<T> = (...args: Parameters<T>) => {
       return debouncedFuncInstance(...args)
@@ -104,12 +109,13 @@ export function useDebounceCallback<T extends (...args: any) => ReturnType<T>>(
     }

     return wrappedFunc
-  }, [func, delay, options])
+  }, [func, delay, memoizedOptions])

   // Update the debounced function ref whenever func, wait, or options change
   useEffect(() => {
-    debouncedFunc.current = debounce(func, delay, options)
-  }, [func, delay, options])
+    debouncedFunc.current = debounce(func, delay, memoizedOptions)
+  }, [func, delay, memoizedOptions])

   return debounced
 }


return debounced
}