-
Notifications
You must be signed in to change notification settings - Fork 218
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
TODO for graphql-client 1.0 #15
Comments
First off: thanks for taking the project over and reviving it! There's one thing that would help us a bit: it would be great to have access to response headers from GraphQL HTTP call. I believe those were either discarded or just not returned by the client. Do you think that's doable? |
Hey, yes, I think it is. Here's where the request details are dropped: graphql-client/lib/graphql/client/http.rb Lines 73 to 80 in 925e3c7
I bet we can modify the returned object there to make it include the HTTP response and retain compatibility. I'll take a look! |
@rmosolgo Given that the response is either the body or the errors There are a few ways we could achieve this
One is a breaking change, and I would instead create a breaking change for 1.0 that is clearer. 2 I am not sure exactly how to code this in ruby, so I would not be able to help immediately. 3. This could cause issues in a multi-threaded environment. If we do create a breaking change we can include an option pre 1.0 that gives 1.0 behaviour to allow early usage and migration. This feature is quite important to me to handle rate limits so happy to make contributions. let me know your thoughts. |
Number 3 seems quite hacky, out of these 3 options I'm partial to number 2. Another potential solution would be to be able to opt into the old API that returns |
This library has been around a long time, and it's a hard dependency for a lot of projects. I'd like to release a 1.0.0 version, and before that, I have at least a few things in mind:
If anyone has other suggestions for 1.0, please share them in a comment below.
The text was updated successfully, but these errors were encountered: