BackburnerJS / backburner.js

A rewrite of the Ember.js run loop as a generic microlibrary
MIT License
392 stars 80 forks source link

Debounce no longer works without a context #296

Closed workmanw closed 6 years ago

workmanw commented 6 years ago

This worked in Ember 2.16 (BackburnerJS ^1.2.2), but no longer works in Ember 2.18 (BackburnerJS ^1.3.4).

function someFunction() {
  /* ... */
}

Ember.run.debounce(someFunction, 150);

It now throws the following error:

TypeError: method.apply is not a function at Backburner._run (https://localdev:4200/assets/vendor.js:28041:35) at https://localdev:4200/assets/vendor.js:27978:28

Per @rwjblue the issue seems related to: https://github.com/BackburnerJS/backburner.js/commit/55f3bb0860a5cf1dd3a7f3dc1f1fb88f8030cd69

rwjblue commented 6 years ago

The refactor to split out the args massaging into separate methods introduced this. The issue here is that basically debounce (and throttle I believe?) do not actually process the first 2 or 3 arguments before calling _join (which has a much more stringent argument signature now).

bekzod commented 6 years ago

also dropping target completely (in favour of => function and .bind) from all bb methodes could simplify bb internals :trollface:

workmanw commented 6 years ago

Not sure if that was serious because of the :trollface: , but unless I'm missing something that would reduce the ergonomics of it.

This:

Ember.run.debounce(this, this.showTooltip, 250);

Would have to become:

this._showTooltipDebouncer = () => this.showTooltip;
Ember.run.debounce(this._showTooltipDebouncer, 250);

Right? Because the following would not properly debounce given that each time it would generate a different function:

Ember.run.debounce(() => this.showToolip, 250);
rwjblue commented 6 years ago

@workmanw - Confirm.

But @bekzod is generally right, we should work to reduce the number of argument variations that we offer...