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

On specifying ordered vs unordered enum definitions #1062

Closed
cdaringe opened this issue Nov 20, 2023 · 5 comments
Closed

On specifying ordered vs unordered enum definitions #1062

cdaringe opened this issue Nov 20, 2023 · 5 comments

Comments

@cdaringe
Copy link

cdaringe commented Nov 20, 2023

Problem

There is no specification on whether EnumTypeDefinition are sorted by Name, and it causes friction for GQL clients who presume there is order.

Context

As observed from the above, ordering matters during codegen to GQL consumers. If a schema maintainer changed the enum to enum { Yy Xx Zz }, some clients would see this as a breaking change in the status quo.

Discussion

  • To help consumers/clients of GQL succeed, the spec should disambiguate ordering on EnumTypeDefinition.

Personal recommendation: enums and tagged unions alike are Sets, thus intrinsically unordered. We should state as much. Equipped with this assertion, client libraries whom seek to do type generation should apply stable sorting.

@benjie
Copy link
Member

benjie commented Nov 20, 2023

Good catch. In my opinion, like fields, they should be in the order in which they were defined, which effectively means tools are free to make their own decisions. Either way, the order should be stable in my opinion:

  1. introspect a GraphQL schema
  2. build a schema from these introspection results
  3. introspect this new schema using the same introspection query

The results of 1 and 3 should be identical.

@cdaringe
Copy link
Author

@benjie! long time, no-GitHub-see :)

Either way, the order should be stable in my opinion ...

Just to clarify, are you asserting that you think, in the spec, EnumTypeDefinition should specify that entries are ordered?

@martinbonnin
Copy link
Contributor

We've been occasionaly hit by such sorting issues in Apollo Kotlin and the more I think about it the more I think the "good" order for tools is the schema order. Everything else is surprising and error prone.

Sorting codegen by name is dangerous because the schema author doesn't control the sorting anymore so adding an enum starting with "a" offsets all the subsequent ones. All in all, my favorite solution is to keep things as is and have tools that analyze schema changes output a warning for such order changes.

@benjie
Copy link
Member

benjie commented Nov 21, 2023

@cdaringe 👋 😁

Just to clarify, are you asserting that you think, in the spec, EnumTypeDefinition should specify that entries are ordered?

Yes; I was writing a reply here but figured more valuable to just make the changes I want to see... so please see explanation in #1063

@cdaringe
Copy link
Author

Sounds good. Let’s move subsequent convo over there

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

No branches or pull requests

3 participants