-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat: Ignore sorting for queries and mutations when printing schema #2362
Conversation
Excellent enhancement 👍 |
The build is failing due to not updated unit tests snapshots. I will hold off with updates to not obfuscate the change that was done |
Yes. This is a very good enhancement. |
I totally forgot that schema will contain |
@wtrocki Thanks for PR 👍
Only schemas built from SDL has I experimented a bit with a few different approaches and found one that keeps the order intact without introducing the performance penalty and I'm working on PR. |
Thank you so much for reaching out. I have done some and found out that order will be preserved when iterating using Object.keys
Source: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map
Basing on what I see that could be the best way to get this. https://gist.github.com/wtrocki/df0f9cdca8f3c41d4d607a57a211e68f
That will be also a good alternative. Edit: |
73d8fee
to
c174dc8
Compare
c174dc8
to
0912c06
Compare
Fixes graphql#2362 It's a step toward having reversable `buildSchema`: ``` sdl === printSchema(buildSchema(sdl)) ``` At the same time if you need fully predictable SDL output (e.g. to diff schemas) you can achieve this using `llexicographicSortSchema`: ``` printSchema(lexicographicSortSchema(schema)) ``` But if some reason you can't use `lexicographicSortSchema` please open an issue and describe your use case in more details.
Fixes #2362 It's a step toward having reversable `buildSchema`: ``` sdl === printSchema(buildSchema(sdl)) ``` At the same time if you need fully predictable SDL output (e.g. to diff schemas) you can achieve this using `llexicographicSortSchema`: ``` printSchema(lexicographicSortSchema(schema)) ``` But if some reason you can't use `lexicographicSortSchema` please open an issue and describe your use case in more details.
The order of types returned by, e.g. `getImplementation`, will now be dependent on the order that those types first appear in the schema. See the referenced issues, the first of which implemented the change and the second which indicates the motivation. Ref: graphql/graphql-js#2410 Ref: graphql/graphql-js#2362
The order of types returned by, e.g. `getImplementation`, will now be dependent on the order that those types first appear in the schema. See the referenced issues, the first of which implemented the change and the second which indicates the motivation. Ref: graphql/graphql-js#2410 Ref: graphql/graphql-js#2362
… to upcoming changes in graphql 15.0.0 Related graphql/graphql-js#2362
The order of types returned by, e.g. `getImplementation`, will now be dependent on the order that those types first appear in the schema. See the referenced issues, the first of which implemented the change and the second which indicates the motivation. Ref: graphql/graphql-js#2410 Ref: graphql/graphql-js#2362
The order of types returned by, e.g. `getImplementation`, will now be dependent on the order that those types first appear in the schema. See the referenced issues, the first of which implemented the change and the second which indicates the motivation. Ref: graphql/graphql-js#2410 Ref: graphql/graphql-js#2362
In a general sense this just updates tests to accommodate the new error conditions and schema hashes since a lot of the work has already been done in #3712 / 131c9b8. A larger note is due to explain the updating the snapshots. Those changes are due the fact that types returned by, e.g. `getImplementation`, will now be dependent on the order that those types first appear in the schema. See the referenced issues, the first of which implemented the change and the second which indicates the motivation. Ref: graphql/graphql-js#2410 Ref: graphql/graphql-js#2362 Ref: #3712 (131c9b8)
Motivation
When using
printSchema
it's implementation will automatically sort all types by names.While for user types scalars and inputs this makes sense, for very large schemas finding mutation, query etc. can be challenging without using find command. Working with that schema layout is also hard as it is usually better to have mutations and queries together.
I heard opinions from the community that would expect schema to be organized the same way as it was before printing was applied. Print schema is often used by some CLI tools where we just want to append some specific object to the schema while operating on ast objects rather than strings. It will be amazing to see print schema being used more for those use cases.
How this could be done
Always put queries/mutations on the bottom of printed schema
This is implemented in this PR as I thought that it will be the least controversial change, but it looks like it is causing failures to some tests that rely on print schema order.
I wanted to make sure that the community will get that behavior out of the box without using some advanced functions etc. IMHO Scalars and directives could be also reorganized
Allow developers to apply their own sorting function
Currently, the order of the types in schema is not preserved as they are stored in map.
What it means is if users organized schema to their liking in DSL and then we parsed that into object original order is lost (not 100% sure about this but this is what I see happening)
To be able to get some organization when printing schema an separate sorting function could be applied. For example, first list scalars scalars and directives. Then user types in alphabetical order. Then Queries, Mutations. Subscriptions.
This can be done by adding additional parameter to options or exposing
printFilteredSchema
for advanced use cases