Merge all GraphQL types?

[dotansimha]

I was wondering why this package merges (without extend type) only Query, Mutation and Subscription. The logic I refer to is this: https://github.com/okgrow/merge-graphql-schemas/blob/master/src/merge_types.js#L8

I tried to create a version of this package without this constraint and everything works, and I can just write type User { A: String } and merge it with type User { B: String } (without the need for extend type).

[cfnelson]

@dotansimha The merge-graphql-schemas pkg was written to support merging types before the usage/adoption of extend (last I checked extend is still undocumented). This package should also support merging Input types see here.

What other types are you trying to merge? And we are always happy to accept PR's to improve/maintain this package. 😄

Maybe @RodMachado the original author might be able to better answer your question.

[dotansimha]

@RodMachado @cfnelson
I think we can merge everything.
Because merging type User { id: String } and type User { username: String } could be very helpful and helps to create much modular GraphQL server.
Also, I think that Enum values can be merged, and GraphQL directives can be merged together.

[cfnelson]

@dotansimha I believe this has been discussed at some point in the past but am unsure of the details. Will need to ask @RodMachado to clarify if there was a discussion and the outcome of it.

If I understand your suggestion correctly my main concern and I assume there will always be a contentious issue around what the expected behaviour should be and if we should allow/support resolving two of the same types together which would allow a scenario where you are trying to merge type User { id: String! } in fileA and type User { id: ID! } in fileB together and having to decide on how to handle/resolve any Field Definition conflicts.

[RodMachado]

Hey @dotansimha,

@cfnelson is right, we did have this discussion before and decided not to support multiple declarations for the same type.

Consider a scenario where there's a file A with type User { id: String } and a file be with type User { id: Int }.

Another scenario could be having type Address { zipcode: String } in file A and type Address { postalcode: String } in file B.

I know these are not the most elaborate examples, but I'm sure you can see how these could lead to some hard to debug or maintainability issues. Isn't it better to know that someone already created a type and that all fields are specified the same place?

Give it a thought. Maybe there are scenarios where this could be beneficial that we are not considering. I would love to hear your opinion.

Cheers!

[kohlikohl]

I would have following use case for merging every type together:
When using apollo-client and with the @client directives (which lets you create a client side field and resolver that does not exist on the graphql server), I would want to be able to combine a server schema with a client schema (being a superset of the server schema). I'd then use the resulting schema with apollo-codegen to generate interfaces for Typescript.

Granted, this is quite specific but with the rise of apollo client and typescript I can see this becoming a feature more devs would want.

Regarding the edge cases mentioned by @RodMachado: Why not just throw an error when finding fields with the same name that have different types and let the dev resolve it.
I don't see why the same field of the same type should be able to have two different data types.

[majelbstoat]

I don't know @kohlikohl (yet? 😉) , but it appears we've had the same requirements at the same time :) would love to voice support for the @client/apollo-codegen/typescript use-case.

[kohlikohl]

So I tried the code from PR #118 and it does exactly what I need.

[cfnelson]

merge-graphql-schemas v1.5.0 🎉 has just been released which includes the ability to merge a GQL Type defined multiple times in separate files. Will throw an error when fieldDefintitons have conflicting values defined. Usage mergeTypes(types, { all: true }). Many thanks to @squidfunk work for making this possible. To see more check out PR #118.