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

Debounce does not handle await promises well #228

Closed
sreeharsha-rav opened this issue Aug 24, 2024 · 1 comment
Closed

Debounce does not handle await promises well #228

sreeharsha-rav opened this issue Aug 24, 2024 · 1 comment

Comments

@sreeharsha-rav
Copy link

sreeharsha-rav commented Aug 24, 2024

Issue

Does not handle uncaught awaited promises well. https://github.com/cedmax/youmightnotneed/blob/production/src/content/lodash/function/debounce/vanilla.js

Use case

using debounce for making asynchronous calls for a weather API.

Problem

Does not handle uncaught promises well:

Using debounce in async fetchData below:
image

Current debounce function ported from vanilla JS:
image

Suggested changes

Add an array of resolves to handle uncaught await promises.
image

Since above code is in typescript, equivalent code in js is:

export const debounce = (func, delay, { leading = false } = {}) => {
  let timerId = null;
  let resolveList = [];

  return (...args) => {
    if (!timerId && leading) {
      func(...args);
    }
    if (timerId) {
      clearTimeout(timerId);
    }

    return new Promise((resolve) => {
      resolveList.push(resolve);
      timerId = setTimeout(async () => {
        await func(...args);
        timerId = null;
        resolveList.forEach((res) => res());
        resolveList = [];
      }, delay);
    });
  };
};
@cedmax
Copy link
Owner

cedmax commented Sep 12, 2024

Hi @sreeharsha-rav, thanks for reaching out.
I understand where you are coming from, but I don't think this is a change worth introducing in here: as much as the changes would make the implementation more robust, the point of this project is educational and not to create a drop in replacement for lodash.

The proposed changes complicate the script significantly, making it harder to read and, potentially, to understand.
I made a point of trying to keep the implementations as simple as possible, at the cost of some edge cases, and I think this one falls in that category.

@cedmax cedmax closed this as completed Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants