-
Notifications
You must be signed in to change notification settings - Fork 57
fix: useDebouncedCallback to support generic parameter type for onChange #267
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the useDebouncedCallback hook to support generic typing, replacing the fixed boolean parameter type with a generic type parameter T. This change enables the hook to work with various data types (strings, numbers, objects, etc.) instead of being restricted to boolean values.
- Adds generic type parameter
<T>to the hook function - Updates the
onChangecallback and debounced function parameter types frombooleantoT - Changes the internal ref value type from
booleantoT | null
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| useDebouncedCallback.ts | Adds generic type support by introducing <T> parameter and updating all related type annotations |
| useDebouncedCallback.md | Updates documentation to reflect the new generic type in the onChange parameter |
| ko/useDebouncedCallback.md | Updates Korean documentation with the same type changes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return useCallback( | ||
| (nextValue: boolean) => { | ||
| (nextValue: T) => { | ||
| if (nextValue === ref.current.value) { |
Copilot
AI
Sep 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The strict equality comparison may not work correctly for object types. When T is an object, this comparison will check reference equality rather than value equality, potentially causing the debounce logic to fail. Consider using a deep equality check or allowing callers to provide a custom comparison function.
|
The lint error of useImpressionRef has been fixed in the PR, but the test code passed by relying on the case where the initial value of useDebouncedCallback was false, so I think the test code needs to be fixed. What do you think? it('should not call handlers if element is not intersecting on visibility change', async () => {
const { observerCallback } = setup();
act(() => {
// isIntersecting is equal to false, which is the initial value of useDebounceCallback,
// so the callback was not executed, but now it has been modified to null, so the test code does not pass.
observerCallback([{ isIntersecting: false, intersectionRatio: 0 }], null);
vi.runAllTimers();
});
Object.defineProperty(document, 'visibilityState', { value: 'hidden', writable: true });
act(() => document.dispatchEvent(new Event('visibilitychange')));
Object.defineProperty(document, 'visibilityState', { value: 'visible', writable: true });
act(() => document.dispatchEvent(new Event('visibilitychange')));
expect(mockOnImpressionStart).not.toHaveBeenCalled();
expect(mockOnImpressionEnd).not.toHaveBeenCalled();
}); |
Overview
This PR updates the
useDebouncedCallbackhook to introduce a generic type parameterTfor theonChangeargument and the debounced callback parameter, instead of being fixed toboolean.Previously, the
onChangeargument was strictly typed asboolean, which caused type errors in practical use cases such as the example below, where a string value is passed:This change enables flexible typing, allowing the hook to work with various value types, like strings, thereby fixing those type errors.
Changes
<T>to theuseDebouncedCallbackfunction.ref.current.valuetype frombooleantoT | null.onChangefunction and debounced callback parameter type toT.Checklist
yarn run fixto format and lint the code and docs?yarn run test:coverageto make sure there is no uncovered line?