Skip to content
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

useDebouncedCallback will miss the trailing call when when maxWait is set #1357

Closed
fengkx opened this issue Aug 25, 2023 · 4 comments · Fixed by #1510
Closed

useDebouncedCallback will miss the trailing call when when maxWait is set #1357

fengkx opened this issue Aug 25, 2023 · 4 comments · Fixed by #1510
Labels
🪲 bug Something isn't working released

Comments

@fengkx
Copy link
Contributor

fengkx commented Aug 25, 2023

What is the current behavior?

What does @react-hookz/web do right now.

When I set a maxWait in useDebouncedCallback, it will drop the triling call.

Steps to Reproduce

https://codesandbox.io/s/usedebouncedcallback-bug-gj69sg?file=%2Fsrc%2Fuse-debounce-demo.tsx

The codesandbox has two implementation of counter, and a log in useEffect. One use @react-hookz/web, the other use the use-debounce package.
You can click the two button quickly in a short time. You can see the log is mismatch with the actual final state in the react-hookz one , beacause the trailling call is drop when maxWait is set.

image

What is the expected behavior?

Give a options to enabled traling call or add traling call by default like lodash.

What should @react-hookz/web be doing?

https://github.com/react-hookz/web/blob/master/src/useDebouncedCallback/index.ts#L34 Maybe maxWait excute should not clear waitTimeout.

Environment Details

  • _@react-hookz/web version: 23.0.0
  • _react version: 18.2.0
  • _react-dom version: 18.2.0
  • _typescript version: 4.4.2
  • OS:
  • _Browser:Chrome
  • _Did this work in previous versions? No
@xobotyi xobotyi added the 🪲 bug Something isn't working label Sep 17, 2023
@malavshah9
Copy link

@xobotyi Any idea on why we have used setInterval instead of requestAnimationFrame?

If we use code for useDebounce they have used requestAnimationFrame with manually calculating diff between time.

@xobotyi
Copy link
Contributor

xobotyi commented Oct 3, 2023

Mostly because crapton of code to make raf work like timeout.

moreover i dont think that it is right solution to make debounce use raf in order to make it work, because in case of RAF usage and, say, debounce set to 500ms and debounce function called once - raf callback would be called 30 times before debounced function invocation - it is huge overhead in my eyes.

@xobotyi
Copy link
Contributor

xobotyi commented Feb 3, 2024

ookay, dug into the problem, reason is quiet simple - use-debounce always calls mot recent implementation of debounced function, whereas our only the one that have been called no matter if deps has changed.
i'm not agree that latest should be called all the time, therefore i'll add the code that will update invoked implementation at the time debounced function being updated.

github-actions bot pushed a commit that referenced this issue Feb 4, 2024
## [24.0.4](v24.0.3...v24.0.4) (2024-02-04)

### Bug Fixes

* **useDebouncedCallback:** make invoked function to be updated with deps ([#1510](#1510)) ([12658ee](12658ee)), closes [#1357](#1357)
@xobotyi
Copy link
Contributor

xobotyi commented Feb 4, 2024

🎉 This issue has been resolved in version 24.0.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 bug Something isn't working released
Development

Successfully merging a pull request may close this issue.

3 participants