[WIP] Transfrorm schema API

[IvanGoncharov]

As discussed in #1185 I rewrote sort function to sort GraphQLSchema.
In a process, I notice that sortSchema duplicates > 50% of code from extendSchema so I decided to extract common part.
After this change, extendSchema became way simpler and #1095 trivial to implement.
To make this PR easier to review I split it into two commits one implementing sortSchema and another changing extendSchema.

@leebyron What do you think?

[langpavel]

Hi @IvanGoncharov Can you pick up use case for this?
I see it sorts even enum values. I hope that this cannot be dangerous even in any badly taken implementation..

[IvanGoncharov]

Can you pick up use case for this?

@langpavel See #941

I hope that this cannot be dangerous even in any badly taken implementation..

We are living in the dangerous world 😆 Seriously, the order of things shouldn't make any difference and even if for some weird reason you depend on it just don't use lexographicSortSchema on your schema.

[IvanGoncharov]

I renamed this function to lexographicSortSchema based on #941 (comment)

[IvanGoncharov]

I think implementing #922 and #1095 will be the real test for this API and may require some changes so I don't want to freeze this API

[IvanGoncharov]

@leebyron Can you please give a quick look at this PR?
This PR has some non-trivial changes (especially SchemaTransformer class) so I'm not sure if I'm on a right track.

[leebyron]

There's a lot happening in this PR that I think the quality could be improved by breaking it into smaller separate diffs. My suggested progression:

First, if the goal is a schema sorting function, I'd suggest building that directly. Then I'd suggest building this much more complex schema transformer API. Finally, I'd suggest using that transformer API to replace implementations of each function that makes sense.

That should be at least 3-4 separate PRs, each of which can use a previous PR as a base to make them reviewable as a stack

[leebyron]

I'm not sure why this is a class since the only think you can do with it is instance.transformSchema(). I'd suggest refactoring this to just export a function transformSchema(schema, transformer) since that presents the same behavior in a simpler API

[leebyron]

We should avoid * as much as possible - Flow often cannot resolve them, so these are either effectively all any or they may be derived to some type which could produce an error that is very difficult to understand. They appear a lot in this file, I think almost every case should be able to be replaced with a true type reference

[leebyron]

Why $Exact all of the config types?

[leebyron]

This API is a bit confusing - this isn't a transform function, it's something that takes a name and return a type, so I'd expect this to be more like the getType() API

[IvanGoncharov]

That should be at least 3-4 separate PRs, each of which can use a previous PR as a base to make them reviewable as a stack

@leebyron After #1208 being merged what is a next PR I should extract?
Maybe it worth to add toConfig method into every GraphQL*Type class since it useful on its own and will simplify implementation of transformSchema.

What do you think about adding toConfig and convert both lexographicSortSchema' and extendSchema` to use it as a next PR?

[leebyron]

That could definitely be a good next step - I'll leave it to you - it's also fine to build up multiple PRs, having each cite the previous as a way to illustrate a path of changes

[yaacovCR]

Inspired by this prior art, see now fleshed out mapSchema in ardatan/graphql-tools#1463

One implementation consideration I had that might be useful, right now FieldMappers get a fieldConfig and can return a fieldConfig, but TypeMappers get a type and have to return a type, so that a given object type could actually be converted to a scalar as desired and vice a versa.

This could be changed to eliminate that unlikely use case. Alternatively separate OBJECT_TYPE and OBJECT_TYPE_CONFIG mappers could be used. At this point, I am happy with retaining the discrepancy, as it maintains flexibility with decreased API surface, but it does require some repetitive

function mapperFunction(originalConfig) {
  const originalConfig = objectType.toConfig();
  ....<create a new config based on old>...
  return new GraphqlObjectType(newConfig)
}

instead of

function mapperFunction(originalConfig) {
  ....<create a new config based on old>...
  return newConfig
}

[IvanGoncharov]

Extracted in #3226