Added flag for mergeTypes to merge all types

[squidfunk]

This PR adds the ability to merge custom types, as demanded here. Precisely, it adds an option to mergeTypes, so the behavior is entirely downward compatible. Tests are also included. Now we can do the following:

# a.gql
type Custom {
  id: ID!
}

# b.gql
type Custom {
  name: String
  age: Int
}

Merge types with:

mergeTypes(types, { all: true })

Result:

type Custom {
  id: ID!
  name: String
  age: Int
}

[cfnelson]

@squidfunk Thank you for the PR! It's appreciated a lot & thank you for including a test. 🔥 I have pulled down the branch and tested locally.

We would like to accept this PR but a few additional features & tests will need to be added before we will be comfortable accepting.

Two scenarios I noticed that were missing from the tests was when two or more Type Definitions had the same Field Definitions. e.g -> age field exists in both Types. The 2nd scenario is when the field definitions have differing type values.

export default `
  type Person {
    id: ID!
    name: String
    age: Float
  }
  type Person {
    name: String
    age: Int
  }
`;

Based on your modifications the output of the above would be:

"schema { query: Query } type Person { id: ID! name: String age: Float name: String age: Int }"

At minimum we would need to have additional tests created for the above scenarios and have them pass.

Suggestions for this:

For Scenario 1: name - we would expect name to only occur once in the Person type.

For Scenario 2: age - An acceptable first step would be to throw an Error and report where the conflicting field definitions are occurring to prompt the pkg user to resolve these conflicts in their type definitions.

There are some additional ideas/improvements that could be made and would be happy to collaborate/assist in improving this PR so that we can merge and accept it into master.

[squidfunk]

Thank you for the PR review. I will take a look and try to handle the two cases you discussed, but I may need some assistance/directions. I will keep you updated when I make progress.

[cfnelson]

@squidfunk No worries, will be happy to collaborate further to improve the PR and get it accepted. Having a working naive solution as a basis to work off would be a good starting point. We would want to have multiple tests to cover any other scenarios that we have yet to discuss as well.

[squidfunk]

Currently the problem with non-disjoint types (one field occurs in both type definitions) also applies to the mergeable type definitions (queries, mutations, subscriptions):

type Query {
  foo: Int
  bar: String
}
type Query {
  foo: String
  baz: Boolean
}

Results in:

schema {
  query: Query
}

type Query {
  foo: Int
  bar: String
  foo: String
  baz: Boolean
}

Therefore, in my opinion, it's not exclusively related to this PR. However, I will try to fix that too.

[squidfunk]

I covered the cases you specified in new test cases for custom types and for query types and seem to have fixed the stated issues. One issue remains: The arguments for fields (methods) of query types and custom types need to be checked prior to merging. However, this is also broken in the latest version and I would favor to do it in a separate PR (me or somebody else).

Any feedback is appreciated.

[squidfunk]

Fixed one more error for comment nodes

[cfnelson]

@squidfunk Thanks for the updates 🔥 . @RodMachado and myself will review and test these new updates hopefully sometime this week.

Did you have any opinions/thoughts on how to approach the argument fields issue? Attempt to resolve or throwing an error?

[squidfunk]

Did you have any opinions/thoughts on how to approach the argument fields issue? Attempt to resolve or throwing an error?

I would do it exactly as with the field types - if the definitions match, just merge them, otherwise throw an error. We also have to check arity and input types, but I think this should be done in a separate PR.

[mohebifar]

LGTM 👍 🚀 Definitely agree with throwing errors in case of conflicting fields and skipping the matching ones.

[kohlikohl]

What is the status of this PR?
Looks awesome and is exactly the addition to the library I am waiting for.
Anything I can do to help get this merged in?

[cfnelson]

@kohlikohl It's ready to go! Waiting for final review, though we shouldn't hold this up any longer. We just need to update the README & CHANGELOG. After this is merged we can bump the pkg versions and publish to npm.

Any help with the above would be appreciated otherwise I will try to get it out in the next 24hrs.

[peterclemenko]

This is going to be awesome

[cfnelson]

@RodMachado Been waiting for your review. Planning to merge without it now.