-
Notifications
You must be signed in to change notification settings - Fork 214
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 type declaration file #190
Conversation
Thanks @adamvoss - I like it. @doug-martin thoughts? |
This provides completion information for users of both JavaScript and TypeScript with supported editors. This also allows TypeScript users to have type checking and be able to use the library when strict checking is enabled (which is the default for new TS projects).
c20ce04
to
3678dff
Compare
Did a force-push that corrected the EOL character. Also, corrected some of the |
7595064
to
25e648c
Compare
This would be a lovely thing to have! We're using I'm going to test this locally. |
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.
This looks really great, and the types are far more ambitiously detailed that I was planning to attempt.
Checking this against our in-house code using fast-csv
, I did notice two places where it might be improved, if I'm understanding everything correctly.
Thank you for working on this, and I'd love to have in an official fast-csv
release.
index.d.ts
Outdated
|
||
export = fast_csv; | ||
|
||
declare function fast_csv(): ParserStream; |
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.
As far as I can tell (from looking at the docs and source code), this should probably be:
declare function fast_csv(options?: ParsingOptions): ParserStream;
The implementation uses arguments
to get the argument list:
function csv() {
return parser.apply(void 0, arguments);
}
index.d.ts
Outdated
|
||
declare namespace fast_csv { | ||
// Circular reference from fast_csv | ||
function parse(): ParserStream; |
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.
I believe this should also be changed to:
function parse(options?: ParsingOptions): ParserStream;
Committer's note: @emk explicitly outlined these changed on GitHub, set him as author in way of attribution.
👍 Looks good to me! I've tested this against a real project, with the lastest Visual Studio Code and TypeScript 2.4.1. Everything works nicely. As heavy users of Thank you to @adamvoss for implementing this, and to the |
Any update on this? Will this be merged? Maybe need to fix the shippable issue first? |
Sorry - ill just merge it. |
Published https://www.npmjs.com/package/fast-csv |
This provides completion information for users of both JavaScript and TypeScript with supported editors. This also allows TypeScript users to have type checking and be able to use the library when strict checking is enabled (which is the default for new TS projects).