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

async: Various fixes, mostly related to strict-nulls. #15011

Merged
merged 7 commits into from Mar 14, 2017
34 changes: 16 additions & 18 deletions async/index.d.ts
Expand Up @@ -6,17 +6,16 @@
interface Dictionary<T> { [key: string]: T; }

interface ErrorCallback<T> { (err?: T): void; }
interface AsyncWaterfallCallback<E> { (err: E, ...args: any[]): void; }
interface AsyncBooleanResultCallback<E> { (err: E, truthValue: boolean): void; }
interface AsyncResultCallback<T, E> { (err: E, result: T): void; }
interface AsyncResultArrayCallback<T, E> { (err: E, results: T[]): void; }
interface AsyncResultObjectCallback<T, E> { (err: E, results: Dictionary<T>): void; }
interface AsyncBooleanResultCallback<E> { (err?: E, truthValue?: boolean): void; }
interface AsyncResultCallback<T, E> { (err?: E, result?: T): void; }
interface AsyncResultArrayCallback<T, E> { (err?: E, results?: (T | undefined)[]): void; }
interface AsyncResultObjectCallback<T, E> { (err: E | undefined, results: Dictionary<T>): void; }

interface AsyncFunction<T, E> { (callback: (err?: E, result?: T) => void): void; }
interface AsyncFunction<T, E> { (callback: (err?: E, result?: T) => void): any; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this any necessary? A function that returns a value is assignable to a function type that returns void, so you can still call the functions below with functions that return a value. void makes it clearer that the return value is not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that a function declared as returning void actually had to return void (since it works that way with variables).
Thanks for explaining that, I'll fix it.

interface AsyncIterator<T, E> { (item: T, callback: ErrorCallback<E>): void; }
interface AsyncForEachOfIterator<T, E> { (item: T, key: number|string, callback: ErrorCallback<E>): void; }
interface AsyncResultIterator<T, R, E> { (item: T, callback: AsyncResultCallback<R, E>): void; }
interface AsyncMemoIterator<T, R, E> { (memo: R, item: T, callback: AsyncResultCallback<R, E>): void; }
interface AsyncMemoIterator<T, R, E> { (memo: R | undefined, item: T, callback: AsyncResultCallback<R, E>): void; }
interface AsyncBooleanIterator<T, E> { (item: T, callback: AsyncBooleanResultCallback<E>): void; }

interface AsyncWorker<T, E> { (task: T, callback: ErrorCallback<E>): void; }
Expand Down Expand Up @@ -76,7 +75,7 @@ interface AsyncPriorityQueue<T> {

interface AsyncCargo {
length(): number;
payload: number;
payload?: number;
push(task: any, callback? : Function): void;
push(task: any[], callback? : Function): void;
saturated(): void;
Expand Down Expand Up @@ -186,25 +185,25 @@ interface Async {
cargo<E>(worker : (tasks: any[], callback : ErrorCallback<E>) => void, payload? : number) : AsyncCargo;
auto<E>(tasks: any, concurrency?: number, callback?: AsyncResultCallback<any, E>): void;
autoInject<E>(tasks: any, callback?: AsyncResultCallback<any, E>): void;
retry<T, E>(opts: number, task: (callback : AsyncResultCallback<T, E>, results: any) => void, callback: AsyncResultCallback<any, E>): void;
retry<T, E>(opts: { times: number, interval: number|((retryCount: number) => number) }, task: (callback: AsyncResultCallback<T, E>, results : any) => void, callback: AsyncResultCallback<any, E>): void;
retryable<T, E>(opts: number | {times: number, interval: number}, task: AsyncFunction<T, E>): AsyncFunction<T, E>;
apply<E>(fn: Function, ...arguments: any[]): AsyncFunction<any,E>;
retry<T, E>(opts: number, task: (callback : AsyncResultCallback<T, E>, results: any) => void, callback: AsyncResultCallback<any, E | Error>): void;
retry<T, E>(opts: { times: number, interval: number|((retryCount: number) => number) }, task: (callback: AsyncResultCallback<T, E>, results : any) => void, callback: AsyncResultCallback<any, E | Error>): void;
retryable<T, E>(opts: number | {times: number, interval: number}, task: AsyncFunction<T, E>): AsyncFunction<T, E | Error>;
apply<E>(fn: Function, ...arguments: any[]): AsyncFunction<any,E | Error>;
nextTick(callback: Function, ...args: any[]): void;
setImmediate: typeof async.nextTick;

reflect<T, E>(fn: AsyncFunction<T, E>) : (callback: (err: void, result: {error?: Error, value?: T}) => void) => void;
reflectAll<T, E>(tasks: AsyncFunction<T, E>[]): ((callback: (err: void, result: {error?: Error, value?: T}) => void) => void)[];
reflect<T, E>(fn: AsyncFunction<T, E>) : (callback: (err: null, result: {error?: E, value?: T}) => void) => void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is null appropriate here? Are you guaranteed not to get an error from a reflected function? Could you provide a source doc please so I can understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are guaranteed not to get an error from a reflected function: https://caolan.github.io/async/docs.html#reflect

And in the implementation, they way the callback is called is:

reflectCallback(null, {
    value: value
});

Or

reflectCallback(null, {
    error: err
});

That is why i put in the null, because it is allways null.

reflectAll<T, E>(tasks: AsyncFunction<T, E>[]): ((callback: (err: undefined, result: {error?: E, value?: T}) => void) => void)[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above, and also why is this undefined and the other one null? Again a doc would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually just forgot to change the second one to null (same argumentation as above).

I then later changed it from void to undefined, because it wasn't in a function-return.

I'm changing it to null now.


timeout<T, E>(fn: AsyncFunction<T, E>, milliseconds: number, info?: any): AsyncFunction<T, E>;
timeout<T, R, E>(fn: AsyncResultIterator<T, R, E>, milliseconds: number, info?: any): AsyncResultIterator<T, R, E>;
timeout<T, E>(fn: AsyncFunction<T, E>, milliseconds: number, info?: any): AsyncFunction<T, E | Error>;
timeout<T, R, E>(fn: AsyncResultIterator<T, R, E>, milliseconds: number, info?: any): AsyncResultIterator<T, R, E | Error>;

times<T, E> (n: number, iterator: AsyncResultIterator<number, T, E>, callback: AsyncResultArrayCallback<T, E>): void;
timesSeries<T, E>(n: number, iterator: AsyncResultIterator<number, T, E>, callback: AsyncResultArrayCallback<T, E>): void;
timesLimit<T, E>(n: number, limit: number, iterator: AsyncResultIterator<number, T, E>, callback: AsyncResultArrayCallback<T, E>): void;

transform<T, R, E>(arr: T[], iteratee: (acc: R[], item: T, key: string, callback: (error?: E) => void) => void): void;
transform<T, R, E>(arr: T[], acc: R[], iteratee: (acc: R[], item: T, key: string, callback: (error?: E) => void) => void): void;
transform<T, R, E>(arr: T[], acc: R[], iteratee: (acc: R[] | null, item: T | undefined[], key?: string, callback?: (error?: E) => void) => void): void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An array of undefineds for "item" seems really odd. Is there a doc reference that shows this is possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to type the empty array.
Docs: https://caolan.github.io/async/docs.html#transform

But looking at it again, I misunderstood the code (it was passed the accumulator, and in my case that was always empty), and you are right it isn't the right fix.

Actually there is an inconsistency between the implementation and the documentation (both the .d.ts and the official documentation).
If two arguments are passed to the transform function, it crashes.
I've tried to fix that in the implementation: caolan/async#1381
And I'll wait with updating this, until i get an answer there.

transform<T, R, E>(arr: {[key: string] : T}, iteratee: (acc: {[key: string] : R}, item: T, key: string, callback: (error?: E) => void) => void): void;
transform<T, R, E>(arr: {[key: string] : T}, acc: {[key: string] : R}, iteratee: (acc: {[key: string] : R}, item: T, key: string, callback: (error?: E) => void) => void): void;

Expand All @@ -226,4 +225,3 @@ declare var async: Async;
declare module "async" {
export = async;
}