-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Change order of fields inside Response #384
Conversation
To reflect change from graphql/graphql-js#838
Why should the order matter? I like having |
I've always found the ordering requirements odd. At most, the specification can put a should on them, because JSON encoders are free to do whatever they want. If the internal representation of a JSON dict is a large hash table, then the rendering order is going to be arbitrary. Furthermore, some languages (Go is one example) randomizes the order in which you visit a hash table in order to make it explicit to the programmer that they cannot rely on the ordering. In short, requiring order here is rather brittle and is bound to create problems down the line. |
I think order requirements in the spec are for parsers which rely on the order to more efficiently parse the result. IMO that makes errors at the front even more valuable, since it is super useful to know about errors first in the case of a huge result. If it's framed as a recommendation it shouldn't be an issue since things are still valid GraphQL even if they return data first. |
You right. But I think the spec should recommend only best practices and error first approach provide better UX. Moreover spec recommendation should be implemented at least in the reference implementation. |
Yep, I just think there's a big difference between "recommend" vs. "require" in a spec! I totally agree errors first is the best approach. |
"A response to a GraphQL operation must be a map." does not imply order (otherwise it would say "ordered map") - I agree that the "first" and "next" terminology is more ambiguous than helpful in this context, though I like declaring the order via the order of paragraphs. I think we can add a more explicit phrase about ordering errors first in a formatted output as a convenience |
Remove "first" and "next" and include an explicit note about errors appearing first when serialized as a convenience.
@leebyron Great improvment 👍 |
Currently, the spec requires
![image](https://user-images.githubusercontent.com/8336157/33083372-e26a2dea-cee7-11e7-9361-a1b4a2686665.png)
data
to be the first field in a response.But after my PR was merged into
graphql-js
,errors
always precededata
:I still think that having
errors
as the first property is a big DX improvement so I updated the spec to reflect the change ingraphql-js
.