Graphql-js: `findBreakingChanges` function does not find all possible breaking chnges

[OlegIlyenko]

findBreakingChanges function was introduced in https://github.com/graphql/graphql-js/pull/508. Unfortunately it does not cover all of the cases.

For example:

• it does not consider field arguments at all
• adding a not-null input field is a breaking change (the same is true for arguments)

In some cases it may even produce false positives. For instance, changing the type of output field T → T! (making it a not-null field) is a backwards compatible change. The opposite is true for input fields: T! → T (making type nullable) is also backwards compatible change.

I ported this utility function to sangria and improved it a bit, including fixes for mentioned issues (I collect non-breaking changes as well). Maybe these test cases would be helpful (though I still not 100% sure that I have considered all possible breaking changes too):

https://github.com/sangria-graphql/sangria/blob/master/src/test/scala/sangria/schema/SchemaComparatorSpec.scala

[leebyron]

cc @sam-swarr to look into this.

[sam-swarr]

Thanks @OlegIlyenko! I'll try to get a PR out to fix the issues you found, else I'm happy to review if someone beats me to it.

[joelgriffith]

Fixed in https://github.com/graphql/graphql-js/pull/701

cc @sam-swarr

[joelgriffith]

Once 0.9.3 is out, we can probably consider closing this?

[OlegIlyenko]

Thanks a lot @joelgriffith for your work! 🏆

There are still some things missing (like directives, scalar values, etc.), but as far as this particular issue is concerned, it's implemented. I guess it would be to create new issues as new types of diffs are needed.

[wincent]

I guess it would be to create new issues as new types of diffs are needed.

Sounds like a good idea to me.