Skip to content
This repository has been archived by the owner on Sep 29, 2020. It is now read-only.

@throttle / @debounce - allow to cancel method call #72

Closed
avocadowastaken opened this issue Apr 15, 2016 · 8 comments
Closed

@throttle / @debounce - allow to cancel method call #72

avocadowastaken opened this issue Apr 15, 2016 · 8 comments

Comments

@avocadowastaken
Copy link

It would be great to add functionality to cancel method execution

Currently I use @decorate on lodash.throttle and lodash.debounce to make it work

class EventHelper() {
    cancelChange() {
        this.triggerChange.cancel();
    }

    @decorate(_.debounce, 100)
    triggerChange() {
         // do something
    }
}

That's work just fine, but if i want to use @autobind on it, .bind removes .cancel() function from method.

So I had looked through @throttle and @debounce code and noticed that timeout ids are stored in class instance meta

So the fastest and dumbest solution will be:

debounce.cancel = function(instance, method) {
    const { debounceTimeoutIds } = metaFor(instance);

    let methodKey;

    for (let key in instance) {
        if (instance[key] === method) {
            methodKey = key;
            break;
        }
    }

    clearTimeout(debounceTimeoutIds[methodKey]);

    delete debounceTimeoutIds[methodKey];
}

and

class EventHelper() {
    cancelChange() {
        debounce.cancel(this, this.triggerChange);
    }

    @autobind
    @debounce(100)
    triggerChange() {
         // do something
    }
}

But that syntax can make people cry and worth discussion.

May be put it to utils as debounceCancel

Or make method argument as optional and clear all timeouts in meta

Or create another method in class that will cancel its debounce like triggerChange => __cancelTriggerChange (warn or throw exception if method exists)

@jayphelps
Copy link
Owner

Definitely down to discuss this, in the meantime just in case you hadn't thought of it you can work around this:

class EventHelper() {
  cancelChange() {
    this.triggerChangeDebounced.cancel();
  }

  @autobind
  triggerChange() {
    this.triggerChangeDebounced(this);
  }

  @decorate(_.debounce, 100)
  triggerChangeDebounced(context) {
    // do something with context, like context.kickDog()
  }

  kickDog() {}
}

Let me think through some syntax options for cancel support.

@avocadowastaken
Copy link
Author

@jayphelps thanks for your response,

I looked up through my code and noticed that I need to .cancel delayed execution only when i destroying classes (in other cases it can be done without using @decorate)

So if you want to keep it simple and easy for people to use i think you can adopt clearTimeout function syntax:

import {debounce, clearTimeouts} from 'core-decorators';

class Bar {
    @debounce(1000)
    baz() {
          // do something
    }
}

const foo = {
    bar:  new Bar(),
    bas: new Bar()
};

foo.bar.baz();
foo.bas.baz();

delete foo.bar;
delete foo.bas;

clearTimeouts(foo.bar);

Here foo.bar.baz - will be canceled, foo.bas.baz will be executed.

clearTimeouts - will iterate through instance meta and clear every meta timeout id

I think it's will be easiest way to implement and use

@nehaleem
Copy link

+1 Also wanted to cancel() debounce after React component unmount (isMounted() is deprecated!). Cant use clearTimeout, because youre hiding your metas behind non-exported Symbol.

@jayphelps
Copy link
Owner

Is a cancel() function on the debounced method ideal? I'm happy with that.

class Example extends Component {
  @debounce(1000)
  doStuff() {
    // do stuff
  }

  componentWillUnmount() {
    this.doStuff.cancel();
  }
}

@jayphelps
Copy link
Owner

jayphelps commented Jul 27, 2016

@nehaleem also--in the meantime if you're also already using lodash, you can accomplish this with the @decorate decorator and lodash's _.debounce:

class Example extends Component {
  @decorate(_.debounce, 1000)
  doStuff() {
    // do stuff
  }

  componentWillUnmount() {
    this.doStuff.cancel();
  }
}

If you need autobinding too, you can do this:

class Example extends Component {
  @autobind
  doStuff() {
    this.doStuffDebounced();
  }

  @decorate(_.debounce, 1000)
  doStuffDebounced() {
    // do stuff
  }

  componentWillUnmount() {
    this.doStuffDebounced.cancel();
  }
}

@nehaleem
Copy link

@jayphelps Yeah, thats what i used for now, with future "//TODO" 😄

@jayphelps
Copy link
Owner

This turns out to be a non-trivial thing to do.

The issue is around making it so that the cancel function is unique for each instance of that class so it does not cancel all debounces for every instance.

This is a similar problem that @decorate had and I sort of solve by generating a new function for each instance, the first time you look up that property: https://github.com/jayphelps/core-decorators.js/blob/master/src/decorate.js

I may try to do that here, but just a heads up that it probably will take some time..

@avocadowastaken
Copy link
Author

There are no "elegant" ways to do it with decorators now, so i end up with:

class EventHelper() {
  cancelChange() {
    this.triggerChange.cancel()
  }

  triggerChange = _.debounce(() => {
    // do something
  }, 100)
}

Anyways, thanks you @jayphelps 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants