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.

Sign up for GitHub

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 TypeScript definition #31

Merged
merged 7 commits into from
Aug 20, 2018
Merged

Add TypeScript definition #31

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5d9f202
Add typescript typings
dmitry-korolev Aug 9, 2018
d01ed3a
Add tsdoc comments and default export
dmitry-korolev Aug 20, 2018
d5a7a44
Fix code style
dmitry-korolev Aug 20, 2018
0204f6e
Add todo notice about reason type
dmitry-korolev Aug 20, 2018
b5f8fd2
Remove type definitions from tsdoc comments
dmitry-korolev Aug 20, 2018
1a17bee
Split delay type definition into two overloads
dmitry-korolev Aug 20, 2018
4a197ec
Update index.d.ts
sindresorhus Aug 20, 2018
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
10 changes: 10 additions & 0 deletions index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
interface ClearablePromise<T> extends Promise<T> {
clear (): void
Copy link
Owner

Choose a reason for hiding this comment

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

clear(): void

}

declare const delay: {
<T = never>(ms: number, value?: T): ClearablePromise<T>
Copy link
Owner

Choose a reason for hiding this comment

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

I know it's ms in the docs, but can you use milliseconds here for clarity?

reject(ms: number, reason?: any): ClearablePromise<never>
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this not also T?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typescript Promise definition doesn't allow to set an error type. Since the Promise, created by reject will always fail, it should have never type.

Copy link
Owner

Choose a reason for hiding this comment

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

I found the relevant TS issue: microsoft/TypeScript#5413 Can you add a TODO comment about making it more strict when that issue is fixed?

}

export = delay
Copy link
Owner

Choose a reason for hiding this comment

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

Semicolon

Copy link
Owner

Choose a reason for hiding this comment

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

Can you use export default delay;?

Copy link
Contributor

Choose a reason for hiding this comment

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

export default isn't going to work because it's just CJS. So if you would write module.exports.default = delay, we could use export default delay; here as well.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, let's do that then. I think it's much nicer to allow TS consumer to use ES2015 import syntax.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"files": [
"index.js"
],
"typings": "index.d.ts",
Copy link
Owner

Choose a reason for hiding this comment

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

There's is no need to add this since it's named index, but you do need to add it to the files property.

"keywords": [
"promise",
"resolve",
Expand Down