Ensure types are consistently sorted in introspection result #959
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I do think performance is a concern here, those functions are called for example when an introspection query is made. I think this behavior is desirable, but we should probably refactor this so that it's done just once when the schema is generated... |
|
Merging this into a working branch to try and get it accomplished w/o the performance penalty |
|
I opened a new PR that extends this... It's not possible to sort these things at compile time since they are stored as maps. I also made sure each item that is a list is sorted... |
|
@binaryseed awesome, thanks for fixing! Will test this in our app :) |
aaronrenner
added a commit
to Boulevard/absinthe
that referenced
this pull request
Jun 22, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Wanted to get some feedback on how we're sorting these.
The reason for this change is because we have had issues using graphql-code-generator, which would output different order when the order of introspection changed (which happens when recompiling, I think because of how ParallelCompiler might randomize the order in which types are compiled). When considering which library to change, I considered Absinthe to be the best place, since one might expect the order of introspection to define the order of type generation, and that seems like it's generally a good practice.
I chose to add sorting to
Absinthe.Schema.types/1rather than to introspection alone, as I assumed sorting it there would be the most performant (as we're sorting just the keys, not the types themselves), but I can change this behaviour if this is a hot code path or if there are unwanted side effects to this.I also considered trying to keep the order in which they are defined/imported in the schema, but because maps with > 32 keys are not sorted, this would require additional work.