Skip to content
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

Ordering fields as in GQL query #24

Merged

Conversation

abidon
Copy link

@abidon abidon commented Jul 16, 2023

Summary

As defined in GraphQL specification (paragraph 7.2.2):

[...] the serialized Map of results should preserve this order by writing the map entries in the same order as those fields were requested as defined by query execution

Serialization formats which only represent unordered maps but where order is still implicit in the serialization’s textual order (such as JSON) should preserve the order of requested fields textually.

Fortunately, GraphQLSwift provides a special encoder for this purpose. The changes were relatively easy to implement.

What's new

  • GraphQL result serialization keep fields order as defined in query (by overriding Content's default implementation of GraphQLResult.encodeResponse(for: Request))
  • Adding a unit test (which both covers the serialization order and non scalar types)

@alexsteinerde
Copy link
Owner

Thanks for the PR! This is something I missed.
I really like to approve it.
But I'm not sure about potentially breaking things with this change. If someone is relying on the encoder that is set via Vapor this would break their assumption.
Do you know if it is possible to have the same result by using the encoder set in the Vapor context?

@cameroncooke
Copy link
Contributor

Any chance of getting this merged given the current implementation violates the GraphQL spec, if it's an issue of braking change, could this be opt-in only or released as a major version?

@alexsteinerde alexsteinerde merged commit 8179cae into alexsteinerde:master Jan 9, 2024
3 checks passed
@alexsteinerde
Copy link
Owner

Sure, merged.

@abidon abidon deleted the feature/preserve-query-fields-order branch January 9, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants