TypeScript declarations don't have useful response types

justinfagnani commented 7 years ago

Every API call is annotation as returning Promise<any>, which makes the typings quite a bit less useful than they should be.

gr2m commented 6 years ago

I don’t know Typescript myself, what would you suggest?

orta commented 6 years ago

This is pretty tricky, we'd need to know the shapes of all responses upfront - this is feasible, but requires a bunch of scraping from the GitHub docs.

I did this for all GitHub webhook events ( https://github.com/orta/github-webhook-event-types ) for GH apps - but it's going to make the d.ts for this repo reeeealllly long. Ideally that could be its own module too.

gr2m commented 6 years ago

@zeke did some scraping tool recently. I’m just not familiar with Typescript myself. I can do the chore work of setting up scraping and running a daily cronjob to keep them up-to-date, we already have a semilar setup for fixtures: https://github.com/octokit/fixtures.

So it’d be great to know what the ideal resulting d.ts file would look like, and then we can figure out how to make it work and maintainable backwards from there :) I’d also need help from folks testing that actually use Typescript. Please comment here if you are interested, I’ll ping ya once we get there. Things will move faster now

justinfagnani commented 6 years ago

Would it have to use scraping? I assume GitHub generates their docs from some structured source. They probably have some incentive to publish it to enable higher-quality client APIs... Is there anyone to reach out to?

orta commented 6 years ago

I doubt they will provide structured data given the focus on a new v4 API (which is typed, so none of that data is useful long term) - WRT the types, you can throw any the JSON into http://json2ts.com to give a preview of that that looks like

gr2m commented 6 years ago

Is there anyone to reach out to?

Leave that to me :) Don’t worry if it’s being scraped or provided by GitHub directly. Given that we had all data we need, how would a d.ts need to look like to provide helpful context information for a method like github.repos.get(options)?

orta commented 6 years ago

As it is now:

  repos: {
    get(params: Github.ReposGetParams, callback?: Github.Callback): Promise<any>;
  }

As it would be using the repo from my webhooks:

export interface RepositoryOwner {
  login: string;
  id: number;
  avatar_url: string;
  gravatar_id: string;
  url: string;
  html_url: string;
  followers_url: string;
  following_url: string;
  gists_url: string;
  starred_url: string;
  subscriptions_url: string;
  organizations_url: string;
  repos_url: string;
  events_url: string;
  received_events_url: string;
  type: string;
  site_admin: boolean;
}

export interface RepositoryRepository {
  id: number;
  name: string;
  full_name: string;
  owner: RepositoryOwner;
  private: boolean;
  html_url: string;
  description: string;
  fork: boolean;
  url: string;
  forks_url: string;
  keys_url: string;
  collaborators_url: string;
  teams_url: string;
  hooks_url: string;
  issue_events_url: string;
  events_url: string;
  assignees_url: string;
  branches_url: string;
  tags_url: string;
  blobs_url: string;
  git_tags_url: string;
  git_refs_url: string;
  trees_url: string;
  statuses_url: string;
  languages_url: string;
  stargazers_url: string;
  contributors_url: string;
  subscribers_url: string;
  subscription_url: string;
  commits_url: string;
  git_commits_url: string;
  comments_url: string;
  issue_comment_url: string;
  contents_url: string;
  compare_url: string;
  merges_url: string;
  archive_url: string;
  downloads_url: string;
  issues_url: string;
  pulls_url: string;
  milestones_url: string;
  notifications_url: string;
  labels_url: string;
  releases_url: string;
  created_at: string;
  updated_at: string;
  pushed_at: string;
  git_url: string;
  ssh_url: string;
  clone_url: string;
  svn_url: string;
  homepage?: any;
  size: number;
  stargazers_count: number;
  watchers_count: number;
  language?: any;
  has_issues: boolean;
  has_downloads: boolean;
  has_wiki: boolean;
  has_pages: boolean;
  forks_count: number;
  mirror_url?: any;
  open_issues_count: number;
  forks: number;
  open_issues: number;
  watchers: number;
  default_branch: string;
}

export interface RepositoryOrganization {
  login: string;
  id: number;
  url: string;
  repos_url: string;
  events_url: string;
  members_url: string;
  public_members_url: string;
  avatar_url: string;
}

export interface RepositorySender {
  login: string;
  id: number;
  avatar_url: string;
  gravatar_id: string;
  url: string;
  html_url: string;
  followers_url: string;
  following_url: string;
  gists_url: string;
  starred_url: string;
  subscriptions_url: string;
  organizations_url: string;
  repos_url: string;
  events_url: string;
  received_events_url: string;
  type: string;
  site_admin: boolean;
}

export interface Repository {
  action: string;
  repository: RepositoryRepository;
  organization: RepositoryOrganization;
  sender: RepositorySender;
}

declare class Github {
 [ ... ] 
  repos: {
    get(params: Github.ReposGetParams, callback?: Github.Callback): Promise<Repository>;
  }

Though you always scope to GitHub. Repository in the current d.ts - so they'd exist inside the class

justinfagnani commented 6 years ago

Exactly. And ideally comments if they're available.

gr2m commented 6 years ago

This is great, thanks you two!