-
Notifications
You must be signed in to change notification settings - Fork 62
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
Require support of application/graphql-response+json by servers #327
base: main
Are you sure you want to change the base?
Conversation
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.
You've put it well in the description, I think this makes sense.
Clients can already use these benefits for responses that use it. For servers that don't support this content type, clients will still need to handle old-style responses. Fortunately, clients should be able to base decisions on the body of responses that return 2XX status and/or use The main thing that this change accomplishes is it makes it reasonable for clients to drop support for servers that don't implement
It does make a difference. It's not an official spec, and as such only the people involved in this WG and their colleagues are likely to have implemented it - we've hardly told anyone else about it! I'm sure all the big servers may have done, but there's a lot of software out there that serves GraphQL in really simple ways. You can spin up an old-school GraphQL server in like 20 lines of code (parse request body, validate types of parameters, feed result into
No, but it does mean that clients don't need to support them any more, and thus a software update to the latest client may break existing applications, causing people to choose between running outdated software on the client or nagging their server teams to deploy servers with updated compliance - requests that may well be met with crickets in some environments.
Agreed, but making it mandatory and marking everyone who doesn't do it as non-compliant may not be the best way of achieving this goal... Let's use the carrot rather than the stick - talk to people about the benefits of using this content type, and give them time to migrate over.
What are you seeing that simple clients will get out of this change? If their requests are successful or partially successful they'll get 2xx same as they ever did; the only statuses that change are those that fail completely (e.g. due to being invalid) can no longer be 2xx status, and for those they can either check for a missing The main value of the |
I can only offer my own personal experience, which shapes my own viewpoint, and why I am advocating for the switch. I started working with GraphQL around 6 years ago. I was tasked with writing a high-performance website, and chose to use GraphQL over simple REST APIs as it met all of these objectives:
However, the Apollo client for React had multiple major issues and could not be used. First, it was very bulky, disproportionate to the rest of the application size. Second, it was very slow, on two fronts: application startup incurred significant delays by Apollo processing the response (CPU time), and it injected __typename into every query, increasing bandwidth requirements. And finally, it had problems with how it handled I have no doubt that by now Apollo or another library may work for me now, but at the time, I just needed and wrote some React wrappers for fetch, designing a lightweight GraphQL library with some basic caching features, which has worked well for me. This brings me to the content type and response codes. I wanted my server to be able to easily tell whether the result was a good request or not. So I configured my servers to issue 4xx responses appropriately. I like being able to review the HTTP logs and determine which requests are failing (typically auth errors) and not. However, as many people have indicated, this can cause a conflict with HTTP relays and such returning 400 also. As such, As such, I recommend that So, this PR simply requires support of the new content type. It doesn't remove the requirement that |
What you describe explains exactly why this exists - so that you can see from the server logs what requests failed/succeeded - but it doesn't seem to answer the question as to how your client itself would benefit?
Indeed, but it does allow clients to stop supporting If we require all servers support the new content type, then taken at face value there's no reason for clients to support the legacy content type even though the code required to do so is minimal (i.e. to uncomment the commented part of this code: |
But the reverse is just as true. Why support the new content type if all servers support the old one? Why add support for a content type that is not even required to be supported by the spec? I follow your logic about the clients. But it seems we are fixing the wrong end of the problem. Why not simply require compliant clients to support both content types until 2030? |
As of 1/1/2025, the proposed watershed date has passed. As the GraphQL over HTTP specification has not been released yet, eliminating the watershed period entirely may leave clients without adequate time to adjust to some of the new behaviors mandated by the specification.
This PR aims to separate the date that the server is required to support
application/graphql-response+json
from the date that it is required to be considered the default when noAccept
header is provided. This allows for clients to immediately take advantage of the benefits of theapplication/graphql-response+json
media type for all compliant servers.I'm suggesting that this support is required immediately. While the original concept was that the spec would be released quickly, representing the current state of GraphQL servers, this did not occur. However, the draft GraphQL-over-HTTP spec has been out since 2020, and has had no changes of substance (of any kind) since the addition of the
application/graphql-response+json
media type in Aug 2022. As such, the fact that it was not "released as a spec" makes little difference. Server implementations have had time to review and implement the new features of the draft specification, and various servers have done so.Further, releasing the spec with this change does not break existing implementations. They simply are not officially compliant until they support the new content type. And finally, popular server implementations (assuming they have been watching this repository) have been expecting this change to take effect on 1/1/2025 and would expect to be non-compliant if they did not support the new content type.
So why make this change now? I believe the sooner that all servers support the new media type, the sooner that the entire GraphQL ecosystem will adopt it. Otherwise simple clients (e.g. handwritten with
fetch
) might not be able to use the new media type if servers are not supporting it, delaying general adoption.This PR is complementary to pushing the watershed date further out as proposed within: