Remove application/json response section; additional editorial#404
Remove application/json response section; additional editorial#404benjie wants to merge 27 commits into
application/json response section; additional editorial#404Conversation
| To improve compatibility with _legacy client_, if the `Accept` header does not | ||
| indicate support for one of the server's preferred media types but does indicate | ||
| support for `application/json`, it is RECOMMENDED to perform the request as if | ||
| it had `Accept: application/graphql-response+json` but use | ||
| `Content-Type: application/json` for any response with a `2xx` status code. |
There was a problem hiding this comment.
This is the key new paragraph. This normative paragraph, this recommendation, eliminates most of the need for the application/json section in the spec (whether that be in the spec or in an appendix).
From what I understand, this follows the behavior that @michaelstaib has already implemented in HotChocolate.
…the server wording
* Restore important guidance and recommend support for Accept:application/json * Reword sentence to make the exclusion clear * Clarify another paragraph * Clarify * Non-normative recommendation of application/json for successful legacy requests * Legacy can use any 2xx status code * Consistent order * Trim extraneous text
benjie
force-pushed
the
legacy-client-editorial
branch
from
e6fbe5a to
1ea7ee4
Compare
There was a problem hiding this comment.
Looks good! 👍
PS: good job on filing this as PR #404 😄
benjie
changed the title
application/json response format; additional editorialapplication/json response section; additional editorial
| is RECOMMENDED. | ||
| - If the client did not produce a request within the time that the server was | ||
| prepared to wait, status code `408` is RECOMMENDED. | ||
| - If the size of the URI was too large, status code `414` is RECOMMENDED (and |
There was a problem hiding this comment.
Are we missing a ) somewhere here? That (and \422` is RECOMMENDED.` looks kinda out of place to me in general.
There was a problem hiding this comment.
You might be seeing a rendering bug? Here’s what I see:
jerelmiller
left a comment
jerelmiller
left a comment
There was a problem hiding this comment.
Love the changes! "does not conform to this specification" is so much simpler. Appreciate the updates!
|
Thanks for the feedback everyone; I've done some small tweaks to the wording to add clarity (and sidestep the plurality problem). If you're happy, we can get this merged and present the GraphQL-over-HTTP spec to the GraphQL WG tomorrow ready to join the main spec for spec cut in September! Please slap another ✅ on this even if it seems redundant. cc @michaelstaib @martinbonnin @Shane32 @jerelmiller @phryneas @BoD @PascalSenn |
7 participants
application/jsonresponses to Appendix A #379This pull request simplifies the GraphQL-over-HTTP spec by removing the sections on the legacy
Content-Type: application/jsonresponse type, and instead handling it through a recommendation: honour a request forapplication/jsonon successful requests, and useapplication/graphql-response+jsonfor all other responses1. This small change maintains broad (but not full) compatibility with legacy GraphQL clients without requiring an entire dedicated section (or appendix) detailing these rules.Further, I've done a full review of the specification and made a few other adjustments based on expected upcoming needs.
Footnotes
This is paraphrased, read the actual text for the specific wording. ↩