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 typescript definitions #303

Merged
merged 13 commits into from
Apr 25, 2017
Merged

Add typescript definitions #303

merged 13 commits into from
Apr 25, 2017

Conversation

gpbl
Copy link
Owner

@gpbl gpbl commented Apr 20, 2017

See #299 and DefinitelyTyped/DefinitelyTyped#16010

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a4a4c4f on typescript-definitions into 17f707e on master.

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

Consider adding dtslint as well. I haven't tried it yet but it looks fairly self-contained (installs typescript & tslint for you) and applicable here.

cc @giladgray

// Type definitions for react-day-picker 5.2
// Project: https://github.com/gpbl/react-day-picker
// Definitions by: Giampaolo Bellavite <https://github.com/gpbl>, Jason Killian <https://github.com/jkillian>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need the above 4 lines, just the TypeScript compiler version

package.json Outdated
@@ -62,6 +62,7 @@
"peerDependencies": {
"react": "~0.13.x || ~0.14.x || ^15.0.0"
},
"types": "./react-day-picker.d.ts",

Choose a reason for hiding this comment

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

i'd say this belongs up near main and style keys as it's a distributable file

React.ComponentClass<WeekdayElementProps> |
React.SFC<WeekdayElementProps>;
weekdaysLong?: [string, string, string, string, string, string, string];
weekdaysShort?: [string, string, string, string, string, string, string];

Choose a reason for hiding this comment

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

real game changing feature would be to include /** docs */ on each of these props. i know you have great docs for every prop already... but that could easily be added in a future release.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 532d8c6 on typescript-definitions into 17f707e on master.

commit 0dcc6cd
Author: Giampaolo Bellavite <gpbellavite@gmail.com>
Date:   Tue Apr 25 12:30:01 2017 -0500

    Remove prop quick reference

commit fbc3979
Author: Giampaolo Bellavite <gpbellavite@gmail.com>
Date:   Tue Apr 25 12:29:38 2017 -0500

    Make example’s code cleaner

commit 536c8d0
Author: Giampaolo Bellavite <gpbellavite@gmail.com>
Date:   Tue Apr 25 12:25:08 2017 -0500

    Add docs for modifiersStyles

commit f96e389
Merge: 17f707e 461d377
Author: Giampaolo Bellavite <io@gpbl.org>
Date:   Thu Apr 20 11:13:56 2017 -0500

    Merge pull request #302 from gpbl/fix-function-modifier

    Fix function modifiers

commit 461d377
Author: Giampaolo Bellavite <gpbellavite@gmail.com>
Date:   Thu Apr 20 11:02:39 2017 -0500

    Add example

commit f997d6b
Author: Giampaolo Bellavite <gpbellavite@gmail.com>
Date:   Thu Apr 20 10:59:02 2017 -0500

    Accept function in array of modifiers
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 5f8a74d on typescript-definitions into 17f707e on master.

docs/API.md Outdated
@@ -1,47 +1,4 @@
# Component API

* [Component props](APIProps.md)
* [Component methods](APIMethods.md)

## Props quick reference
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you might want to add a link to types/index.d.ts here? People might not know to look for that immediately

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sure 👍🏽 However, aren't the definitions inferred by the tools using them?

PS. thanks @adidahiya @giladgray for the help! I never used TypeScript and I'm happy to learn something new!

Choose a reason for hiding this comment

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

not exactly: the tools rely on package.json "types" key to tell them where to find an entry .d.ts file, and the node_modules/@types scope directory to list all the available external modules.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 6d0b9e7 on typescript-definitions into 17f707e on master.

package.json Outdated
"files": [
"DayPicker.js",
"lib",
"moment.js",
"utils.js"
"utils.js",
"./types/index.d.ts"
Copy link

@giladgray giladgray Apr 25, 2017

Choose a reason for hiding this comment

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

for consistency i'd omit the ./ but whatever 🚢

(also above in "types")

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 2384a62 on typescript-definitions into 17f707e on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 40f02c7 on typescript-definitions into 17f707e on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 59a0aad on typescript-definitions into e40c06a on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 59a0aad on typescript-definitions into e40c06a on master.

@gpbl gpbl merged commit 72bb679 into master Apr 25, 2017
@gpbl gpbl deleted the typescript-definitions branch April 25, 2017 22:00
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.

4 participants