-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat(body-parsing): support the application/graphql+json
content-type
#6295
Conversation
c84a41c
to
b5f3c0d
Compare
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 76d03bf:
|
👷 Deploy request for apollo-server-docs pending review.Visit the deploys page to approve it
|
👷 Deploy request for apollo-server-docs pending review.Visit the deploys page to approve it
|
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.
🎉
Huh, that's news to me. Kinda surprised that the HTTP wg folks didn't reach out to server implementers when making that proposal, though maybe they did and we missed it. I think it's honestly a bummer to make this choice which means that GraphQL servers won't work with web framework JSON parsing libraries without extra configuration. In AS4 we are actually moving responsibility for JSON parsing out of our code and making it the users' responsibility (unless using the new |
I also note that it's not registered with IANA. |
Registering to IANA is on the issues currently graphql/graphql-over-http#31 the main driving factor was to have a unique way to identify graphql requests over http. I think other servers support the content-type implicitly as in they don't throw, apollo-server does not parse the json in this case |
The spec has been updated to no longer encourage the use of a custom request type (just a custom response type, which is supported in AS4). |
Yes, that's for responses, not requests.
…On Tue, Oct 18, 2022, 5:28 PM Scott Busche ***@***.***> wrote:
Am I reading the spec wrong?
Note: From 1st Jan 2025, every server and client must support
application/graphql-response+json, so including this in the Accept header
should give your client compatibility with any server.
—
Reply to this email directly, view it on GitHub
<#6295 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAECVAJVGQ2YQJJOSWJJTTWD46C7ANCNFSM5STN7TJQ>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Such a sad development from the GraphQL specification :( It's year 2022 and you still cannot reliably tell apart a GraphQL request from a seemingly compatible generic HTTP request. Although you can try inferring that from the request's Maybe one day. |
This PR aims to add support for the
application/graphql+json
content-type, this has been added to the spec here we see that we can support bothapplication/json
andapplication/graphql+json
.The main issue seems to be related to the body-parser used in koa and express as well as a check used in micro, an alternative would be to introduce a new body-parser.