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

fix index.d.ts for noImplicitAny option #638

Merged
merged 1 commit into from Mar 15, 2016
Merged

fix index.d.ts for noImplicitAny option #638

merged 1 commit into from Mar 15, 2016

Conversation

bouzuya
Copy link
Contributor

@bouzuya bouzuya commented Mar 11, 2016

$ $(npm bin)/tsc --noImplicitAny
node_modules/ava/index.d.ts(132,2): error TS7010: 'throws', which lacks return-type annotation, implicitly has an 'any' return type.
node_modules/ava/index.d.ts(136,2): error TS7010: 'notThrows', which lacks return-type annotation, implicitly has an 'any' return type.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @ivogabe and @SamVerschueren to be potential reviewers

@@ -129,11 +129,11 @@ export interface AssertContext {
* Assert that function throws an error or promise rejects.
* @param error Can be a constructor, regex, error message or validation function.
*/
throws(value: (() => void) | Promise<{}>, error?: ErrorValidator, message?: string);
throws(value: (() => void) | Promise<{}>, error?: ErrorValidator, message?: string): void;
Copy link
Member

Choose a reason for hiding this comment

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

@ivogabe
Copy link
Contributor

ivogabe commented Mar 11, 2016

If you change the return type of throws to any, 👍. The return type of notThrows looks correct to me.

@sindresorhus
Copy link
Member

@ivogabe There's no good way to indicate that throws returns an error type?

@SamVerschueren
Copy link
Contributor

@sindresorhus yes with Error. But what if someone throws a string or a number? It isn't parsed to an Error object anymore, right?

@ivogabe
Copy link
Contributor

ivogabe commented Mar 11, 2016

Indeed, one might write throw "".

@sindresorhus
Copy link
Member

Oh yeah. Even though nobody should do that...

@SamVerschueren
Copy link
Contributor

Oh yeah. Even though nobody should do that...

Well, we all know how that goes ;)

@bouzuya
Copy link
Contributor Author

bouzuya commented Mar 12, 2016

s/void/any/

NOTE:

throws returns thrown object or PromiseLike or void.
https://github.com/sindresorhus/ava/blob/v0.13.0/lib/assert.js#L76-L85
https://github.com/sindresorhus/ava/blob/v0.13.0/lib/assert.js#L95-L106

notThrows returns PromiseLike or void.
https://github.com/sindresorhus/ava/blob/v0.13.0/lib/assert.js#L117-L124

document:
https://github.com/sindresorhus/ava#throwsfunctionpromise-error-message

Returns the error thrown by function or the rejection reason of promise.
E

@ivogabe
Copy link
Contributor

ivogabe commented Mar 12, 2016

More accurate would be:

throws(value: Promise<{}>, error?: ErrorValidator, message?: string): Promise<any>;
throws(value: () => void, error?: ErrorValidator, message?: string): any;
notThrows<U>(value: Promise<U>, message?: string): Promise<U>;
notThrows(value: () => void, message?: string): void;

sindresorhus added a commit that referenced this pull request Mar 15, 2016
fix index.d.ts for noImplicitAny option
@sindresorhus sindresorhus merged commit c8faedb into avajs:master Mar 15, 2016
@sindresorhus
Copy link
Member

Thanks all :)

@bouzuya bouzuya deleted the fix-d-ts branch March 15, 2016 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants