Unexpected API semantics of Q.delay()

776 views
Skip to first unread message

Wout Mertens

unread,
Jun 24, 2013, 4:49:22 PM6/24/13
to q-con...@googlegroups.com
When I found Q.delay() I thought it was a chainable delay behaving like the jQuery .delay() (http://api.jquery.com/delay/)

However, it simply resolves a promise after a delay from the initial setup of the call. Meaning that

> function f(i){console.log(i++); return i;}
undefined
> Q(1).then(f).delay(1000).then(f).delay(1000).then(f).delay(1000).then(f).delay(1000).then(f)
[object Object]
> 1
2
3
4
5

2 through 5 are printed immediately. I was expecting .delay() to behave like

> function delay(i){return Q(i).delay(1000)}
undefined
> Q(1).then(f).then(delay).then(f).then(delay).then(f).then(delay).then(f).then(delay).then(f)
[object Object]
> 1
2
3
4
5

Here we see a 1 second delay between each number.

Basically what I would expect delay to be is

Q.delay = delay;
function delay(promise, timeout) {
    if (timeout === void 0) {
        timeout = promise;
        promise = void 0;
    }
    var deferred = defer();
    function doDelay() {
        setTimeout(function () {
            deferred.resolve(promise);
        }, timeout);
    }
    when(promise, doDelay, doDelay, deferred.notify);
    return deferred.promise;
}
 
In other words, forward the resolution or rejection timeout milliseconds after it occurred. This is useful for tests and invalidating cache objects etc.

Is that an acceptable change in semantics? I think that people that were using delay() in the original sense were only using Q.delay, or didn't notice that the delays didn't add up.

Wout.

Wout Mertens

unread,
Jun 25, 2013, 4:53:05 AM6/25/13
to q-con...@googlegroups.com
I created PR 326 for this. This renames the current delay() to throttle() and adds the delay() I propose below.

Note that Q.delay(x) and Q.throttle(x) are virtually the same, and thus the most-used use case in the tests is not impacted.

Has tests for both, all tests pass.

Pretty please? *puppyeye*

Wout.

Kris Kowal

unread,
Jun 25, 2013, 1:59:19 PM6/25/13
to Q Continuum
This makes sense. I’ll follow up on the PR.
Reply all
Reply to author
Forward
0 new messages