sindresorhus / debounce

Delay function calls until a set time elapses after the last invocation
Other
798 stars 80 forks source link

Protect against improper use #8

Closed jamestalmage closed 1 year ago

jamestalmage commented 8 years ago

I've seen people use debounce to do this:

function MyClass() {}

MyClass.prototype.debounced = debounce(function () {
 // ...
});

This can lead to really confusing problems if they create multiple instances of MyClass.

In this case:

var c1 = new MyClass();
var c2 = new MyClass();

c1.debounced('a');
c2.debounced('b');

This leads to the method only ever being called on c2 with arguments "b". It is almost certainly not what the user intended.

I think it's fairly easy to catch that misuse here: index.js#L41

I think the solution is to compare context to previously set values. If it changes, then throw

if (context && this !== context) {
  // If people feel an Error is to strong, maybe just log a warning?
  throw new Error('debounced method called with different contexts');
}

Happy to create a PR if anyone is still maintaining this.

WofWca commented 1 year ago

I don't think it's worth the bloat. You can use the Lodash's debounce, there's plenty of checks there.

oleksandr-danylchenko commented 2 months ago

I believe context protection leads to code bloating when I want to provide a single function to the window.onresize and ResizeObserver.

For example:

  const onResize = debounce(() => {...}, 10);

  window.addEventListener('resize', onResize);
  const resizeObserver = new ResizeObserver(onResize);

The code above will lead to the Debounced method called with different contexts exception. However, I expect that on the 1st call, it'll have the Window context, and on the 2nd it'll be the ResizeObserver.

Also, I cannot use .bind(), because it will wipe out the custom function properties like clear, flush, etc. image

There may be an option to ignore the context mismatch error, something like enforceContextEquality that will be true by default.


UPD: The solution I applied for now is to wrap the debounce into the context-unaware function:

import libDebounce from 'debounce';

export const debounce: typeof libDebounce = (function_, wait = 10, options) => {
  const fn = libDebounce(function_, wait, options);

  const boundFn = fn.bind(undefined);

  Object.getOwnPropertyNames(fn).forEach(
    prop => Object.defineProperty(boundFn, prop, Object.getOwnPropertyDescriptor(fn, prop))
  );
  const proto = Object.getPrototypeOf(fn);
  Object.setPrototypeOf(boundFn, proto);

  return boundFn;
}

UPD2: Created a PR that looses the context equality check strictness by adding the prototype equality check - https://github.com/sindresorhus/debounce/pull/43