feat(toSDL): allow custom sorting and sorting by entity types

[rhengles]

I know this will very likely need refactoring before being accepted, but I want some input if this feature is desired.

By default, when you call mySchemaComposer.toSDL(), all objects are printed to a string, sorted by the name of the entity (be it scalar, enum, interface, input, object or union). I think a much more human friendly way of printing it is:

1. Scalar types
2. Root Object types (Query, Mutation, Subscription)
3. Enum types
4. Interface types
5. Input types
6. Object types
7. Union types

Actually I have almost no experience in GraphQL, I'm starting to learn and use it, but anyway I didn't like that the default way of printing mixes all these entity types. Maybe for an experienced GraphQL user the "ideal" way to sort entities when printing could be different, and the default way to sort entities by type could be changed in that case.

But what do you think?

How to use it?

import { SchemaComposer, printSortOptionsByType } from 'graphql-compose';

const schemaComposer = new SchemaComposer();

// build your schema

console.log(schemaComposer.toSDL({
	sortCustomOptions: printSortOptionsByType,
}));

[nodkz]

@rhengles great idea for custom sorting 👍
I agree that alphabetic+kind sorting is a good feature.

But we need to

• polish options (avoid simultaneous using of new options)
• gives better names for option and new type definitions
• add tests (it's the most important part!)
• try to simplify life for developers when they use custom built-in sorting (avoid importing built-in function in the app code via providing custom sorting name as a string)

[rhengles]

Thank you for your review and your directions! I'm very honored that you're willing to merge my suggestion, including the specific sorting order. But there's no need for two different sorting by types inside the package, at least from my point of view of course, we can replace the sorting by types with your suggestion (root, scalar, etc).

I'll do the changes you requested. Thank you again so much!

[rhengles]

@nodkz

• add tests (it's the most important part!)

I completely agree about this, I just want to reach an agreement first about the public interface, so that I won't need to waste time rewriting tests 😁

I've pushed another commit applying your suggestions (with some slight modifications 😅), what do you think now? Can you review it ?

Thanks a lot again for your attention.

[nodkz]

Looks very good 👍
It's time to write tests 😉

[rhengles]

@nodkz

I did some further simplification and added tests, please let me know if it is conforming to expectations.

Thanks

[nodkz]

According to tests printing of Directives was broken:

Did you run all tests locally?

[rhengles]

@nodkz You're right, I didn't, I only ran npx jest SchemaComposer-test.ts because my machine is slow, but after I finished I should have run them all.

I just pushed another commit fixing them, the tests passed on my machine this time, can you review it ?

[nodkz]

I think that [1, pos] is more expensive in runtime than 1 + rootPos/10. And comparePositionLists can be simplified to work with floats rather than arrays with loops/length checks:

function comparePositionLists(
  p1: number,
  p2: number,
): CompareTypeComposersResult {
  if (p1 < p2) return -1;
  if (p1 > p2) return +1;
  return 0;
}

Or maybe I something missing.

[rhengles]

Yeah, that's a tricky one. I always like to make functions as generic as possible to enable reuse (or future modification), but in this case I guess it's not warranted 😅

As I said before, I'm not an expert on GraphQL, so I don't know if it's possible that one can have 10 or more root objects in the schema, in which case the sorting would break. That's the only reason I'd want to avoid a float.

But what do you think about this?

function comparePositionLists(
  p1: number[],
  p2: number[],
): CompareTypeComposersResult {
  if (p1[0] < p2[0]) return -1;
  if (p1[0] > p2[0]) return +1;
  if (p1[1] < p2[1]) return -1;
  if (p1[1] > p2[1]) return +1;
  return 0;
}

[nodkz]

I so much like your sorting algorithm ❤️ that I want to make sortTypes = 'GROUP_BY_TYPE' by default in schemaComposer.toSDL().

v8.0.0 was published just 1-2 weeks ago, so we may introduce this breaking change under v8.1.0 (not under v9.0.0).

[nodkz]

@rhengles could you please allow for Maintainers to add changes in your Pull Request.
I wanted to add junit reporter with annotations in Github interface for broken tests.

[rhengles]

@nodkz Thank you! I never used that option before ("allow mantainers add changes"). From what I searched online, it should appear right on this page? But there is not that option here. ( print: https://prnt.sc/12rxrek )

Yes, my machine is not very fast so I ran the test with npx jest SchemaComposer-test.ts, sorry for not running them all.

I also would like if the sorting group_by_type was the default option 😀

I'll see each reply above, just wanted to give you a heads up about the setting you asked.

[nodkz]

I never used that option before ("allow mantainers add changes"). From what I searched online, it should appear right on this page? But there is not that option here.

Yeah, it's strange that you don't have this checkbox. Anyway when you open Pull Request checkbox (allow mantainers add changes) is still available.

I've just merged your changes in master and I'll make final changes during the day and publish a new package version. Thanks a lot for your solid improvements. I was very pleased to work with you! 🤩

PS. If you want to make improvements according to these changes or any other – feel free to open a new Pull Request. Thanks again 🙏💪

[github-actions]

🎉 This PR is included in version 8.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀