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

Adding canceling of debounced validations #428

Conversation

estshy
Copy link

@estshy estshy commented Jan 5, 2017

Let's say I have async email validation which will fire every keydown. In that case I want to debounce it every several milliseconds. I would assume that debounce will abort all promises before any new debounced function will start. Unfortunately it's not the case. In Ember CP Validations deboucing works more like deferring the validation because it never cancels previous async operation.

@offirgolan
Copy link
Collaborator

offirgolan commented Jan 7, 2017

@estshy thats not what should be happening.

From the Ember.run.debounce docs:

Delay calling the target method until the debounce period has elapsed with no additional debounce calls. If debounce is called again before the specified time has elapsed, the timer is reset and the entire period must pass again before the target method is called.

This means that the validator shouldnt even be executed until the specified time has elapsed without the debounce method getting called again. The only thing debounce does is guarantee that your validations will only be run once after the specified time has passed without changes to its dependents. Ideally, there shouldnt be any promises to abort because only a single promise should exists per a successful debounce.

@estshy
Copy link
Author

estshy commented Jan 9, 2017

@offirgolan You are partially right. Debounce should work out of the box provided that you will pass to it reference to the same function but we are passing a new instance each time validators are being created. In that case here is no way for ember to know whether one function is actually the same as the other one.

I will make a demo for you to demonstrate that async validators are actually not debounced properly soon.

@muttli
Copy link

muttli commented Jan 9, 2017

+1. Would love if it would cancel previously queued validations (within debounce timer).

@offirgolan
Copy link
Collaborator

@estshy good catch! This seems to be a regression from a previous version after I changed it to use resolve as the passed method.

I think at this point a neater solution would be to just use an "in-house" debounce.

value = new Promise((resolve) => {
          let t = run.later(resolve, debounce);
  
          if (!opts.disableDebounceCache) {
           let guid = guidFor(validator);
           run.cancel(cache[guid])
           cache[guid] = t;
          }
        })

Thoughts? If you can test that out as well as add a test case that would be fantastic!

@offirgolan
Copy link
Collaborator

Closing this PR as I addressed it in #434.

@offirgolan offirgolan closed this Jan 12, 2017
@offirgolan
Copy link
Collaborator

Released the fix in v3.1.5.

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 this pull request may close these issues.

3 participants