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

Multiple debounce calls with different wait values will replace other timers #365

Open
bloemendal opened this issue Oct 2, 2019 · 1 comment · May be fixed by #366
Open

Multiple debounce calls with different wait values will replace other timers #365

bloemendal opened this issue Oct 2, 2019 · 1 comment · May be fixed by #366

Comments

@bloemendal
Copy link

bloemendal commented Oct 2, 2019

We had an issue where we would have a component calling debounce multiple times with different wait times. Albeit a bit weird and (maybe) bad practice, it lead to some strange behaviour.

This test sums is up the problem that occurred:

QUnit.test('debounce with later', function(assert) {
  let done = assert.async();
  let bb = new Backburner(['batman']);

  function debounceFunc(obj) {
    assert.notOk(obj.isFoo, 'debounce called with foo');
    assert.ok(obj.isBar, 'debounce called with bar');
  }
  function laterFunc() {
    assert.ok(true, 'later called');
    done();
  }

  const foo = { isFoo: true };
  const bar = { isBar: true };
  // set debounce with a 500ms wait, then a later and then a new debounce
  // with a smaller wait (10ms)
  // the debounce will replace the later and the later will never run
  bb.debounce(debounceFunc, foo, 500);
  bb.later(laterFunc, 100);
  bb.debounce(debounceFunc, bar, 10);
});

The first debounce had a bigger wait than the second debounce, but between the two calls a later timer was added that had a smaller timeout than the first debounce. The result is that the latter debounce will be added to the timers queue not after the first debounce but before the first debounce. The following code which removes the first debounce from the timers queue caused a problem:

let index = findTimerItem(target, method, _timers);
...
let i = searchTimer(executeAt, _timers);
...
this._timers.splice(i, 0, executeAt, timerId, target, method, args, stack);
this._timers.splice(index, TIMERS_OFFSET);

The index is the position of the first debounce, the i is the index where the new debounce will be scheduled. When removing the first debounce from the timers queue the assumption is that i > index, but in our test case i < index because the executeAt is before the first debounce. Then this._timers.splice(index, TIMERS_OFFSET); will remove the wrong scheduled timer - in our test case the later.

I'm curious to see what you guys think about this. The multiple debounce calls are weird, but the behaviour that occurs is - in my opinion - extremely unwanted. In our case some pretty important function calls where disappearing and it took a while for us to discover the cause.

@sophypal
Copy link

sophypal commented Apr 22, 2020

I've hit this particular issue too. You've expressed clearly what I was seeing. In my case, I was working with quite a few independent addons all running debounce, throttle and later at their own discretion. I discovered the mutation in this._timers and as a result my task wasn't scheduled when it expired. I really hope someone sees this as it's an annoying issue.

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

Successfully merging a pull request may close this issue.

2 participants