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

Add type definitions #47

Merged
merged 2 commits into from
May 13, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions index.d.ts
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";
Copy link
Member

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.

Copy link
Member

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.

Copy link
Member Author

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.


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;
11 changes: 8 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,25 @@
"contributors": [
"Blaine Bublitz <blaine.bublitz@gmail.com>",
"Pawel Kozlowski <pkozlowski.opensource@gmail.com>",
"Matthew Podwysocki <matthew.podwysocki@gmail.com>"
"Matthew Podwysocki <matthew.podwysocki@gmail.com>",
"Charles Samborski <demurgos@demurgos.net>"
],
"repository": "gulpjs/async-done",
"license": "MIT",
"engines": {
"node": ">= 0.10"
},
"main": "index.js",
"types": "index.d.ts",
"files": [
"index.js",
"LICENSE"
],
"scripts": {
"lint": "eslint . && jscs index.js test/",
"pretest": "npm run lint",
"test": "mocha --async-only",
"test": "mocha --async-only && npm run test-types",
"test-types": "tsc -p test/types",
"cover": "istanbul cover _mocha --report lcovonly",
"coveralls": "npm run cover && istanbul-coveralls"
},
Expand All @@ -32,6 +35,7 @@
"stream-exhaust": "^1.0.1"
},
"devDependencies": {
"@types/node": "^9.3.0",
"eslint": "^1.7.3",
"eslint-config-gulp": "^2.0.0",
"expect": "^1.19.0",
Expand All @@ -41,8 +45,9 @@
"jscs-preset-gulp": "^1.0.0",
"mocha": "^2.4.5",
"pumpify": "^1.3.6",
"rx": "^4.0.6",
"rxjs": "^5.5.6",
"through2": "^2.0.0",
"typescript": "^2.6.2",
"when": "^3.7.3"
},
"keywords": [
Expand Down
4 changes: 2 additions & 2 deletions test/observables.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

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

Can you leave a comment that this was previously Observable.return in case someone was using the docs as a reference for older Rx?

Copy link
Member

Choose a reason for hiding this comment

The 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??

Copy link
Member Author

Choose a reason for hiding this comment

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

rx is only used for the tests so I don't think we need to keep both versions as dependencies but it's definitely valuable to add a comment and mention it in the documentation. I'll do it.

}

function failure() {
Expand Down
44 changes: 44 additions & 0 deletions test/types/callback.ts
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");
// });
19 changes: 19 additions & 0 deletions test/types/child_processes.ts
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");
});
46 changes: 46 additions & 0 deletions test/types/observables.ts
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");
});
34 changes: 34 additions & 0 deletions test/types/promises.ts
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");
});
34 changes: 34 additions & 0 deletions test/types/streams.ts
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;
Copy link
Member

Choose a reason for hiding this comment

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

why are these returning undefined as any?

Copy link
Member Author

Choose a reason for hiding this comment

The 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 Readable is accepted.

I did not care about the implementation of the task (runtime behavior is tested in test/streams.js). Using any allows to bypass type-checking locally ("trust me, this function returns a Readable").
I wrote the body of the functions for other kinds of objects because they are one-liners and it is more helpful to visualize a concrete example. In the case of streams, the boiler plate was slightly higher so I just used a dummy body.
I'll replace it with return new Stream(); instead of replicating what's in the unit-tests.

}

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");
});
Loading