-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add type definitions #47
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/** | ||
* Notes about these type definitions: | ||
* | ||
* - Callbacks returning multiple completion values using multiple arguments are not supported by these types. | ||
* Prefer to use Node's style by grouping your values in a single object or array. | ||
* Support for this kind of callback is blocked by Microsoft/TypeScript#5453 | ||
* | ||
* - For ease of use, `asyncDone` lets you pass callback functions with a result type `T` instead of `T | undefined`. | ||
* This matches Node's types but can lead to unsound code being typechecked. | ||
* | ||
* The following code typechecks but fails at runtime: | ||
* ```typescript | ||
* async function getString(): Promise<string> { | ||
* return "Hello, World!"; | ||
* } | ||
* | ||
* async function evilGetString(): Promise<string> { | ||
* throw new Error("Hello, World!"); | ||
* } | ||
* | ||
* function cb(err: Error | null, result: string): void { | ||
* // This is unsound because `result` is `undefined` when `err` is not `null`. | ||
* console.log(result.toLowerCase()); | ||
* } | ||
* | ||
* asyncDone(getString, cb); // Prints `hello, world!` | ||
* asyncDone(evilGetString, cb); // Runtime error: `TypeError: Cannot read property 'toLowerCase' of undefined` | ||
* ``` | ||
* | ||
* Enforcing stricter callbacks would require developers to use `result?: string` and assert the existence | ||
* of the result either by checking it directly or using the `!` assertion operator after testing for errors. | ||
* ```typescript | ||
* function stricterCb1(err: Error | null, result?: string): void { | ||
* if (err !== null) { | ||
* console.error(err); | ||
* return; | ||
* } | ||
* console.log(result!.toLowerCase()); | ||
* } | ||
* | ||
* function stricterCb2(err: Error | null, result?: string): void { | ||
* if (result === undefined) { | ||
* console.error("Undefined result. Error:); | ||
* console.error(err); | ||
* return; | ||
* } | ||
* console.log(result.toLowerCase()); | ||
* } | ||
* ``` | ||
*/ | ||
import { ChildProcess } from "child_process"; | ||
import { EventEmitter } from "events"; | ||
import { Readable as ReadableStream } from "stream"; | ||
|
||
declare namespace asyncDone { | ||
|
||
/** | ||
* Represents a callback function used to signal the completion of a | ||
* task that does not return any completion value. | ||
*/ | ||
type VoidCallback = (err: Error | null) => void; | ||
|
||
/** | ||
* Represents a callback function used to signal the completion of a | ||
* task that does return a single completion value. | ||
*/ | ||
interface Callback<T> { | ||
(err: null, result: T): void; | ||
|
||
// Set the type of `result` to `T` if you want to enforce stricter callback functions. | ||
// (See comment above about risks of unsound type checking) | ||
(err: Error, result?: any): void; | ||
} | ||
|
||
/** | ||
* Minimal `Observable` interface compatible with `async-done`. | ||
* | ||
* @see https://github.com/ReactiveX/rxjs/blob/c3c56867eaf93f302ac7cd588034c7d8712f2834/src/internal/Observable.ts#L77 | ||
*/ | ||
interface Observable<T = any> { | ||
subscribe(next?: (value: T) => void, error?: (error: any) => void, complete?: () => void): any; | ||
} | ||
|
||
/** | ||
* Represents an async operation. | ||
*/ | ||
export type AsyncTask<R = any> = | ||
((done: VoidCallback) => void) | ||
| ((done: Callback<R>) => void) | ||
| (() => ChildProcess | EventEmitter | Observable<R> | PromiseLike<R> | ReadableStream ); | ||
} | ||
|
||
/** | ||
* Takes a function to execute (`fn`) and a function to call on completion (`callback`). | ||
* | ||
* @param fn Function to execute. | ||
* @param callback Function to call on completion. | ||
*/ | ||
declare function asyncDone<R = any>(fn: asyncDone.AsyncTask<R>, callback: asyncDone.Callback<R>): void; | ||
|
||
export = asyncDone; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,14 +4,14 @@ var expect = require('expect'); | |
|
||
var asyncDone = require('../'); | ||
|
||
var Observable = require('rx').Observable; | ||
var Observable = require('rxjs').Observable; | ||
|
||
function success() { | ||
return Observable.empty(); | ||
} | ||
|
||
function successValue() { | ||
return Observable.return(42); | ||
return Observable.of(42); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you leave a comment that this was previously There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or maybe we should keep both dependencies (rx & rxjs) since 4.x is still published as rx?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
function failure() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import asyncDone, { Callback } from "async-done"; | ||
|
||
function success(cb: Callback<number>): void { | ||
cb(null, 2); | ||
} | ||
|
||
function failure(cb: Callback<number>): void { | ||
cb(new Error("Callback Error")); | ||
} | ||
|
||
function neverDone(): number { | ||
return 2; | ||
} | ||
|
||
// `success` and stricter callback | ||
asyncDone(success, function (err: Error | null, result?: number): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// The following code fails to compile as expected: | ||
// asyncDone(success, function (err: Error | null, result?: string): void { | ||
// console.log("Done"); | ||
// }); | ||
|
||
// `success` and unsound callback | ||
asyncDone(success, function (err: Error | null, result: number): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// `failure` and stricter callback | ||
asyncDone(failure, function (err: Error | null, result?: number): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// `failure` and unsound callback | ||
asyncDone(failure, function (err: Error | null, result: number): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// I don't think TS is currently able to prevent the current code from compiling | ||
// (`neverDone` matches with `(done: VoidCallback) => void` for example) | ||
// asyncDone(neverDone, function(err: Error | null, result?: number): void { | ||
// console.log("Done"); | ||
// }); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import asyncDone from "async-done"; | ||
import cp from "child_process"; | ||
|
||
|
||
function success(): cp.ChildProcess { | ||
return cp.exec("echo hello world"); | ||
} | ||
|
||
function failure(): cp.ChildProcess { | ||
return cp.exec("foo-bar-baz hello world"); | ||
} | ||
|
||
asyncDone(success, function (err: Error | null): void { | ||
console.log("Done"); | ||
}); | ||
|
||
asyncDone(failure, function (err: Error | null): void { | ||
console.log("Done"); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import asyncDone from "async-done"; | ||
import { Observable } from "rxjs/Observable"; | ||
import 'rxjs/add/observable/empty'; | ||
import 'rxjs/add/observable/of'; | ||
|
||
function success(): Observable<undefined> { | ||
return Observable.empty(); | ||
} | ||
|
||
function successValue(): Observable<number> { | ||
return Observable.of(42); | ||
} | ||
|
||
function failure(): Observable<number> { | ||
return Observable.throw(new Error("Observable error")); | ||
} | ||
|
||
// `success` callback | ||
asyncDone(success, function (err: Error | null): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// The following code fails to compile as expected (`undefined` is not assignable to `number`): | ||
// asyncDone(success, function (err: Error | null, result: number): void { | ||
// console.log("Done"); | ||
// }); | ||
|
||
// `successValue` and stricter callback | ||
asyncDone(successValue, function (err: Error | null, result?: number): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// `successValue` and unsound callback | ||
asyncDone(successValue, function (err: Error | null, result: number): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// `failure` and stricter callback | ||
asyncDone(failure, function (err: Error | null, result?: number): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// `failure` and unsound callback | ||
asyncDone(failure, function (err: Error | null, result: number): void { | ||
console.log("Done"); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import asyncDone from "async-done"; | ||
|
||
function success(): Promise<number> { | ||
return Promise.resolve(2); | ||
} | ||
|
||
function failure(): Promise<number> { | ||
return Promise.reject(new Error("Promise Error")); | ||
} | ||
|
||
// `successValue` and stricter callback | ||
asyncDone(success, function (err: Error | null, result?: number): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// The following code fails to compile as expected: | ||
// asyncDone(success, function (err: Error | null, result?: string): void { | ||
// console.log("Done"); | ||
// }); | ||
|
||
// `successValue` and unsound callback | ||
asyncDone(success, function (err: Error | null, result: number): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// `failure` and stricter callback | ||
asyncDone(failure, function (err: Error | null, result?: number): void { | ||
console.log("Done"); | ||
}); | ||
|
||
// `failure` and unsound callback | ||
asyncDone(failure, function (err: Error | null, result: number): void { | ||
console.log("Done"); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import asyncDone from "async-done"; | ||
import { Readable, Stream } from "stream"; | ||
|
||
function readableSuccess(): Readable { | ||
return undefined as any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are these returning undefined as any? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These file are compiled but never run. I just needed to check that a function returning a I did not care about the implementation of the task (runtime behavior is tested in |
||
} | ||
|
||
function readableFail(): Readable { | ||
return undefined as any; | ||
} | ||
|
||
function streamSuccess(): Stream { | ||
return undefined as any; | ||
} | ||
|
||
function streamFail(): Stream { | ||
return undefined as any; | ||
} | ||
|
||
asyncDone(readableSuccess, function (err: Error | null): void { | ||
console.log("Done"); | ||
}); | ||
|
||
asyncDone(readableFail, function (err: Error | null): void { | ||
console.log("Done"); | ||
}); | ||
|
||
asyncDone(streamSuccess, function (err: Error | null): void { | ||
console.log("Done"); | ||
}); | ||
|
||
asyncDone(streamFail, function (err: Error | null): void { | ||
console.log("Done"); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be Readable, Writeable or Transform/Duplex because we exhaust the stream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be able to reference the base "Stream" instead? Not sure if it contains the indicators we use to check for stream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally had
Stream
but wasn't sure if Writable streams were supported. Thanks for confirming that any Node stream is fine. I'll update it.