async: Various fixes, mostly related to strict-nulls. #15011
Conversation
Also some places where the error type was wrong.
|
async/index.d.ts to authors (@borisyankov @Kern0 @Penryn @fenying @pascalmartin). Could you review this PR? Checklist
|
async/index.d.ts
Outdated
|
|
||
| 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; |
There was a problem hiding this comment.
An array of undefineds for "item" seems really odd. Is there a doc reference that shows this is possible?
There was a problem hiding this comment.
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.
async/index.d.ts
Outdated
|
|
||
| interface AsyncFunction<T, E> { (callback: (err?: E, result?: T) => void): void; } | ||
| interface AsyncFunction<T, E> { (callback: (err?: E, result?: T) => void): any; } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| 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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
async/index.d.ts
Outdated
| 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; | ||
| reflectAll<T, E>(tasks: AsyncFunction<T, E>[]): ((callback: (err: undefined, result: {error?: E, value?: T}) => void) => void)[]; |
There was a problem hiding this comment.
Same question as above, and also why is this undefined and the other one null? Again a doc would be helpful.
There was a problem hiding this comment.
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.
|
Additionally since these are strict null fixes, could you turn on |
|
I still havn't looked into I fixed the other things, and I found 3 other small errors (8875c23) |
|
So the async guys merged my pull request (caolan/async#1381), and I updated |
Various fixes, mostly related to strict-nulls.
Also some places where the error type was wrong.
I wrote the type for the empty array as
undefined[]. At first i wrotenever[], but I think a lot of people would get confused by thenever.masterbranch.tscwithout errors.npm run lint package-nameif atslint.jsonis present. (used it to change a few things, but there is still a lot of warnings (as there was before))Select one of these and delete the others:
If changing an existing definition:
tslint.jsoncontaining{ "extends": "../tslint.json" }.