I like the PR but don't quiet understand why all the JSDoc needs to be touched to add typing support. Also when I copy the files into my node_modules folder (manually since its not yet in the repo), the axios. namespace gives me errors all over the place. How did it look like for you@Aendrew ?

I looked at this but it's a lot more work that I expected. I resolved the merge conflict anyways.

@Aendrew please merge my PR to your fork so if someone wants to pick it up they can:aendra-rininsland#1

Sorry for the really slow response to this issue!

@pascalwhoop I'm using JSDoc to generate the TypeScript definitions, and Axios returns a different Promise interface from the standard NodeJS promise. There are a few others that have been changed to allow tsd-jsdoc to properly generate interfaces.

@mohsen1 Cheers, thanks for doing that! I'll try to review and merge that sometime soon (ideally tomorrow but have been rather busy as of late).

tsd-jsdoc has released a new version and I plan to pick this up again shortly. Hopefully the new version will resolve some of the issues with interface generation that resulted in me having to tweak the JSDoc more than I'd have liked.

I was trying to use the generated .d.ts file directly in my project. It seems that all references toaxios.Promise that are marked as 'the Promise for the http request' should beaxios.AxiosPromise, according toaxios/index.d.ts.

Those axios related typings are broken since axios@0.15.3, which is the version I got when I installed github-api@3.0.0 with yarn.

Here is my standalone .d.ts file that works with webpack/ts-loader toolchain:
https://gist.github.com/akfish/b9a42945f1e681ed173b0b06365303a7

Given the codebase has moved along quite a lot and there's a new version oftsd-jsdoc, I've redone the TypeScript generation and made it so that no code changes are actually necessary. It's not quite automated yet, but the definition in the above commit should work a lot better. See above commit, or thedefinition here.

The steps needed to generate ad.ts file upon release are:

1. Generate d.ts vianpm run make-ts-def
2. Addimport {AxiosPromise} from 'axios' at top of newly-createdgithub-api.d.ts
3. Replace all instances ofPromise withAxiosPromise
4. Move theRequestable class into theRequestable module;export default it.
5. Replace all instances ofcallback withRequestable.callback (except inRequestable module itself)
6. Replace all instances ofauth withRequestable.auth (except inRequestable module itself)
7. Fix any line that includes nestedoptions values, i.e.:options.author?: any, options.commiter?: any, options.encode?: boolean
8. Change anyParams type toany
9. Fix errors re: optional arguments (add ?)
10. ChangeRequestable.auth to two overloaded interfaces, ensuring bothRequestable.auth andRequestable.callback are exported.

This admittedly isn't the best TypeScript def; it has far too manyany types in it. But it's definitely a start, and so far this is the only GitHub API lib around that has anything resembling a TypeScript def.

To improve this in the future:

• ReplacePromise withAxiosPromise in JSDoc. This will help a lot as it will get rid of a lot of theany types.
• Monitorthis bug, which will fix the issue with parsingoptions interfaces
• Probably worth somehow creating generic interfaces for options and params — having these documented as part of the definition greatly increases its usability.
• Figure out why Requestable isn't exported as part of Requestablemodule

The remaining steps can probably be accomplished using codemods or somesuch.

I don't know why tests are failing; am doubting it has anything to do with me given I don't touch the actual codebase in this PR.

@akfish Would you mind taking a look at this again? I'd normally just add the excellent def you wrote to the repo and close this PR, but I feel some sort of automatic generation is necessary to keep everything up-to-date.

Actually my typing was based on the automatic generation. So I can't take credit for that. :)

I took a look at the typing. It would be easier if some automatic tests are included. Here's how I set it up based onDefinitelyTyped's method:

I changed thetsconfig.json to:

{"compilerOptions": {"module":"commonjs","lib": ["es6"        ],"noImplicitAny":true,"noImplicitThis":true,"strictNullChecks":false,"baseUrl":"./","typeRoots": ["./"        ],"types": [],"noEmit":true,"forceConsistentCasingInFileNames":true    },"files": ["github-api.d.ts","tests.ts"    ]}

And added this line topackage.json:

{// other lines"types":"github-api.d.ts",// other lines}

And addedtests.ts:

// Write tests here// This file will be compiled by tsc to check typings// It will not be executedimportGitHub= require('github-api')letgh=newGitHub()

Then you just runtsc as the test.

Now here're some issues I spotted:

• I believe you will need this line at the beginning of the generated d.ts file:

  /// <reference types="axios" />

• Compiler error:

  tests.ts(3,10): error TS2351: Cannot use 'new' with an expression whose type lacks a call or construct signature.

  This suggests the exported signatures are not correct.
  Adding:

  exports=GitHub

  seems to fix this issue. But I'm not familiar with the code base so I'm not sure if this is a correct fix.

Seehttps://github.com/akfish/github/tree/d.ts

@akfish It might be thetsconfig.json that's causing the issue, IIRC settingtypes and/ortypeRoots will preventnode_modules/ and thus the Axios defs won't be picked up (from what I understand, using triple-slash directives in defs is considered a bad practice).

Will take a closer look sometime tomorrow! Thanks for picking this up and adding some tests!

No problem.

/// <reference types="..." /> is used for declaring type dependencies. It's widely used in@types packages. Though a second look at thedocument seems to suggest that it's only for@types/ packages. I'll verify this later.

I just verified that the/// <reference types="..." /> is not necessary for npm package typings. There's no need to changetsconfig.json too. It must be a problem with my global TypeScript version yesterday.

What's the status here?

I'm interested in this enhancement. Could I take over if no one's working on it?

@dvargas92495 Feel free! I don't expect I'll ever finish it tbth.