Conversation
martinbonnin
left a comment
martinbonnin
left a comment
There was a problem hiding this comment.
Quick cursory review. Couple of comments but looks good!
| | `application/json` | An alternative type for responses (to support legacy clients) | | ||
| | Name | Description | | ||
| | ----------------------------------- | --------------------------------------- | | ||
| | `application/graphql-response+json` | The preferred type for server responses | |
There was a problem hiding this comment.
Does it still makes sense to use a table, now that there's only one row (same for the table for requests, above).
Also is "preferred" still the right word?
| | `application/graphql-response+json` | The preferred type for server responses | | ||
|
|
||
| Note: Servers MAY additionally support `application/json` as a response media | ||
| type. |
There was a problem hiding this comment.
Does it make sense to link somewhere that might specify what the behavior should be? (i.e. non-200x status codes unacceptable, etc.). We (the Apollo Client team) used this document to throw errors properly for application/json and non-compliant status codes, and with the removal here, that gets lost.
But maybe thats the point of this change? 🤔
There was a problem hiding this comment.
Would it be OK to put that as markdown file in this repo? https://github.com/graphql/graphql-over-http/LEGACY.md or something similar?
There was a problem hiding this comment.
I'm fine with that too! I'm mostly looking for it written down somewhere so that client libraries know what to expect.
There was a problem hiding this comment.
Yup, keeping that somewhere for reference would be greatly appreciated.
It would also be great if that document could show the q=0.9 as an example. I get why it's removed here, but implementers will need to know what to expect in real life, and some examples will keep the number of different usages down. Without examples, there will be all kinds of different strings floating around very soon.
|
@michaelstaib I took a stab at consistency and formatting there. I also added a You can browse it there: https://graphql-over-http.mbonnin.net/draft/#sec--application-json-response-media-type |
5 participants
No description provided.