fix(async): doWhilst, doUntil definitions and tests

[rlindgren]

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: https://github.com/caolan/async/blob/master/CHANGELOG.md#breaking-changes
• Increase the version number in the header if appropriate.
• If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

Breaking changes in version 2.0.0 explain that the test function for doWhilst and doUntil may take a non-Error argument passed from the iteratee callback, and that only while and until pass no arguments.

Additionally, though not in this PR due to a failure to define the API implementation with Typescript, the documentation for doDuring says that test function for doDuring can take any number of non-Error values passed from the iteratee callback followed by the test function callback ie. testFunction(...args, callback). Resulting in Typescript error: A rest parameter must be last in a parameter list.

Relevant Section from Breaking Changes

The test function for whilst, until, and during used to be passed non-error args from the iteratee function's callback, but this led to weirdness where the first call of the test function would be passed no args. We have made it so the test function is never passed extra arguments, and only the doWhilst, doUntil, and doDuring functions pass iteratee callback arguments to the test function (#1217, #1224)

[dt-bot]

types/async/index.d.ts

to authors (@borisyankov @Kern0 @Penryn @fenying @pascalmartin). Could you review this PR?
👍 or 👎?

[typescript-bot]

@rlindgren Please fix the failures indicated in the Travis CI log.

[rlindgren]

As far as I can tell, the failure is unrelated to the code changes. It appears to have been caused by a compilation/lint failure of libpq which has since been resolved here #20884

[wshaver]

Please pull this in!

[fenying]

Emmm, don't you think that the argument result of test callback should also be type of T?

export function doUntil<T, E>(
    fn: AsyncFunction<T, E>,
    test: (result?: T) => boolean,
    callback: ErrorCallback<E>
): void;

Besides, according to the document, there is not only one argument could be passed to the test callback. Thus, why don't we changed it into this style,

export interface AsyncFunctionEx<T, E> {
    (callback: (err?: E, ...args: T[]) => void): void;
}

/**
 * Put the T behind E so that there is no compatibilities problem.
 */
export function doUntil<E, T = any>(
    fn: AsyncFunctionEx<T, E>,
    test: (...args: T[]) => boolean,
    callback: ErrorCallback<E>
): void;

and use it like

import * as libAsync from "async";

libAsync.doUntil<Error, number>(function(callback) {

    console.log("123");
    setTimeout(function() {
        callback(undefined, 123, 321);
    }, 1000);

}, function(...args: number[]): boolean {

    console.log(args[1]);

    return args[0] !== 123;

}, function(e: Error) {

});

[rlindgren]

@fenying You're right. Although, I think that putting type parameter E before T would be strange because then these two would be the only functions in the library that lead with the Error type (for functions that define more than just an Error type). Leading with T means people will need to make changes to existing code... but new code will make sense going forward and since this was technically a "breaking change" it makes sense that existing code would need to be updated. In short, I think its better to maintain consistency rather than avoid compatibility issues for just these two cases.

[fenying]

Fine, I think that's okay. I would vote 👍.
Thanks you for the works very much.

[typescript-bot]

Approved by a listed owner. PR ready to merge pending express review by a maintainer.