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

Protect against improper use #8

Closed
jamestalmage opened this issue Apr 1, 2016 · 2 comments
Closed

Protect against improper use #8

jamestalmage opened this issue Apr 1, 2016 · 2 comments

Comments

@jamestalmage
Copy link

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
Copy link

WofWca commented Jul 2, 2023

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

oleksandr-danylchenko added a commit to oleksandr-danylchenko/text-annotator-js that referenced this issue Aug 30, 2024
# See - sindresorhus/debounce#8. The debounced function has a strict context comparison that breaks when applied in the resize listener
@oleksandr-danylchenko
Copy link
Contributor

oleksandr-danylchenko commented Aug 30, 2024

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 - #43

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

3 participants