-
Notifications
You must be signed in to change notification settings - Fork 837
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 #444
Conversation
moment-timezone.d.ts
Outdated
__momentBuiltinFormatBrand: any; | ||
} | ||
|
||
type MomentFormatSpecification = string | MomentBuiltinFormat | (string | MomentBuiltinFormat)[]; |
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.
Is the inconsistent indentation intentional? (This and above vs. the interfaces below.)
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.
It was not intentional. Haven't done much with JS. I used the initial from DefinitelyTyped and referenced other typescript files to get to this one. If there is a preferred format, I can make the updates.
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.
Most of the file is indented correctly--just be consistent.
You should also add some tests like what moment has. See https://github.com/moment/moment/tree/d28a819dc668f107566f306f827df07d43e30287/typing-tests and https://github.com/moment/moment/blob/d28a819dc668f107566f306f827df07d43e30287/Gruntfile.js#L173. |
Not sure if this is something that can be added to the build deps to allow CI to continue, but there is a dependency on "grunt-exec" and "ntypescript". Tests came directly from DefinitelyTyped. |
Changes to ".travis.yml" align with the changes being requested in #310 |
Any updates on this getting merged in? |
So, this situation has been nothing but heartbreak since we started it in the main moment library. Does this package currently work? https://www.npmjs.com/package/@types/moment-timezone If so, strong preference for leaving them there. |
At this point I don't remember if it worked and I couldn't figure it out or if it didn't work. I am going to say I likely had an issue with the mentioned package likely similar if not the same as #343. |
Closing PR, trying to redo the update and it appears this was resolved, likely in #343 |
Reference: Add typescript definitions #341
Reference: Module 'moment' has no exported member 'MomentFormatSpecification' #343
I couldn't come up with a solution to 343 other than copying lines 9-13 into the typescript file for moment-timezone.
Supports: grafana/clock-panel#19