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

Use GitHub GraphQL query to get the file content #12

Closed
xcv58 opened this issue Feb 8, 2021 · 7 comments · Fixed by #81
Closed

Use GitHub GraphQL query to get the file content #12

xcv58 opened this issue Feb 8, 2021 · 7 comments · Fixed by #81
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@xcv58
Copy link
Collaborator

xcv58 commented Feb 8, 2021

There are multiple benefits to use GraphQL query instead of the RESTful endpoints:

  1. It will have fewer network calls against the RESTful endpoint so that the rate-limited issue would be mitigated a lot. The current implementation is per file per call, and GraphQL query could easily change to per-directory per query and could be optimized further.
  2. The current Quick Open feature (⌘P) supports the files only if they were opened before. This would be solved since GraphQL query could return all files & text content for a directory via a single query, and we could pre-populate the data to VSCode engine.
  3. The implementation can leverage GraphQL client built-in cache feature like https://www.apollographql.com/docs/react/caching/cache-configuration/.
  4. It's easier to add more GitHub-related features since no need to call different GitHub RESTful endpoints. Like switch branch, check out a previous commit, switch to a forked repo.

Sample GraphQL query:

https://gist.github.com/MichaelCurrin/6777b91e6374cdb5662b64b8249070ea

and screenshot via https://docs.github.com/en/graphql/overview/explorer

image

The blocker I saw is the GitHub GraphQL API needs authentication: https://docs.github.com/en/graphql/guides/forming-calls-with-graphql. Which could use the OAuth flow to build a UI redirect page to request before users start to use:
https://docs.github.com/en/developers/apps/authorizing-oauth-apps#web-application-flow

@conwnet
Copy link
Owner

conwnet commented Feb 8, 2021

Thanks for this wonderful idea, yes I thought GraphQL query would be better for this, I will try it in this days

@conwnet conwnet added the good first issue Good for newcomers label Feb 8, 2021
@xcv58
Copy link
Collaborator Author

xcv58 commented Feb 8, 2021

Thanks for your awesome idea & project. And I'd like to contribute and file PR.

Could you please tell me if you want any means to collaborate together?

@kanhegaonkarsaurabh
Copy link

Hey @conwnet First off, kudos on the project as it's been great to use! Lacking the use of Github's graphQL API is the first thing that I realized while looking at the codebase and I want to contribute to building this with you @xcv58. So let me know if I can grab this issue to work on or collaborate with you to work on it!

@xcv58
Copy link
Collaborator Author

xcv58 commented Feb 10, 2021

Thanks @kanhegaonkarsaurabh,

I think there are multiple tasks to enable GraphQL and we need input from @conwnet about how we deal with the users don't want to add a personal access token.

  • Add GitHub OAuth workflow to get token without users' manual effort
  • Evaluate & add a GraphQL client library that fits the use case (since most GraphQL clients are designed for front end frameworks not the way we would use)
  • Implement the readDirectory and readFile with GraphQL

For users who don't want to authorize their GitHub account, I think there are two solutions:

  • Use the RESTful endpoint for such customers (this will increase long term maintenance cost but the user experience is good)
  • Just don't support this use case with an explanation like GitHub did for their Explorer https://docs.github.com/en/graphql/overview/explorer
  • Build a simple GraphQL proxy for such use case and the proxy would use our own token (this wouldn't scale well because the host cost & rate limit, but we could reach to GitHub to see whether there is any alternative way to fetch the public data or increase the quota)

And above is just my thought, please share your thought @conwnet @kanhegaonkarsaurabh .

@xcv58
Copy link
Collaborator Author

xcv58 commented Feb 10, 2021

I had a sample apollo client setup here and will iterate on it: https://github.com/xcv58/github1s/pull/1/files

@conwnet
Copy link
Owner

conwnet commented Feb 10, 2021

Thanks everyone first. The GitHub OAuth workflow is really better way to improve experience.
In my opinion, the user experience is the first thing we should consider. I think maybe we should keep the RESTful endpoint as a fallback (even only 60 request per hour) for someone who just want to take a taste.
I think use own token maybe can not solve this problem (and even may been abused), and I dont know if there has other way to increate this. What do you think? @xcv58 @kanhegaonkarsaurabh

@xcv58
Copy link
Collaborator Author

xcv58 commented Feb 10, 2021

I think maybe we should keep the RESTful endpoint as a fallback (even only 60 request per hour) for someone who just want to take a taste.

+1 for this.

I think use own token maybe can not solve this problem (and even may been abused), and I dont know if there has other way to increate this.

We don’t need this approach if you think the effort to maintain the RESTful fallback is acceptable.

I had requested a high rate limit GitHub API access five years ago through education program. Not sure it’s still doable. But anyway I prefer to keep the RESTful fallback.

@xcv58 xcv58 added the enhancement New feature or request label Feb 11, 2021
@xcv58 xcv58 linked a pull request Feb 12, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants